Skip to content

Fixes for parallel lzma usage - #1402

Merged
adamhathcock merged 4 commits into
masterfrom
adam/parallel-lzma-fixes
Aug 4, 2026
Merged

Fixes for parallel lzma usage#1402
adamhathcock merged 4 commits into
masterfrom
adam/parallel-lzma-fixes

Conversation

@adamhathcock

Copy link
Copy Markdown
Owner

Furthering #1399

This pull request introduces significant improvements to the parallel LZMA2 decoding path in the SevenZip implementation, focusing on safer handling of multipart archives, memory efficiency, and stricter stream usage patterns. It also adds new tests to ensure correct fallback behavior when parallel decoding is not possible and refactors the test setup for clarity.

Parallel LZMA2 Decoding Improvements

  • Added a new method, TryGetPositionallyEquivalentInputFile, to ensure parallel decoding only uses positional I/O when the logical stream maps directly to a single file, preventing incorrect behavior with multipart or wrapped streams. [1] [2]
  • Refactored Lzma2ParallelDecoder.DecodeBlocksParallel to use a new RandomAccessBlockStream for reading compressed blocks, improving memory efficiency by using buffer pools and avoiding allocating large arrays per block. [1] [2]
  • Added buffer size constants and switched worker buffer management to pooled arrays, reducing allocations and improving performance. [1] [2]

Stream Usage and Safety

  • Introduced strict enforcement in LzmaStream to prevent mixing synchronous and asynchronous reads, throwing an exception if both are attempted on the same stream instance. [1] [2] [3] [4] [5] [6]

Testing and Validation

  • Added a new test to verify that parallel decoding is not used for multipart (split) archives, ensuring the decoder safely falls back to sequential mode. Refactored test setup for reusability and clarity. [1] [2]

Minor Improvements

  • Improved stream length validation and error handling in async read paths.
  • Cleaned up legacy code and clarified comments for maintainability.

These changes collectively make the parallel decoding path safer, more efficient, and more robust, especially when dealing with complex archive layouts or high-concurrency workloads.

Copilot AI lite review requested due to automatic review settings August 3, 2026 14:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens SharpCompress’s SevenZip parallel LZMA2 decode path by making positional I/O eligibility stricter (avoiding multipart/concatenated stream hazards), reducing per-block allocations via pooled buffering, and enforcing clearer stream-usage rules in LzmaStream (no mixing sync/async reads). It also expands tests to cover fallback behavior and buffer-return correctness.

Changes:

  • Refactors Lzma2ParallelDecoder.DecodeBlocksParallel to stream packed bytes via a RandomAccessBlockStream and use ArrayPool<byte> for bounded per-worker buffers.
  • Adds “read mode” enforcement to LzmaStream to prevent mixing synchronous and asynchronous reads (with new tests).
  • Updates SevenZip parallelism tests, including multipart fallback, and tweaks async length validation logic.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/SharpCompress.Test/Streams/LzmaStreamTests.cs Extends parallel decoder tests to validate pooled buffer usage and buffer return behavior on failures/edge cases.
tests/SharpCompress.Test/Streams/LzmaStreamAsyncTests.cs Adds async tests for the new “no sync/async mixing” enforcement and introduces a reusable raw LZMA stream helper.
tests/SharpCompress.Test/SevenZip/SevenZipArchiveTests.cs Refactors archive creation helper and adds a multipart test ensuring parallel decode safely falls back to sequential.
src/SharpCompress/Compressors/LZMA/LzmaStream.cs Introduces sync/async read-mode tracking and enforces it for sync reads and ReadByte().
src/SharpCompress/Compressors/LZMA/LzmaStream.Async.cs Enforces async read-mode selection in both async read overloads (byte[] and Memory-based).
src/SharpCompress/Compressors/LZMA/Lzma2ParallelDecoder.cs Switches parallel decode to random-access streaming + pooled buffers and adds a bounded buffered stream implementation for packed data.
src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs Adjusts async “decoded stream longer than expected” validation to avoid sync reads in async flow.
src/SharpCompress/Common/SevenZip/ArchiveDatabase.cs Tightens positional-file eligibility checks via TryGetPositionallyEquivalentInputFile and pre-sizes the parallel decode temp output file.

Comment thread src/SharpCompress/Common/SevenZip/ArchiveDatabase.cs
Comment thread src/SharpCompress/Common/SevenZip/ArchiveReader.Async.cs
Copilot AI review requested due to automatic review settings August 4, 2026 08:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

Copilot AI review requested due to automatic review settings August 4, 2026 08:49
@adamhathcock
adamhathcock enabled auto-merge August 4, 2026 08:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 4, 2026 09:16
@adamhathcock
adamhathcock merged commit 26c1797 into master Aug 4, 2026
7 checks passed
@adamhathcock
adamhathcock deleted the adam/parallel-lzma-fixes branch August 4, 2026 09:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/SharpCompress/Common/Tar/TarReadOnlySubStream.cs:34

  • On LEGACY_DOTNET targets, TarReadOnlySubStream has no DisposeAsync override, so callers disposing it after async-only stream usage (streams that throw on synchronous Read/CopyTo) can now fail because Dispose(bool) always uses the synchronous AdvanceToNextHeader/Skip path. This regresses the previous behavior that used AdvanceToNextHeaderAsync().GetResult() on legacy frameworks.
        _isDisposed = true;
        if (disposing)
        {
            AdvanceToNextHeader();
        }
        base.Dispose(disposing);

src/SharpCompress/Common/EntryStream.cs:34

  • On LEGACY_DOTNET, Dispose(bool) now calls the generated synchronous SkipEntry(), which ultimately performs synchronous reads (CopyTo) and can throw for async-only streams (a scenario already handled elsewhere, e.g., WinzipAesCryptoStream). Consider restoring the legacy behavior of draining the remaining entry via SkipEntryAsync().GetResult() when DisposeAsync cannot be used.
        _isDisposed = true;
        if (!(_completed || _reader.Cancelled))
        {
            SkipEntry();
        }

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants