Repository navigation
perf(j2k-native): decode region bands on the pool ahead of the streamed IDWT - #310
Conversation
…ed IDWT A parallel region decode on the row-streamed IDWT decoded every code-block of a component on the tile route, then ran the IDWT. Now the route's code-block rows are bands that pool workers decode ahead of the IDWT, which runs on the calling thread, so Tier-1 and the IDWT overlap (stage 3b of frames-sg#133). - Each sub-band holds a ring of band slots: three at the finest level, two at the others. A sub-band whose bands fit in fewer rows holds its whole window instead, each band in its own rows, so its bands can all be in flight at once. - Workers take the band the IDWT reads first, from one queue, and return to the pool when none can start. The IDWT waits only for a band a worker is decoding, and decodes one nobody has started itself, so it never depends on a free pool thread. - Every band decodes once, including bands the IDWT never reads. A failure, in planning or in any band, reruns whole-window Tier-1 serially for its error. - A wait that finds a band running with no worker active and no error gives up the pool, and the component decodes over whole windows; debug builds assert. - Tiles the route doesn't decode, or that it would decode serially or per sub-band, keep their current paths. A 1024x1024 region of an 18000x12000 RGB image decodes 3 to 11% faster at 12 threads and 9 to 18% faster at 4, with about 3 MiB less peak heap. Output is unchanged.
|
Awesome, thanks for getting this up! I ran the original native suite and it passed, 834 tests with 5 ignored. I found two things while checking the budget and some less usual region shapes. The first is the memory accounting. The other is Could you take those two here? My CPU optimization branch already includes this PR, so once you've fixed them here, I'll apply those fixes there too and rerun the tests. I put the two reproducers in this test-only commit on Fetch the tests and run the reproducersFrom your PR branch, fetch and cherry-pick the test commit: git fetch https://github.com/frames-sg/j2k.git review/pr310-repro-tests
git cherry-pick FETCH_HEADRun the budget regression: cargo test -p j2k-native --lib pool_bands_respect_the_combined_plan_and_executor_budget --locked -- --nocaptureLocally, it reports 276182 bytes of headroom, a 269496-byte route and a 13373-byte executor: the combined charges exceed the available budget by 6687 bytes, but the route returns Run the timing report: env -u J2K_PROFILE_STAGES cargo test -p j2k-native --profile gpu-quick --lib pool_bands_tall_region_timing_report --locked -- --ignored --nocapture --test-threads=1
|
…ssions Add an aggregate-budget regression for PR frames-sg#310 and an opt-in timing report for tall HT regions. The budget test intentionally fails on the current PR; the timing report checks pixels and prints interleaved medians without asserting wall time.
…eap bands on the route - The executor's records are charged to the budget that planning charged the route's plan, payload arena and clipped-block scratch to, since those stay live while the bands decode. A fresh budget for the records let a decode through when the combined charges were over the cap. - Freeing a band advances a per-ring cursor instead of walking the ring from its start, and the drain frees the bands that finished from a list, with running and freed counts, instead of scanning every band. On a 32x65536 region of 16x4 HT blocks at 4 threads the pool took 182 ms; it takes 49 ms. - A component whose bands average less than 1024 of the route's work units (payload bytes plus an eighth of the samples) decodes over whole windows on the route: scheduling bands that cheap one by one costs more than overlapping Tier-1 with the IDWT saves. That 32x65536 region averages 88 and now decodes as fast as whole windows; 1024 px regions of the benchmark images average 2,000 to 15,000 and are unchanged.
|
Thanks for the careful review and the reproducers! Both are fixed in Budget. Scheduling.
That brought 32×65536 from 182 to 49 ms, about what you saw with the cursor, and the ratio stopped growing with height (about 3.3× to 3.6× at every size). What's left is per-band overhead: about 32,000 bands of one 16×4 block each, and roughly a microsecond of lock, queue and wake-up work per band, more than decoding the block. So, as you suggested, cheap bands now stay on the existing route.
As with Your timing report on my machine (12-core M4 Pro, four threads), median of 7:
The benchmark regions still take the pool and keep their gains. At 12 threads the PLT 1024² middle region is 33.1 → 29.4 ms against main, with identical hashes and the same 3 MiB lower peak as before.
|
Whole-window recovery starts from the storage budget, which excludes the parallel route plan and payload arena. Release the route after recording its statistics and before either fallback allocates replacement storage.
|
Merged, thanks for the fixes and benchmark results! I added one small follow-up to release the parallel plan before the whole-window fallback allocates its buffers. The focused tests and CI passed, including CUDA and Metal validation. I’ve also brought this into my CPU optimization branch. I’ll follow up on #311 with the BYPASS integration and combined ARM/x86 results. Thanks again! |
Stage 3b from #133: the parallel lookahead executor for banded Tier-1, with the shared queue we discussed on #287.
Change
Today a parallel region decode on the row-streamed IDWT decodes every code-block of a component on the tile route, then runs the IDWT. Here the route's code-block rows become bands that pool workers decode ahead of the IDWT, which runs on the calling thread, so Tier-1 and the IDWT overlap.
plan_band_route(insubband/tile_parallel.rs) plans one component the wayplan_tile_routedoes, with the same coding checks, cutoffs and workspace bank, plus one workspace for the calling thread. Strips are addressed within each sub-band's stored rectangle rather than at whole-window offsets.BandRouteexposes the strips and a per-strip decode for classic and HT.storage.parallel_bands,lay_out_region_coefficientsgives each sub-band a ring of band slots: 3 at the finest active level and 2 at the others (FINEST_LEVEL_BAND_SLOTS,OTHER_LEVEL_BAND_SLOTS). A ring never exceeds the sub-band's window. When its bands fit in fewer rows than the slots would take, the ring is the whole window, and each band decodes into its own rows. That matters more than I expected: in my first version such sub-bands ended up with a single slot, so the IDWT decoded those bands itself, and 1024² regions were 7–23% slower than main and a 512² region 70% slower.decode/parallel_bands.rs). OneMutexholds each band's status (waiting, running, done, freed), the free slots and the ready queue. The queue is ordered by the row of the reconstructed component at which the IDWT first reads the band, then largest first. A band becomes ready once the band a ring's length before it has been freed, which happens when the IDWT has read all of it into its own rings. Workers run underrayon::in_place_scopeand return to the pool when nothing can start, and the IDWT sends them again when it frees slots, so pool threads never wait on our condvar. The IDWT waits only for a band in flight, and decodes a band nobody has started itself. So it doesn't depend on a free pool thread, even when the pool is busy with other work or the decode runs inside the pool.banded_tier1::whole_window_error), so the error is the one the serial path reports.decode_componentssends banded components, serial or on the pool, through a smalldecode_component_bandshelper.A hang the fuzz found, and the guard
The first long differential fuzz of an earlier build hung once, after about 231,000 parallel decodes. The calling thread was waiting in the drain phase on a band marked running, while every pool thread was idle. There was no panic, and replaying that iteration 300 times didn't reproduce it.
I couldn't find the path that leaves a band running without a worker. Every place that starts a band either finishes it or returns an error, and a test that makes the IDWT thread hit corrupt bands with the pool's other thread held busy passes. The only behavioural differences between that build and this one are in
drainand insend_workers, which no longer spawns while holding the lock.Since then this code has run about 2.7 million parallel fuzz decodes without stranding a band, including runs at the original load of 12 concurrent processes on 12 cores. At the old rate, a clean run that long would happen with probability under 10⁻⁵. Still, because I can't point to the cause, waits now guard against it.
pool_band_fallbackscounts it, and debug builds assert.a_stranded_band_falls_back_to_whole_windowschecks that the decode finishes with the right pixels.The guard checks the error first, because a band whose decode failed stays running by design and its error ends the wait.
Results
Parallel (
Auto) component-plane region decodes on a 12-core M4 Pro, against main (0c6b6e47). The figures are interleaved medians of 11 runs at 12 threads and 9 at 4. Peak heap comes from a counting allocator. Output hashes are identical in every case.The machine had other load during some runs, so a few percent either way is noise. The peak saving is the coefficients, about 3 MiB on 1024²; the 12 MiB of planes remain, since the row sink is serial only.
Before building this, I simulated the scheduler from traced strip timings, and it predicted about 18 ms instead of about 28 ms for Tier-1 and the IDWT on the PLT 1024² region. The real figure for those two stages is about 30 → 24 ms (medians of 5 profiled runs: 30.3 and 23.6 ms). I think most of the gap is that components still decode one after another: each starts cold, since its first output row needs the first band of every level, and ends with a drain where few bands are left for the workers. Letting the next component's bands start while the current one drains would need every component's rings live at once (Tile layout, a few MiB more). That's a natural follow-up, and it's also what a parallel row sink would need.
Not covered
Tests
tests/parallel_bands.rs:pool_bands_match_whole_window_tier1_bit_for_bitcompares against whole-window Tier-1 over the stage 3 cases, 8 regions and 3 reductions, on pools of 2, 4 and 12 threads, with the same decoded and skipped counts. It checks that every routed case reaches the pool on 2 threads, that multi-layer classic never does, and that the coefficient peak is lower on the large region.pool_bands_finish_while_the_other_pool_thread_is_busy: the IDWT decodes every band itself.concurrent_pool_band_decodes_inside_the_pool_finish: 12 decodes inside 2- and 3-thread pools.pool_bands_reject_corrupt_code_blocks_as_serial_whole_window_tier1_does: 1,200 mutants, classic and HT, give the same outcome as the serial whole-window path.a_stranded_band_falls_back_to_whole_windows, described above.window_clipped_region_storage_matches_whole_code_blocks_bit_for_bitis unchanged; it found that rings could exceed the window, which is why they are now capped.decode_region_budgets_coefficients_for_the_requested_regionnow checks the whole-window budget withwhole_window_tier1. It also checks that banded storage, serial or on the pool, gets the 9000² region as far as the packets, because banded coefficients no longer grow with the region.cargo xtask test(5,462 passed),clippy,clippy-strict,repo-lintanddocpass, and clippy is clean without default features. GPU validation hasn't been run.