Skip to content

[release/7.0.2xx] Revert hot reload script injection response buffering #29152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

MackinnonBuck
Copy link
Member

Summary

Fixes a regression in memory consumption caused by large responses when debugging with hot reload enabled.

Description

As one of the changes introduced in .NET 7 by #27537, the hot reload script injection middleware now buffers the entire response before performing script injection. This was done to improve script injection reliability, but we recently began receiving reports from customers that were confused by the increase in server memory usage for large responses when debugging with hot reload is enabled.

This PR reverts the buffering behavior but keeps the logging improvements that help customers identify the cause of script injection failures and take steps to fix them.

For .NET 8, we will assess an alternative approach to improve script injection reliability that doesn't involve buffering each response.

Validation

Manual validation + existing automated tests for this scenario.

Risk

Low. This PR simply reverts the offending commit and adds back some of the improved logging behavior with no other changes to functionality.

Fixes dotnet/aspnetcore#45037

@ghost ghost added the Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch label Nov 21, 2022
@javiercn
Copy link
Member

@MackinnonBuck sounds good.

Could you file an issue to revisit this in .NET 8.0? We should consider avoiding the script injection magic in favor of referencing the file in the document directly somehow.

@MackinnonBuck
Copy link
Member Author

Are we clear to merge this?

@mkArtakMSFT
Copy link
Contributor

Hi @marcpopMSFT. Can this be merged or should @MackinnonBuck wait for some green light from you?

@mkArtakMSFT
Copy link
Contributor

Talked to @marcpopMSFT - the branch is open to merge.

@mkArtakMSFT mkArtakMSFT merged commit e9b980c into release/7.0.2xx Jan 4, 2023
@mkArtakMSFT mkArtakMSFT deleted the mbuck/revert-hot-reload-response-buffering branch January 4, 2023 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants