Skip to content

Add pooling for arrays for deflate#1347

Merged
adamhathcock merged 1 commit into
masterfrom
adam/deflate-allocations
Jun 4, 2026
Merged

Add pooling for arrays for deflate#1347
adamhathcock merged 1 commit into
masterfrom
adam/deflate-allocations

Conversation

@adamhathcock

Copy link
Copy Markdown
Owner

This pull request introduces significant memory management improvements across the Deflate compression components by adopting array pooling for buffer allocations. This change aims to reduce memory allocations and garbage collection overhead, enhancing performance and resource efficiency. Key updates include replacing direct buffer allocations with pooled arrays and ensuring proper buffer return and cleanup in disposal and reset paths.

Memory management and array pooling improvements:

  • Replaced direct allocations of buffers (e.g., window, prev, head, pending in DeflateManager, hufts in InflateBlocks, and working buffers in ZlibBaseStream and CRC32) with pooled arrays from ArrayPool<T>.Shared. This change reduces memory pressure and improves performance. [1] [2] [3] [4] [5]
  • Added explicit buffer cleanup: Returned rented arrays to their respective pools (with clearing) in Dispose, DisposeAsync, Reset, and End methods, and introduced helper methods like ReturnBuffers and ReturnWorkingBuffer to centralize cleanup logic. [1] [2] [3] [4] [5] [6]
  • Updated using directives to include System.Buffers where array pooling is now used. [1] [2] [3]

Code consistency and minor refactoring:

  • Ensured all buffer-related logic is consistent across compression and decompression classes, including proper error handling and resource release in all code paths.

These changes collectively make the codebase more efficient and robust in terms of memory usage, especially during high-throughput or large-scale compression operations.

Copilot AI review requested due to automatic review settings June 2, 2026 13:57

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 reduces allocation/GC pressure in the Deflate stack by switching several frequently-allocated buffers to ArrayPool<T>.Shared and adding explicit return/cleanup paths, improving throughput and memory efficiency during compression/decompression.

Changes:

  • Rent/return large Deflate/Inflate working arrays (window, prev, head, pending, hufts) from ArrayPool<T>.Shared.
  • Pool a reusable working buffer in ZlibBaseStream and return it during dispose paths.
  • Pool the temporary copy buffer in CRC32.GetCrc32AndCopy() and ensure it is returned via finally.

Issues to address before approval:

  • ZlibCodec.EndDeflate() discards the return value from dstate.End() and always returns Z_OK, which can mask Z_STREAM_ERROR / Z_DATA_ERROR results now produced by DeflateManager.End().
  • In ZlibBaseStream.Dispose(...) / DisposeAsync(), ReturnWorkingBuffer() is called after end(). If end() throws, the working buffer will not be returned to the pool (and subsequent dispose steps may be skipped), causing resource leakage.

Reviewed changes

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

Show a summary per file
File Description
src/SharpCompress/Compressors/Deflate/ZlibCodec.cs Calls dstate.End() during EndDeflate(), but currently ignores its return value.
src/SharpCompress/Compressors/Deflate/ZlibBaseStream.cs Pools the internal working buffer and adds a helper to return it during dispose paths.
src/SharpCompress/Compressors/Deflate/Inflate.cs Pools the hufts table used by InflateBlocks and returns it in Free().
src/SharpCompress/Compressors/Deflate/DeflateManager.cs Pools core Deflate buffers and centralizes returning them in End().
src/SharpCompress/Compressors/Deflate/CRC32.cs Pools the copy buffer in GetCrc32AndCopy() and returns it in a finally block.

@adamhathcock
adamhathcock merged commit cd55ed2 into master Jun 4, 2026
7 checks passed
@adamhathcock
adamhathcock deleted the adam/deflate-allocations branch June 4, 2026 13:50
This was referenced Jul 13, 2026
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