Use UnixFileStream's ReadAsync implementation on Windows when !IsAsync

[stephentoub]

UnixFileStream's ReadAsync implementation uses a reusable IValueTaskSource implementation to avoid allocating a new work item on every read. We can push that implementation down to OSFileStreamStrategy, and then use it for the Windows implementation of ReadAsync as well when IsAsync==false, rather than delegating to the base Stream implementation.

This PR almost entirely just moves code around. The only change to logic is in RandomAccess.Windows.cs, to only set an offset into the NativeOverlapped if the SafeFileHandle is seekable; otherwise, it fails when used with pipes.

Method	Toolchain	Mean	Error	StdDev	Ratio	Gen 0	Allocated
Read	\main\corerun.exe	36.19 ms	0.568 ms	0.531 ms	1.00	-	31 B
Read	\pr\corerun.exe	35.71 ms	0.296 ms	0.263 ms	0.99	-	34 B
ReadAsync	\main\corerun.exe	45.84 ms	0.360 ms	0.337 ms	1.00	90.9091	1,094,165 B
ReadAsync	\pr\corerun.exe	42.74 ms	0.434 ms	0.406 ms	0.93	-	256 B

    private byte[] _buffer = new byte[4096];
    private FileStream _stream;

    [GlobalSetup]
    public void Setup()
    {
        byte[] bytes = new byte[40_000_000];
        new Random().NextBytes(bytes);

        string path = Path.GetTempFileName();
        File.WriteAllBytes(path, bytes);

        _stream = File.OpenRead(path);
    }

    [GlobalCleanup]
    public void Cleanup()
    {
        _stream.Close();
        File.Delete(_stream.Name);
    }

    [Benchmark]
    public void Read()
    {
        _stream.Position = 0;
        while (_stream.Read(_buffer) != 0) ;
    }

    [Benchmark]
    public async Task ReadAsync()
    {
        _stream.Position = 0;
        while (await _stream.ReadAsync(_buffer) != 0) ;
    }

cc: @adamsitnik

[ghost]

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

Issue Details

---

UnixFileStream's ReadAsync implementation uses a reusable IValueTaskSource implementation to avoid allocating a new work item on every read. We can push that implementation down to OSFileStreamStrategy, and then use it for the Windows implementation of ReadAsync as well when IsAsync==false, rather than delegating to the base Stream implementation.

This PR almost entirely just moves code around. The only change to logic is in RandomAccess.Windows.cs, to only set an offset into the NativeOverlapped if the SafeFileHandle is seekable; otherwise, it fails when used with pipes.

Method	Toolchain	Mean	Error	StdDev	Ratio	Gen 0	Allocated
Read	\main\corerun.exe	36.19 ms	0.568 ms	0.531 ms	1.00	-	31 B
Read	\pr\corerun.exe	35.71 ms	0.296 ms	0.263 ms	0.99	-	34 B
ReadAsync	\main\corerun.exe	45.84 ms	0.360 ms	0.337 ms	1.00	90.9091	1,094,165 B
ReadAsync	\pr\corerun.exe	42.74 ms	0.434 ms	0.406 ms	0.93	-	256 B

    private byte[] _buffer = new byte[4096];
    private FileStream _stream;

    [GlobalSetup]
    public void Setup()
    {
        byte[] bytes = new byte[40_000_000];
        new Random().NextBytes(bytes);

        string path = Path.GetTempFileName();
        File.WriteAllBytes(path, bytes);

        _stream = File.OpenRead(path);
    }

    [GlobalCleanup]
    public void Cleanup()
    {
        _stream.Close();
        File.Delete(_stream.Name);
    }

    [Benchmark]
    public void Read()
    {
        _stream.Position = 0;
        while (_stream.Read(_buffer) != 0) ;
    }

    [Benchmark]
    public async Task ReadAsync()
    {
        _stream.Position = 0;
        while (await _stream.ReadAsync(_buffer) != 0) ;
    }

cc: @adamsitnik

Author:	stephentoub
Assignees:	-
Labels:	area-System.IO, tenet-performance
Milestone:	6.0.0

[adamsitnik]

LGTM, thank you @stephentoub !

[adamsitnik]

after this code removal, SyncWindowsFileStreamStrategy and UnixFileStreamStrategy become de facto classes that do not change or extend the behaviour of OSFileStreamStrategy.

Have you considered the removal of SyncWindowsFileStreamStrategy and UnixFileStreamStrategy and using OSFileStreamStrategy directly?

[stephentoub]

No strong opinion. If you'd like to consolidate, go for it. If not, I'm ok keeping the derived types.

[adamsitnik]

I am going to merge it now so I can get updated numbers for the blog post ;)

[adamsitnik]

I am not sure if this was intentional, but after moving all the logic from Unix to OS strategy it's now much easier to work with the code on Windows. Refactoring in VS renames all usages of methods, and it's less likely for the code to compile fine on Windows and fail on Unix. 👍