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

Keep tracking the file offset in memory, don't perform expensive Seek calls on every Read|WriteAsync #49145

Closed
wants to merge 20 commits into from

Conversation

adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Mar 4, 2021

This PR is blocked because it contains #48813 which needs to be reviewed and merged first.

Until then if someone is interested in looking at the changes you can see the three following commits:
d06f175
91423fa
a7ca4cb

But you should most probably just ignore it until then.

Depending on the file and buffer size, ReadAsync is 10-30% faster:

Method Toolchain fileSize userBufferSize options Mean Ratio Allocated
ReadAsync \after\CoreRun.exe 1024 4096 None 64.79 us 1.00 792 B
ReadAsync \before\CoreRun.exe 1024 4096 None 65.04 us 1.00 792 B
ReadAsync \after\CoreRun.exe 1024 4096 Asynchronous 77.38 us 0.92 953 B
ReadAsync \before\CoreRun.exe 1024 4096 Asynchronous 83.77 us 1.00 952 B
ReadAsync \after\CoreRun.exe 1048576 512 None 1,613.98 us 0.99 86,723 B
ReadAsync \before\CoreRun.exe 1048576 512 None 1,625.09 us 1.00 86,721 B
ReadAsync \after\CoreRun.exe 1048576 512 Asynchronous 3,240.19 us 0.70 95,047 B
ReadAsync \before\CoreRun.exe 1048576 512 Asynchronous 4,632.78 us 1.00 95,047 B
ReadAsync \after\CoreRun.exe 1048576 4096 None 1,054.26 us 1.00 29,353 B
ReadAsync \before\CoreRun.exe 1048576 4096 None 1,051.53 us 1.00 29,353 B
ReadAsync \after\CoreRun.exe 1048576 4096 Asynchronous 3,267.70 us 0.71 80,514 B
ReadAsync \before\CoreRun.exe 1048576 4096 Asynchronous 4,617.27 us 1.00 80,514 B
ReadAsync \after\CoreRun.exe 104857600 4096 None 126,194.78 us 1.01 2,868,024 B
ReadAsync \before\CoreRun.exe 104857600 4096 None 125,496.57 us 1.00 2,868,024 B
ReadAsync \after\CoreRun.exe 104857600 4096 Asynchronous 360,386.97 us 0.77 7,988,704 B
ReadAsync \before\CoreRun.exe 104857600 4096 Asynchronous 470,759.18 us 1.00 7,991,376 B

And depending on file and buffer size WriteAsync is 0 to 70% faster:

Method Toolchain fileSize userBufferSize options Mean Ratio RatioSD Allocated
WriteAsync \after\CoreRun.exe 1024 4096 Asynchronous 188.7 us 1.01 0.02 304 B
WriteAsync \before\CoreRun.exe 1024 4096 Asynchronous 186.6 us 1.00 0.00 304 B
WriteAsync \after\CoreRun.exe 1048576 512 Asynchronous 8,384.7 us 0.53 0.01 86,833 B
WriteAsync \before\CoreRun.exe 1048576 512 Asynchronous 15,923.3 us 1.00 0.00 86,858 B
WriteAsync \after\CoreRun.exe 1048576 4096 Asynchronous 8,288.1 us 0.52 0.02 80,510 B
WriteAsync \before\CoreRun.exe 1048576 4096 Asynchronous 16,087.7 us 1.00 0.00 80,538 B
WriteAsync_NoBuffering \after\CoreRun.exe 1048576 16384 Asynchronous 26,965.8 us 0.94 0.01 20,416 B
WriteAsync_NoBuffering \before\CoreRun.exe 1048576 16384 Asynchronous 28,785.2 us 1.00 0.00 20,418 B
WriteAsync \6.0.0\CoreRun.exe 104857600 4096 Asynchronous 488,837.8 us 0.29 0.02 7,988,032 B
WriteAsync \simple\CoreRun.exe 104857600 4096 Asynchronous 1,671,306.4 us 1.00 0.00 7,988,032 B
WriteAsync_NoBuffering \6.0.0\CoreRun.exe 104857600 16384 Asynchronous 157,768.7 us 0.32 0.03 1,997,376 B
WriteAsync_NoBuffering \simple\CoreRun.exe 104857600 16384 Asynchronous 498,606.8 us 1.00 0.00 1,997,432 B

There is no perf penalty for sync reads and writes:

Method Toolchain fileSize userBufferSize options Mean Ratio Allocated
Read \after\CoreRun.exe 1024 4096 None 49.66 us 1.00 256 B
Read \before\CoreRun.exe 1024 4096 None 49.67 us 1.00 256 B
Read \after\CoreRun.exe 1048576 512 None 714.12 us 1.00 4,376 B
Read \before\CoreRun.exe 1048576 512 None 716.84 us 1.00 4,376 B
Read \after\CoreRun.exe 1048576 4096 None 656.52 us 0.99 256 B
Read \before\CoreRun.exe 1048576 4096 None 661.34 us 1.00 256 B
Read \after\CoreRun.exe 104857600 4096 None 79,517.27 us 1.01 292 B
Read \before\CoreRun.exe 104857600 4096 None 79,127.17 us 1.00 292 B

fixes #16354
fixes #25905

remove finalizer from FileStream, keep it only in DerivedFileStreamStrategy and LegacyFileStreamStrategy
… functional

they can now be used directly without any buffering on top of them!
…NothingToFlush_CompletesSynchronously passing
when users have a race condition in their code (i.e. they call
Close when calling another method on Stream like Read).
@adamsitnik adamsitnik added area-System.IO blocked Issue/PR is blocked on something - see comments tenet-performance Performance related issue labels Mar 4, 2021
@adamsitnik
Copy link
Member Author

I am closing the PR. Our plan is that @jozkee is going to pick it up and combine it with #49145 and send a new PR. In #49754 I've added few edge-case tests to help to ensure we are not breaking anything.

@adamsitnik adamsitnik closed this Mar 17, 2021
@adamsitnik adamsitnik deleted the noExtraSeekCalls branch April 7, 2021 17:22
@ghost ghost locked as resolved and limited conversation to collaborators May 7, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO blocked Issue/PR is blocked on something - see comments tenet-performance Performance related issue
Projects
None yet
2 participants