-
Notifications
You must be signed in to change notification settings - Fork 272
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
FileStream benchmarks #1633
FileStream benchmarks #1633
Conversation
…ire file content was read
src/benchmarks/micro/libraries/System.IO.FileSystem/Perf.FileStream.cs
Outdated
Show resolved
Hide resolved
src/benchmarks/micro/libraries/System.IO.FileSystem/Perf.FileStream.cs
Outdated
Show resolved
Hide resolved
|
||
[Params(DefaultBufferSize / 8, 200000)] | ||
public int BufferSize; | ||
private const int OneKiloByte = 1_000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be interesting to review "real world" values passed to BufferSize
https://grep.app/search?q=new%20FileStream.%2A0x®exp=true&filter[lang][0]=C%23
https://grep.app/search?q=new%20FileStream.%2A%5Cd%7B4%2C%7D®exp=true&filter[lang][0]=C%23
https://grep.app/search?q=new%20FileStream.%2Asize%3A%5Cs%3F.%5B%5E4%5D%5Cd%7B3%2C%7D®exp=true&filter[lang][0]=C%23
0x1000 / 4096 dominate (redundantly)
I also see 0x1_000 , 2000, 1024, 512 << 10, 64 * 1024
using (FileStream reader = new FileStream(_filePath, FileMode.Open, FileAccess.Read, FileShare.Read, DefaultBufferSize, FileOptions.Asynchronous)) | ||
byte[] userBuffer = _userBuffers[userBufferSize]; | ||
long bytesRead = 0; | ||
using (FileStream fileStream = new FileStream(_sourceFilePaths[fileSize], FileMode.Open, FileAccess.Read, FileShare.Read, FourKiloBytes, options)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also end up measuring the open/close time. That's ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like this benchmark to answer the question "How long does it take to read X bytes from a file?". Since opening (and closing) the file is mandatory to read it, I think that it's OK to include it here.
src/benchmarks/micro/libraries/System.IO.FileSystem/Perf.FileStream.cs
Outdated
Show resolved
Hide resolved
src/benchmarks/micro/libraries/System.IO.FileSystem/Perf.FileStream.cs
Outdated
Show resolved
Hide resolved
src/benchmarks/micro/libraries/System.IO.FileSystem/Perf.FileStream.cs
Outdated
Show resolved
Hide resolved
src/benchmarks/micro/libraries/System.IO.FileSystem/Perf.FileStream.cs
Outdated
Show resolved
Hide resolved
…WriteAsync that accepts a Memory
…s already test CancellationTokens
@jozkee @carlossanlop the CI failures are unrelated, PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should measure Flush
as well?
That aside, looks great to me!
Main changes:
Stream.Null
)cc @stephentoub