use PoolingAsyncValueTaskMethodBuilder in BufferedFileStreamStrategy

[adamsitnik]

@stephentoub from the initial benchmark run it seems that it saves a lot of allocations when buffering is actually used:

Method	Toolchain	fileSize	userBufferSize	options	Mean	Ratio	Allocated
ReadAsync	\main\corerun.exe	1048576	512	None	1.470 ms	1.00	83 KB
ReadAsync	\pool\corerun.exe	1048576	512	None	1.399 ms	0.95	33 KB
ReadAsync	\main\corerun.exe	1048576	512	Asynchronous	3.265 ms	1.00	55 KB
ReadAsync	\pool\corerun.exe	1048576	512	Asynchronous	3.269 ms	1.00	5 KB

I am going to run all benchmarks MANY times (>1h) and get back with full results. In the meantime, I hope the CI can test it for me

[ghost]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

---

@stephentoub from the initial benchmark run it seems that it saves a lot of allocations when buffering is actually used:

Method	Toolchain	fileSize	userBufferSize	options	Mean	Ratio	Allocated
ReadAsync	\main\corerun.exe	1048576	512	None	1.470 ms	1.00	83 KB
ReadAsync	\pool\corerun.exe	1048576	512	None	1.399 ms	0.95	33 KB
ReadAsync	\main\corerun.exe	1048576	512	Asynchronous	3.265 ms	1.00	55 KB
ReadAsync	\pool\corerun.exe	1048576	512	Asynchronous	3.269 ms	1.00	5 KB

I am going to run all benchmarks MANY times (>1h) and get back with full results. In the meantime, I hope the CI can test it for me

Author:	adamsitnik
Assignees:	-
Labels:	area-System.IO
Milestone:	-

[stephentoub]

If the numbers end up looking good, LGTM. As I noted offline, this is one of the places I'd been eyeing for us to look at using it. In addition to perf tests validating serialized execution, it'd also be good to run some that stress doing operations on multiple FileStreams in parallel, as you might find on a server. I'm hopeful that my recent changes to the pooling builder will avoid any deleterious effects there.

[karelz]

Http3_MsQuic test failures are tracked in #56090. Should be fixed and merged shortly. Sorry for the inconvenience!

[adamsitnik]

The perf numbers LGTM, there is a nice allocation win for the scenario when buffering is enabled and user buffer is small (512 bytes):

Method	Toolchain	fileSize	userBufferSize	options	Mean	Ratio	Allocated
ReadAsync	\main\corerun.exe	1048576	512	None	1,477.87 us	1.00	84,662 B
ReadAsync	\pool\corerun.exe	1048576	512	None	1,388.53 us	0.94	33,645 B
WriteAsync	\main\corerun.exe	1048576	512	None	4,314.56 us	1.00	78,236 B
WriteAsync	\pool\corerun.exe	1048576	512	None	4,265.07 us	1.00	33,504 B
ReadAsync	\main\corerun.exe	1048576	512	Asynchronous	3,215.73 us	1.00	56,246 B
ReadAsync	\pool\corerun.exe	1048576	512	Asynchronous	3,207.38 us	1.00	5,212 B
WriteAsync	\main\corerun.exe	1048576	512	Asynchronous	5,820.55 us	1.00	50,140 B
WriteAsync	\pool\corerun.exe	1048576	512	Asynchronous	5,765.10 us	0.99	5,399 B

[adamsitnik]

it'd also be good to run some that stress doing operations on multiple FileStreams in parallel, as you might find on a server

We don't have such tests yet. The scenario covered by ASP.NET benchmarks uses FileStream.CopyToAsync which was optimized before the .NET 6 FileStream rewrite and it's not using buffering at all, so we won't notice any difference there.

[davidfowl]

👍🏾 . I'm looking to make websocket reads allocation free as well 😄

[stephentoub]

I'm looking to make websocket reads allocation free as well

As long as we continue to support CloseAsyncs being issued concurrently with ReadAsyncs, that's going to be very challenging to do. (In the past I've questioned whether that's something we need to maintain, but I don't have evidence one way or another as to whether folks might be doing that.)

[stephentoub]

As long as we continue to support CloseAsyncs being issued concurrently with ReadAsyncs, that's going to be very challenging to do.

Actually, I came at it a different way and think I have a reasonable solution.