Skip to content

Conversation

hez2010
Copy link
Contributor

@hez2010 hez2010 commented Jun 21, 2024

I believe there's no reason to keep FileStream non-async by default after we rewrote FileStream since .NET 7.
Flipping the switch to true can benefit all APIs like File.Open, File.OpenWrite, File.Create and etc.

@ghost ghost added the area-System.IO label Jun 21, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 21, 2024
@neon-sunset
Copy link
Contributor

Please note that this may degrade single-threaded IO performance on Windows (and I don't think it does anything on Unix?). For some reason, going through Overlapped IO performs much worse.

Further discussion: https://github.com/Cysharp/Utf8StreamReader?tab=readme-ov-file#optimizing-filestream

@stephentoub
Copy link
Member

stephentoub commented Jun 21, 2024

I believe there's no reason to keep FileStream non-async by default

Making this async will cause all synchronous usage to be sync-over-async on Windows. We should not do that.

Thanks for the thought, though.

@hez2010
Copy link
Contributor Author

hez2010 commented Jun 21, 2024

Can we instead add overloads which have a FileOptions parameter to those APIs?
It's really convenient to use File.OpenWrite instead of instantiating a FileStream manually.

@stephentoub
Copy link
Member

Can we instead add overloads which have a FileOptions parameter to those APIs? It's really convenient to use File.OpenWrite instead of instantiating a FileStream manually.

#24698

@hez2010
Copy link
Contributor Author

hez2010 commented Jun 22, 2024

Can we instead add overloads which have a FileOptions parameter to those APIs? It's really convenient to use File.OpenWrite instead of instantiating a FileStream manually.

#24698

That one didn't cover File.OpenWrite and File.OpenRead.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants