Skip to content

perf(graphics): optionally decode large progressive regions in parallel - #2036

Draft
Willie Abrams (willie) wants to merge 8 commits into
Devolutions:masterfrom
willie:perf/progressive-rlgr-upstream
Draft

perf(graphics): optionally decode large progressive regions in parallel#2036
Willie Abrams (willie) wants to merge 8 commits into
Devolutions:masterfrom
willie:perf/progressive-rlgr-upstream

Conversation

@willie

@willie Willie Abrams (willie) commented Sep 28, 2026 •
edited
Loading

Copy link
Copy Markdown

With the new opt-in rayon feature, a Progressive region of 8 or more distinct, in-bounds tiles decodes its tiles on rayon's global pool. Each tile block touches only its own TileState and sub-band-diffing reference, so these are taken out, decoded in parallel, and put back, and the decoded tiles come back in block order. Smaller regions, regions that name a tile twice, and builds without the feature decode one tile at a time as before.

A full-screen 1920x1080 update takes 10.0 ms to decode instead of 29.2 ms with the new RLGR reader (mean, release build, Apple M4 Pro).

The feature is off by default because ironrdp-graphics is a core-tier crate, and ARCHITECTURE.md allows no non-essential dependency there. cargo xtask check tests also runs the crate's tests with the feature, so CI covers the parallel path.

On an error, both paths return the first failing block's error, but they leave different state. The sequential path stops at that block. The parallel path has already decoded the region's other tiles, so their state and references have advanced.

This builds on #2038 and will be rebased once that merges; until then, its diff also contains the reader commits.

@willie @claude
perf(graphics): read RLGR bits through a 64-bit window
0185f27 
RLGR decoding read bits through bitvec's BitSlice, which made it about
three quarters of a progressive tile's decode time. A small MSB-first
reader over the tile's bytes, with leading_zeros/leading_ones for runs,
decodes identically (compared against the old decoder on random tiles in
both RLGR modes) and takes a full-screen 1080p update from 35.8 to 29.2 ms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@willie @claude
perf(graphics): decode a large progressive region's tiles in parallel
24cd5c4 
A region's tile blocks each touch only their own tile state and
sub-band-diffing reference, so a region of at least 8 distinct, in-bounds
tiles is decoded on rayon's pool: each tile's state and reference are taken
out, decoded, and put back, and results come back in block order with the
first error in block order. Smaller regions (hover changes, cursor-sized
updates) and regions that repeat a tile decode in order as before. After an
error the parallel path still decodes the region's other tiles.

rayon is a default feature of ironrdp-graphics. A full-screen 1080p update
drops from 29.2 to 10.0 ms, and full-screen 3D reaches a client at 60
frames a second instead of 27.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@willie @claude
test(graphics): cover the RLGR bit reader
0828b3d 
Check unaligned reads, reads across the 64-bit window, refused reads
past the end, and zero and one runs that reach the end of the data.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@willie @claude
fix(graphics): leave no tile state for an unreferenced difference tile
2ab50eb 
Before the tile decode was split out, a difference tile without a
sub-band reference failed before its tile state was created. Both
region decode paths now drop a tile state they created for such a
tile, so the surface matches what it was before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@willie @claude
test(graphics): compare region-wide and per-tile progressive decoding
d61c641 
A 12-tile region, first as original then as a mix of difference and
original tiles, must produce the same pixels, tile states and sub-band
references whether decoded as one region (the parallel path with the
rayon feature) or one tile at a time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@willie @claude
build(graphics): make the rayon feature opt-in
b68a877 
ironrdp-graphics is a core-tier crate, where ARCHITECTURE.md allows no
non-essential dependency. Without the feature, regions decode one tile
at a time as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 28, 2026 15:10
@willie

Willie Abrams (willie) commented Sep 28, 2026

Copy link
Copy Markdown
Author

Measurements: release builds on an Apple M4 Pro (10 performance + 4 efficiency cores) with the rayon feature enabled. Each figure is the mean engine processing time per Progressive update over 15 s of full-screen glmark2, received from GNOME Remote Desktop at 1920x1080.

ms per update
master 35.8
64-bit RLGR reader 29.2
+ parallel tiles 10.0

Verification:

  • The new RLGR decoder's output was compared with the BitSlice decoder's on 6 million random well-formed tiles in both RLGR1 and RLGR3 modes, with identical results. That comparison did not cover truncation at every offset, and the harness is not included because it needs the old decoder as an oracle. New unit tests cover the bit reader: unaligned reads, reads across the 64-bit window, refused reads past the end, and zero and one runs that reach the end of the data.
  • A new test decodes a 12-tile region (original tiles, then a mix of difference and original tiles) both as one region and one tile at a time. It checks that pixels, tile states and sub-band references are identical. With rayon enabled, the region-wide decode takes the parallel path. Breaking the parallel path's reference hand-back makes the test fail.
  • The sequential path keeps master's error behaviour. TileOutOfBounds is still checked before MissingTileReference, and a difference tile without a reference leaves no tile state behind; a test covers this on both paths.
  • cargo xtask check fmt, lints, tests and locks pass. ironrdp-graphics also passes clippy and its tests with --features rayon, and ironrdp-testsuite-core passes with ironrdp-graphics/rayon enabled.

Open question: would you accept rayon as an opt-in feature of ironrdp-graphics at all? The alternative is to leave the parallel path out of this crate and let the caller parallelize, for example by exposing per-tile decoding. With the feature off by default, no workspace crate enables it, so the workspace-wide xtask lint and test runs don't cover the parallel path.

Note

LLM-assisted content (no human feedback).

Copilot AI reviewed Sep 28, 2026
View reviewed changes

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 review overview

🟡 Changes recommended

CI does not execute the new feature-gated parallel path or its documented mixed-result behavior.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Optimizes RemoteFX Progressive decoding through faster bit reads and optional tile-level parallelism.

Changes:

  • Replaces bit-slice decoding with a 64-bit bit reader.
  • Adds optional Rayon-based parallel tile decoding.
  • Adds decoder equivalence and edge-case tests.
File Description
crates/​ironrdp-graphics/​src/​rlgr.rs Optimizes RLGR bit reading.
crates/​ironrdp-graphics/​src/​progressive.rs Adds parallel region decoding and tests.
crates/​ironrdp-graphics/​Cargo.toml Adds the optional Rayon feature.
Cargo.lock Records the Rayon dependency.

Comment thread crates/ironrdp-graphics/src/progressive.rs
@github-actions github-actions Bot added kind/protocol Affects RDP or related protocol behavior risk/medium Behavioral change that does not substantially alter a core public API scope/core Touches the core architectural tier size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure triage/overlap Possible overlap with another pull request; advisory only labels Sep 28, 2026
@github-actions

github-actions Bot commented Sep 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This pull request may overlap with #2010.

Both PRs modify RFX Progressive decoding in crates/ironrdp-graphics/src/progressive.rs: this PR restructures region tile-block decoding for optional parallelism, while #2010 fixes SRL/upgrade-pass decoding of Windows-encoded progressive streams. Same crate and codec decode path, so a human should assess scope overlap and possible conflicts.

This notice is advisory only. Automated review continues as usual, and how these pull requests relate is for maintainers and authors to decide.

Note

LLM-assisted content (no human feedback).

@willie @claude
test(graphics): cover a region with failing tiles
b454b8b 
Decode a 12-tile region in which tile 3 lacks a reference and tile 7
has a truncated payload. Both paths return tile 3's error. The
sequential path stops there; the parallel path still decodes the other
tiles, advancing their state and references. The test asserts the
tile states and references each path leaves behind.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@willie @claude
ci(xtask): test ironrdp-graphics with rayon
e4a57ab 
The parallel tile decoding path only builds with the rayon feature,
which no workspace crate enables, so the workspace test run never
executed it. Add a feature-enabled run of the crate's unit tests to
`check tests`, next to the existing per-crate feature runs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@willie
Willie Abrams (willie) marked this pull request as draft September 28, 2026 15:39
@github-actions github-actions Bot added the scope/tooling Build, CI, release, or developer tooling label Sep 28, 2026
@willie Willie Abrams (willie) changed the title perf(graphics): speed up RemoteFX Progressive decoding perf(graphics): optionally decode large progressive regions in parallel Sep 28, 2026
@github-actions github-actions Bot mentioned this pull request Sep 29, 2026
Open

This branch was successfully deployed

1 active deployment
llm-providers — e4a57ab1 Deployed Sep 28, 2026 by willie via Classify pull request #965
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

kind/protocol Affects RDP or related protocol behavior risk/medium Behavioral change that does not substantially alter a core public API scope/core Touches the core architectural tier scope/tooling Build, CI, release, or developer tooling size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure triage/overlap Possible overlap with another pull request; advisory only

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@willie