Aspnetcore.docs: Memory issue in examples with retrieving collections

[komanton]

In the current page, I see three examples which use the following construction:

return View(await students.AsNoTracking().ToListAsync());

As far as I understood such construction is not correct in case of scaling an application due to improper memory consumption and may lead developers who read this doc to choose the wrong way in the implementation of their endpoints.

In details: ToListAsync - produces 'in-memory' buffers which have no constant size. Because the size depends on the number of rows which are retrieved from DB. So, if you multiply the number of requests and buffer without constant size, you may exceed the memory limitation of a container.

To sum up, I would suggest revisiting such examples to fix a memory issue.

Please, let me know if you require more information.

---

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

• ID: 15e2038b-fa1b-897c-32ee-7173c7d1d909
• Version Independent ID: 000c5a97-c1b2-9a48-289d-83ec4f0c0c98
• Content: Tutorial: Add sorting, filtering, and paging - ASP.NET MVC with EF Core
• Content Source: aspnetcore/data/ef-mvc/sort-filter-page.md
• Product: aspnet-core
• Technology: aspnetcore-data
• GitHub Login: @tdykstra
• Microsoft Alias: riande

[Rick-Anderson]

@dougbu I'm going to add a caveat to this doc:

The preceding code works for for small data sets. A real app should limit the number of rows returned.

cc @tdykstra

[dougbu]

@Rick-Anderson that sounds fine. I suggest the caveat should mention settings that make ToListAsync() and similar safe.

Model binding should limit collections to reasonable sizes e.g. using MvcOptions.DefaultMaxModelBindingCollectionSize. @ajcvickers does EF Core have limits on returned data sets that would be applicable here?

[komanton]

@Rick-Anderson I expected that examples will be modified to use the new feature: async enumerable, something like this:

return View(await students.AsNoTracking().ToEnumetableAsync());

I saw this method in the latest version of EF.

I am not sure that some limitations for the result sets may help in this case. Because even 10 records in results will appear like gigabytes of ram at scaling on thousands of requests.

The idea with enumerable is having a memory buffer for only one record at any moment. Because it will be work like streaming without huge buffers.

[Rick-Anderson]

@komanton can you send me a link to ToEnumetableAsync

[komanton]

@Rick-Anderson The correct name of this method is AsAsyncEnumerable

I saw documentation for it here

[Rick-Anderson]

@ajcvickers should we replace the scaffolded code
return View(await students.AsNoTracking().ToListAsync());

with
return View(await students.AsNoTracking().AsAsyncEnumerable());

for queries that are likely to return many entities?

[ajcvickers]

@smitpatel @AndriySvyryd @roji Thoughts on this?

[AndriySvyryd]

Sounds like a reasonable suggestion, though to really fix the issue .Take(DefaultMaxModelBindingCollectionSize) should probably be added.

[roji]

I don't think it's possible to await an IAsyncEnumerable. However, unless I'm mistaken ASP.NET's View() should be able to accept an IAsyncEnumerable directly (at the very least it seems to allow this in web API, which is ideal).

There's one more important point here - whether the IAsyncEnumerable is going to be enumerated multiple times by the view. If it is, it's probably better to do ToListAsync and pass that as the model, although everything depends on the actual amount of data involved. In general, a web page shouldn't be loading an arbitrary number of rows - some sort of paging/limiting is typically involved, in which case ToListAsync should usually be fine.

So there may not be one definite answer here :) The above may be too much complexity for introductory examples, but I'd say that in the general case ToListAsync may be safer.

[smitpatel]

https://github.com/aspnet/EntityFrameworkCore/issues/5089

[komanton]

I don't think it's possible to await an IAsyncEnumerable. However, unless I'm mistaken ASP.NET's View() should be able to accept an IAsyncEnumerable directly (at the very least it seems to allow this in web API, which is ideal).

There's one more important point here - whether the IAsyncEnumerable is going to be enumerated multiple times by the view. If it is, it's probably better to do ToListAsync and pass that as the model, although everything depends on the actual amount of data involved. In general, a web page shouldn't be loading an arbitrary number of rows - some sort of paging/limiting is typically involved, in which case ToListAsync should usually be fine.

So there may not be one definite answer here :) The above may be too much complexity for introductory examples, but I'd say that in the general case ToListAsync may be safer.

@roji Thanks for the detailed explanations. Looks like this doc: in web API is what I have expected to see in the documentation actually. I have no questions at his moment. And, probably, this issue may be closed.

But looks like implementation said that MVC internally creates a buffer for results:

MVC automatically buffers any concrete type that implements IAsyncEnumerable

Probably, it is a topic for another discussion. Because it is really hard to understand why it creates an internal buffer for results? From my point of view, it is really a performance problem. Why not just write each record from action directly to the network stream, like into some file.

[Rick-Anderson]

Need to leave it open to warn about number of rows returned.

[ajcvickers]

@rynowak I think it would be useful to align understanding between EF and MVC here around the best practices for using IAsyncEnumerable. Would it be worth putting something on the calendar for the new year, or do you think we can come to a conclusion here?

[Rick-Anderson]

@rynowak I think it would be useful to align understanding between EF and MVC here around the best practices for using IAsyncEnumerable. Would it be worth putting something on the calendar for the new year, or do you think we can come to a conclusion here?

[ajcvickers]

/cc @JeremyLikness

[Rick-Anderson]

@JeremyLikness @ajcvickers I'll be updating the RP/EF and I'd like to include this. @mkArtakMSFT who's the right person now that RyanNwak is gone?

[Rick-Anderson]

Fixed in #19922 at https://docs.microsoft.com/en-us/aspnet/core/data/ef-rp/intro?view=aspnetcore-5.0&tabs=visual-studio#performance-considerations in