Repository navigation
fix(graphics): bound ZGFX compressor hash table size - #1344
Merged
Benoît Cortier (CBenoit) merged 1 commit intoJun 1, 2026
Merged
Benoît Cortier (CBenoit) merged 1 commit into
Benoît Cortier (CBenoit) merged 1 commit into
Conversation
The ZGFX LZ77 compressor's compact_hash_table trims per-prefix position lists but never reduces the number of distinct prefixes. Incompressible input (an already-encoded H.264 EGFX payload, for example) produces a near-unique 3-byte prefix per byte, so once the table passes MAX_HASH_TABLE_ENTRIES it stays over the cap and add_to_history re-runs compaction on every literal byte, each pass scanning the whole table while freeing nothing. That is O(n * table_size) per frame: compressing 100 KB of incompressible data took about 10 s, and the table grew unbounded with the history. Enforce the cap by evicting whole least-recently-seen prefixes down to a low watermark when the table is over MAX_HASH_TABLE_ENTRIES, so compaction runs at most once per (MAX - target) inserted prefixes and the table stays bounded. Match distance is already capped at MAX_MATCH_DISTANCE, so dropping the oldest prefixes does not change reachable matches; compression of compressible input is unaffected. Add a regression test that compresses 100 KB of deterministic high-entropy data, asserts the round trip through Decompressor, and asserts the table stays within MAX_HASH_TABLE_ENTRIES.
Contributor
There was a problem hiding this comment.
Pull request overview
Bounds the ZGFX compressor's hash table to prevent O(n·table_size) per-frame compaction on incompressible payloads (e.g., already-encoded H.264). Previously, compact_hash_table only halved per-prefix position lists without reducing prefix count, so high-entropy input kept the table above the cap and triggered compaction on every literal byte. The fix evicts whole least-recently-seen prefixes down to a low watermark (half the cap), amortizing compaction to O(1) per byte while preserving reachable matches (distance is already capped at MAX_MATCH_DISTANCE).
Changes:
- Add
COMPACT_TARGET_ENTRIESlow watermark and extendcompact_hash_tableto evict whole prefixes by recency usingselect_nth_unstable. - Document the invariant and amortization rationale.
- Add a regression test that compresses 100 KB of LCG-generated high-entropy data, verifies a Decompressor round trip, and asserts the table stays bounded.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Jun 1, 2026
David T. Martel (David-Martel)
pushed a commit
to David-Martel/IronRDP
that referenced
this pull request
Jul 4, 2026
Bounds the ZGFX compressor's hash table to prevent O(n·table_size) per-frame compaction on incompressible payloads (e.g., already-encoded H.264). Previously, `compact_hash_table` only halved per-prefix position lists without reducing prefix count, so high-entropy input kept the table above the cap and triggered compaction on every literal byte. The fix evicts whole least-recently-seen prefixes down to a low watermark (half the cap), amortizing compaction to O(1) per byte while preserving reachable matches (distance is already capped at `MAX_MATCH_DISTANCE`). (cherry picked from commit 4e11a17)
Marc-André Moreau (mamoreau-devolutions)
pushed a commit
that referenced
this pull request
Oct 9, 2026
…2001) The ZGFX compressor keeps its history in a Vec and, once the history reaches its 2.5 MB limit, add_to_history drains the front of the Vec and walks every entry of the match table to rebase and prune positions. add_to_history runs once per literal byte and once per match, so after the history fills, every emitted token costs a copy of the whole window plus a pass over the table. With ClearCodec output from a 1280x800 desktop fed through one Compressor, the first three frames took 10 to 51 ms each, and the next three took 0.38, 4.9 and 6.8 seconds. #1344 bounded the hash table, which is a different limit, and this shows up on any long session that uses CompressionMode::Auto or Always. The compressor came from my own #1097. This keeps the history as a fixed ring of HISTORY_SIZE bytes addressed by absolute stream position, the same layout the decompressor already uses with FixedCircularBuffer. Appending is a copy into the ring, the match table stores absolute positions, and a candidate is skipped when it has fallen out of the window or is further back than MAX_MATCH_DISTANCE, so nothing has to be rebased. The encoded output format is unchanged, and so is the rule in MS-RDPEGFX 3.1.9.1.2 that every output byte, including those of segments sent uncompressed, is recorded in the history. With the same input the output is the same size on every frame, and the frames after the history fills now take 9 to 15 ms each. A new test compresses more than twice HISTORY_SIZE through one Compressor, in segments that repeat earlier content, and decompresses every segment with one Decompressor, which covers matches that reach across the wrap point. A second test checks the ring directly, including a push larger than the ring. The xtask fmt, lints, tests, typos and locks checks pass. #2003 builds on this: it sends H.264 surface commands uncompressed and records their bytes in the ring.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The ZGFX LZ77 compressor's compact_hash_table trims per-prefix position lists but never reduces the number of distinct prefixes. Incompressible input (an already-encoded H.264 EGFX payload, for example) produces a near-unique 3-byte prefix per byte, so once the table passes MAX_HASH_TABLE_ENTRIES it stays over the cap and add_to_history re-runs compaction on every literal byte, each pass scanning the whole table while freeing nothing. That is O(n * table_size) per frame: compressing 100 KB of incompressible data took about 10 s, and the table grew unbounded with the history. This is reachable from ironrdp-egfx server output when CompressionMode is Auto or Always and the payload is already compressed.
The fix evicts whole least-recently-seen prefixes down to a low watermark when the table exceeds the cap, so compaction runs at most once per (MAX - target) inserted prefixes and the table stays bounded. Match distance is already capped at MAX_MATCH_DISTANCE, so dropping the oldest prefixes does not change reachable matches; compressible input is unaffected.
Validation
cargo xtask check fmt/lints/tests/typos/locks all pass. The new regression test compresses 100 KB of deterministic high-entropy data, asserts the round trip through Decompressor, and asserts the table stays within MAX_HASH_TABLE_ENTRIES (about 10 s to a few ms).
Notes
No public API change; compact_hash_table and the new constant are private. Output remains valid ZGFX (round trip verified); only the worst-case compaction cost and peak table size change.