[Foundation] Set the MaxInputInMemory to match the limit from other BCL classes. Fixes #21537 #21660
Conversation
…CL classes. Fixes #21537 The MaxInputInMemory property is set by default to be long.MaxValue. This value is much larger than the one we can find in other BCL classes such as the HttpContent [max value](https://github.com/microsoft/referencesource/blob/master/System/net/System/Net/Http/HttpContent.cs#L20) and the MemoryStream [max value](https://github.com/dotnet/runtime/blob/1d1bf92fcf43aa6981804dc53c5174445069c9e4/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs#L37). It is important to notice that the HttpContent.MaxValue is used to limit the value of the MaxResponseContentBufferSize property in the HttpClient. This means that by making this move we are following the rest of the BCL. Fixes #21537
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
✅ API diff for current PR / commit.NET (No breaking changes)✅ API diff vs stable.NET (No breaking changes)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 103 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
There was a problem hiding this comment.
While I don't have any problems with this change, I'm not sure it's fixing the user's problem: files smaller than 2gb will still be cached in memory, and that's a rather big chunk of memory.
What about just not caching (or make it opt-in), by deleting this?
xamarin-macios/src/Foundation/NSUrlSessionHandler.cs
Lines 503 to 505 in 06d5fc8
It might be necessary to make it opt-in though, iirc there might be some differences wrt the Content-Length header when caching or no caching (can't compute Content-Length unless the data to send is cached).
The MaxInputInMemory property is set by default to be long.MaxValue. This value is much larger than the one we can find in other BCL classes such as the HttpContent max value and the MemoryStream max value.
It is important to notice that the HttpContent.MaxValue is used to limit the value of the MaxResponseContentBufferSize property in the HttpClient. This means that by making this move we are following the rest of the BCL.
Fixes #21537