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

Improved access to files to use true asynchronous methods #526

Merged
merged 1 commit into from Aug 5, 2018

Conversation

Projects
None yet
3 participants
@xMarkos
Copy link
Contributor

xMarkos commented Aug 3, 2018

@Meir017 Meir017 added the enhancement label Aug 3, 2018

@kblok

This comment has been minimized.

Copy link
Owner

kblok commented Aug 3, 2018

Conflicted @xMarkos

@xMarkos xMarkos force-pushed the xMarkos:improvements/async-files branch from e66c495 to 7168795 Aug 3, 2018

@@ -61,7 +61,7 @@ public async Task ScreenshotAsync(string file, ScreenshotOptions options)

var data = await ScreenshotDataAsync(options).ConfigureAwait(false);

using (var fs = new FileStream(file, FileMode.Create, FileAccess.Write))
using (var fs = new FileStream(file, FileMode.Create, FileAccess.Write, FileShare.Read, 4096, true))

This comment has been minimized.

@kblok

kblok Aug 4, 2018

Owner

Can we move all the 4096 to a constant?
It’s a shame that it’s an internal constant I the FileStream class.
I can buy an internal constant in Frame or Page

@xMarkos xMarkos force-pushed the xMarkos:improvements/async-files branch 2 times, most recently from 102f20b to 0d51eb2 Aug 5, 2018

@xMarkos

This comment has been minimized.

Copy link
Contributor Author

xMarkos commented Aug 5, 2018

@kblok I have moved the logic to a new helper class "AsyncFile" so the default parameters are hidden there as well. Check the new commit whether it suits you or you think it needs more work.

@kblok

This comment has been minimized.

Copy link
Owner

kblok commented Aug 5, 2018

I love it man, great job.
Can we call it AsyncFileHelper?
I’m good after that.

Improved access to files to use true asynchronous methods
- if file is not opened with parameter "useAsync: true", all calls to async method are in fact just wrappers to the synchronous methods, which might hurt performance
- you can read more info on this topic in article https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/async/using-async-for-file-access

@xMarkos xMarkos force-pushed the xMarkos:improvements/async-files branch from 0d51eb2 to 56ffbaa Aug 5, 2018

@xMarkos

This comment has been minimized.

Copy link
Contributor Author

xMarkos commented Aug 5, 2018

@kblok Done

@kblok

kblok approved these changes Aug 5, 2018

@kblok kblok merged commit 31ad5b0 into kblok:master Aug 5, 2018

2 checks passed

CodeFactor No issues found.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@xMarkos xMarkos deleted the xMarkos:improvements/async-files branch Aug 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment