Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Efficient RandomAccess async I/O on the thread pool. #55123

Merged
merged 10 commits into from
Jul 4, 2021

Conversation

teo-tsirpanis
Copy link
Contributor

Until now, every time an async file I/O operation could not complete truly asynchronously (meaning on Windows when the file handle is not opened for async I/O and on non-Windows always), the operation would be scheduled to run on a thread pool thread. This scheduling used to be performed by calling Task.Factory.StartNew, and wrapping the Task to a ValueTask.

The problem with this approach is that contrary to ValueTask's philosophy and to Windows' behavior when using async file handles, every async I/O operation on the thread pool would allocate. This PR addresses that.

A new class named SafeFileHandle.ThreadPoolValueTaskSource was created in the mold of the existing Windows-only ValueTaskSource (which was renamed to OverlappedValueTaskSource) that schedules RandomAccess file operations on the thread pool. This class implements IValueTaskSource of int and long (to support both regular and vectored I/O) but also IThreadPoolWorkItem to directly queue itself on the thread pool. Combined with a reusing mechanism like OverlappedValueTaskSource's, it allows async RandomAccess I/O to be always amortized allocation-free per SafeFileHandle, when used non-concurrently.

…ss I/O on the thread pool.

And use it in the RandomAccess.ScheduleSync methods instead of wrapping Task.Factory.StartNew in a ValueTask.
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@teo-tsirpanis
Copy link
Contributor Author

teo-tsirpanis commented Jul 4, 2021

Done. Queueing the work item to the thread pool was factored in one method that also captures the ExecutionContext which means that the PreferLocal constant was used only once and was removed.

@stephentoub
Copy link
Member

Thanks. Can you share perf numbers?

@teo-tsirpanis
Copy link
Contributor Author

Unfortunately not. 😕

@teo-tsirpanis
Copy link
Contributor Author

Libraries tests pass, failures seem unrelated. 🙏🏻

@stephentoub
Copy link
Member

Method Toolchain UseAsync Mean Ratio Allocated
SyncRead /main/corerun False 5,673,251.7 ns 1.00 6 B
SyncRead /pr/corerun False 5,746,850.8 ns 1.01 6 B
AsyncRead /main/corerun False 11,837,820.4 ns 1.00 1,172,251 B
AsyncRead /pr/corerun False 10,621,098.3 ns 0.90 211 B
SyncRead /main/corerun True 5,817,949.0 ns 1.00 6 B
SyncRead /pr/corerun True 5,749,625.5 ns 0.99 6 B
AsyncRead /main/corerun True 12,165,759.4 ns 1.00 1,172,251 B
AsyncRead /pr/corerun True 10,705,724.0 ns 0.88 211 B
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Columns;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Diagnosers;
using BenchmarkDotNet.Reports;
using BenchmarkDotNet.Running;
using Perfolizer.Horology;
using System.Globalization;
using System.Threading.Tasks;
using System.IO;
using System.Security.Cryptography;

[MemoryDiagnoser]
public class Program
{
    public static void Main(string[] args) =>
        BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args,
            DefaultConfig.Instance.WithSummaryStyle(new SummaryStyle(CultureInfo.InvariantCulture,
                printUnitsInHeader: false, sizeUnit: SizeUnit.B, timeUnit: TimeUnit.Nanosecond, printZeroValuesInContent: true)));

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

    [Params(false, true)]
    public bool UseAsync { get; set; }

    [GlobalSetup]
    public void Setup()
    {
        _stream = new FileStream(Path.GetTempFileName(), FileMode.Create, FileAccess.ReadWrite, FileShare.None, 1, (UseAsync ? FileOptions.Asynchronous : FileOptions.None) | FileOptions.DeleteOnClose);
        _stream.Write(RandomNumberGenerator.GetBytes(10_000_000));
    }

    [GlobalCleanup]
    public void Cleanup()
    {
        _stream.Dispose();
    }

    [Benchmark]
    public void SyncRead()
    {
        var sfh = _stream.SafeFileHandle;
        _stream.Position = 0;
        int totalRead = 0;
        while (true)
        {
            int bytesRead = RandomAccess.Read(sfh, _buffer, totalRead);
            if (bytesRead == 0) break;
            totalRead += bytesRead;
        }
    }

    [Benchmark]
    public async Task AsyncRead()
    {
        var sfh = _stream.SafeFileHandle;
        _stream.Position = 0;
        int totalRead = 0;
        while (true)
        {
            int bytesRead = await RandomAccess.ReadAsync(sfh, _buffer, totalRead);
            if (bytesRead == 0) break;
            totalRead += bytesRead;
        }
    }
}

@stephentoub stephentoub merged commit 0696727 into dotnet:main Jul 4, 2021
@teo-tsirpanis teo-tsirpanis deleted the threadpool-valuetasksource branch July 4, 2021 23:52
@adamsitnik adamsitnik added this to the 6.0.0 milestone Jul 5, 2021
@adamsitnik adamsitnik added the tenet-performance Performance related issue label Jul 5, 2021
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teo-tsirpanis big thanks for your contribution! the allocations wins are impressive!

Now I need to update our Fille IO improvements blogs post (which has not been published yet) ;)

stephentoub added a commit to stephentoub/runtime that referenced this pull request Jul 6, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO tenet-performance Performance related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants