Configure FileStreamOptions with PreallocationSize and async access in ZipArchiveEntry extract methods#125260
Configure FileStreamOptions with PreallocationSize and async access in ZipArchiveEntry extract methods#125260rzikm wants to merge 2 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @karelz, @dotnet/area-system-io-compression |
There was a problem hiding this comment.
Pull request overview
Improves ZipArchiveEntry extraction performance/behavior by configuring FileStreamOptions to (1) preallocate disk space based on the entry’s uncompressed size and (2) enable async file I/O when extracting via async APIs.
Changes:
- Set
FileStreamOptions.PreallocationSizefromZipArchiveEntry.Lengthduring extraction initialization. - Thread an
asyncflag into the shared initialization helper to setFileOptions.AsynchronousforExtractToFileAsync.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs | Adds preallocation and async/sync option selection to the shared file-creation options. |
| src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.Async.cs | Passes the async flag so extracted file streams use async I/O. |
You can also share your feedback on Copilot code review. Take the survey.
...O.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs
Outdated
Show resolved
Hide resolved
...O.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs
Outdated
Show resolved
Hide resolved
...O.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs
Outdated
Show resolved
Hide resolved
…n ZipArchiveEntry extract methods Set PreallocationSize based on the entry's uncompressed size to allow the file system to pre-allocate disk space, reducing fragmentation and improving write performance. Configure FileOptions.Asynchronous when extracting from async methods so that the FileStream uses async I/O. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1d2f2aa to
3c51c56
Compare
ZipArchiveEntry.Length throws InvalidOperationException when the entry has been opened for writing (e.g. in Update mode). Read it in a try/catch and fall back to 0 (no preallocation) to avoid regressing that scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
| { | ||
| preallocationSize = source.Length; | ||
| } | ||
| catch (InvalidOperationException) { } |
There was a problem hiding this comment.
The empty catch (InvalidOperationException) { } makes it unclear why ZipArchiveEntry.Length is expected to fail here and why it’s safe to ignore. Please add a short comment explaining the specific scenario(s) (e.g., entry not readable / length not available) and that preallocation is best-effort, so future changes don’t accidentally rely on preallocationSize being accurate.
| catch (InvalidOperationException) { } | |
| catch (InvalidOperationException) | |
| { | |
| // Length can throw if the entry is not readable or its size is not yet available (for example, | |
| // when the archive is in a mode that does not expose the entry length). In that case we skip | |
| // preallocation and leave preallocationSize as 0. This is a best-effort optimization only and | |
| // callers must not rely on PreallocationSize matching the final file size. | |
| } |
...O.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs
Show resolved
Hide resolved
| long preallocationSize = 0; | ||
| try | ||
| { | ||
| preallocationSize = source.Length; |
There was a problem hiding this comment.
Is the try/catch because the stream may not be seekable? It should instead check CanSeek
| Mode = overwrite ? FileMode.Create : FileMode.CreateNew, | ||
| Share = FileShare.None, | ||
| BufferSize = ZipFile.FileStreamBufferSize, | ||
| PreallocationSize = preallocationSize, |
There was a problem hiding this comment.
Did you measure any impact from this?
Set
PreallocationSizeonFileStreamOptionsbased on the entry's uncompressed size (ZipArchiveEntry.Length) to allow the file system to pre-allocate disk space, reducing fragmentation and improving write performance.Configure
FileOptions.Asynchronouswhen extracting from async methods (ExtractToFileAsync) so theFileStreamuses async I/O.