aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

Results.File and Results.Stream consume large amount of Memory, don't stream file directly anymore

Open davepermen opened this issue 2 years ago • 11 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the bug

using MapGet("/", () => Results.File("bigfile.bin")) causes huge amount of memory consumption, and multiple seconds of delay till the download starts.

In .net6, download started instantly and used up no memory.

Same Issue on Results.Stream

Bug with screenshots for comparison visible here:

image image

Expected Behavior

Results.File and Results.Stream should directly stream the file download without loading it first into ram.

Steps To Reproduce

I created a repo here, needs a big file in wwwroot/1gb.bin

https://github.com/davepermen/ResultsFileBug

to create file quickly (on windows): open cmd in wwwroot and run "fsutil file createnew 1gb.bin 1073741824" but any file works.

switch project between net6 and net7 to see the effect.

Exceptions (if any)

No response

.NET Version

7.0.100

Anything else?

No response

davepermen avatar Nov 11 '22 22:11 davepermen

@davepermen Are you seeing this with HTTP/2? I assume you are using the browser to download the file?

davidfowl avatar Nov 12 '22 04:11 davidfowl

Yes, both cases the browser responds with "h2" (i assume it's short for HTTP/2).

i the browser network tools, it states on net6: timing: it starts after 4.8ms. in net7, 3.2s. transfer itself is a bit above 3seconds in both.

davepermen avatar Nov 12 '22 12:11 davepermen

I can reproduce the issue, looking into it.

davidfowl avatar Nov 13 '22 05:11 davidfowl

OK this seems to be a bug in the BrowserLink middleware (something visual studio installs to do refreshing of HTML).

image

Seems like there's a large allocation happening.

@davepermen You can turn off it off with this feature in visual studio:

image

Can you also confirm that this does not repro from the command line?

cc @vijayrkn @jodavis

davidfowl avatar Nov 13 '22 06:11 davidfowl

OK it's not the browser link middleware but it seems like the BrowserRefresh middleware is also to blame here 😄. (sorry @vijayrkn and @jodavis).

@MackinnonBuck it looks like this change might be the problem. We're using a memory stream to buffer the entire response body instead of the passthrough we had before.

davidfowl avatar Nov 13 '22 06:11 davidfowl

I can confirm, "dotnet run" from the commandline works as expected, and disabling CSS Hot Reload solves the issue in Visual Studio for now, till a proper fix is back 👍

davepermen avatar Nov 13 '22 11:11 davepermen

Yep, we changed the BrowserRefresh middleware to buffer the entire response to improve the reliability of hot reload script injection. We could fix the memory usage problem by checking earlier whether we're working with an HTML response so we can switch back to a passthrough in cases where we know script injection can't be performed.

Possible servicing candidate?

cc @mkArtakMSFT. Note that this issue only affects development scenarios with hot reload enabled.

MackinnonBuck avatar Nov 14 '22 18:11 MackinnonBuck

@mkArtakMSFT @MackinnonBuck yes we need to service this.

davidfowl avatar Nov 15 '22 04:11 davidfowl

Yep, we changed the BrowserRefresh middleware to buffer the entire response to improve the reliability of hot reload script injection. We could fix the memory usage problem by checking earlier whether we're working with an HTML response so we can switch back to a passthrough in cases where we know script injection can't be performed.

I think we should revert the fix and work on a more reliable solution in .NET 8. We need to get rid of the buffering. Developers doing performance testing while running visual studio are going to see huge problems with that.

davidfowl avatar Nov 15 '22 04:11 davidfowl

@davidfowl What is the right way to handle large files? If I need to transfer it.

I have so far come to the solution that I open the stream from the disk and write the data into HttpResponce, but still for a 250mb file it is a problem.

I also tried to do when I receive a file from a user, first write it to disk, and then take action.

I also have a http stream in memory anyway.

Do you have any examples?

KSemenenko avatar Nov 15 '22 10:11 KSemenenko

The sample here is fine. The problem is our middleware that is buffering the response (as described in the replies above).

davidfowl avatar Nov 16 '22 20:11 davidfowl

I have spend multiple hours trying to figure out why this simple code wasnt working

just create a new asp net web core web app mvc, put this in the home controller, .net 7.0.1 and vs 17.4.3

       public async Task<FileStreamResult> Test()
        {
            //random big file found online
            //const string url = "http://ipv4.download.thinkbroadband.com/512MB.zip";
            const string url = "https://speed.hetzner.de/1GB.bin";
            var client = new HttpClient();
            var stream = await client.GetStreamAsync(url);
            return File(stream, "application/zip");
        }

it always load everything in memory before the browser start downloading

I tried disabling the CSS Hot Reload, it doesnt work

it only work when i do a dotnet run

Spirch avatar Dec 27 '22 08:12 Spirch