perf(codegen): licm.rs FxHashMap/FxHashSet swap (+87.9% on inner check) - #235
Closed
TheTom wants to merge 1 commit into
Closed
perf(codegen): licm.rs FxHashMap/FxHashSet swap (+87.9% on inner check)#235TheTom wants to merge 1 commit into
TheTom wants to merge 1 commit into
Conversation
Playbook §"FxHashMap wins on the codegen pipeline" — same shape as PR 0xClandestine#38 (-29.8%) and PR 0xClandestine#39 (-49.5%). The LICM hoist-fixpoint inner check `op_refs.iter().all(|v| invariant.contains(v))` is the densest membership-test loop in the codegen. BTreeSet is log(n) per probe; FxHashSet is single-cycle. ## Measured win `crates/metaltile-codegen/src/passes/licm.rs::perf::perf_licm_invariant_contains` (`#[ignore]`-gated, playbook §"Measurement infrastructure"). Replays the exact pattern: 256-element invariant set, 10K op-refs at 4 refs/op (typical kernel + fixpoint shape), 200 pass iters per variant. 80% in-set, 20% out-of-set membership-mix per the empirical LICM hit rate. ``` === LICM hoist-fixpoint membership check === BTreeSet (old) : 33.29 ms (16.64 ns/op-check) FxHashSet (new) : 4.03 ms ( 2.02 ns/op-check) speedup : 8.25× (+87.9%) ``` Run with: ``` cargo test -p metaltile-codegen --release perf_licm_invariant_contains \ -- --ignored --nocapture ``` The microbench includes a `fx_ns_per <= bt_ns_per * 1.05` regression assertion + a `bt_acc == fx_acc` hit-count equality so a future change that breaks either correctness or perf fails CI loudly. ## Diff - `licm.rs:38` drop `BTreeSet`, add `FxHashSet` to existing `rustc_hash::{FxHashMap}` import. `BTreeMap` import stays — `insert_at` at the rebuild path keeps `BTreeMap<usize, ...>` because the rebuild walks the parent block in source order and consumes `insert_at` keys in ascending iteration (BTreeMap's ordered iteration is load-bearing there — playbook §"Counter-indication: keep BTreeMap when iteration order matters"). - `licm.rs:61` `def_block: BTreeMap<ValueId, BlockId>` → `FxHashMap<ValueId, BlockId>` pre-sized with `find_max_vid(kernel)` (half the dead_store_elim win per PR 0xClandestine#38). - `licm.rs:53` `read_only: BTreeSet<String>` → `FxHashSet<String>`. Param count is small but the membership probe is inside `is_pure_op` which runs per-op per-fixpoint-iter — FxHash on String still pays the SipHash-grade hash on each probe, but matches the same-shape audit in the rest of the file. - `licm.rs:129` `invariant: BTreeSet<ValueId>` → `FxHashSet<ValueId>` — the load-bearing swap; this is what the microbench measures. - `licm.rs:267` `skip: BTreeSet<usize>` → `FxHashSet<usize>` — pure membership probe inside `remove_ops_from_block`, no ordering needed. - `licm.rs:283` `is_pure_op` signature follows the `read_only` swap. ## Iteration-order audit Every swap above is membership-only (`.contains`) or `.get` keyed lookup. None of the swapped collections has its iteration order leak into MSL emit or any downstream content-addressed artifact. Verified by re-reading every use of `def_block`, `invariant`, `skip`, and `read_only` in the file. ## Verification - `cargo test -p metaltile-codegen --lib passes::licm` — 6 passed, 1 ignored (the perf bench). All hoist-correctness tests green. - `cargo check --workspace --all-targets` clean. - `cargo clippy --workspace --all-targets --all-features -- -D warnings` clean. - `cargo fmt --all --check` clean.
Merged
4 tasks
TheTom
added a commit
to TheTom/metaltile
that referenced
this pull request
May 30, 2026
…d/fusion/register_estimate (+57.6%) Continues 0xClandestine#235 (licm) and 0xClandestine#236 (copy_prop/alg_simplify/unroll/vectorize) into the four remaining codegen passes that still used BTree containers keyed by `ValueId` (or `usize` position) for pure get/insert/contains: - **value_sink.rs** — `compute_global_use_count`'s use-count map (`BTreeMap<ValueId, usize>` → `FxHashMap`), `rebuild_with_sinking`'s `remove_at` set + `insert_before` map (`BTreeSet<usize>` / `BTreeMap<usize, …>` → Fx). Pre-sized from the outer plans/op counts. - **const_fold.rs** — DCE's `used` set + cross-block `cross_block_refs` set (`BTreeSet<ValueId>` → `FxHashSet`). `collect_uses` helper signature updated. - **fusion.rs** — `pinned_per_block` inner value (`BTreeSet<ValueId>` → `FxHashSet`); outer was already `FxHashMap`. `fuse_block` only does `.contains()` on the inner set, no iteration order needed. - **register_estimate.rs** — `block_max_live`'s `live` set (`BTreeSet<ValueId>` → `FxHashSet`), pre-sized to `block.ops.len()`. Microbench (M5 Max, release, 2000-op block, 10K iters side-by-side): BTreeSet (old): 40110.2 ns/call FxHashSet (new): 17000.2 ns/call → speedup : 2.36× (+57.6%) Pinned by `assert!(fx_ns_per * 1.05 <= bt_ns_per)`. `type_check.rs` intentionally NOT touched — overlaps with @0xClandestine's in-flight PR 0xClandestine#58 scope (introduces a new typed-value-environment model). Verified with 161/161 codegen tests passing, clippy clean, fmt clean.
0xClandestine
pushed a commit
that referenced
this pull request
May 30, 2026
…egister_estimate (+57.6%) (#239) ## Summary Continues #235 (licm) and #236 (copy_prop/alg_simplify/unroll/vectorize) into the four remaining codegen passes that still used BTree containers keyed by \`ValueId\` (or \`usize\` position) for pure get/insert/contains: | File | Change | |------|--------| | \`value_sink.rs\` | \`compute_global_use_count\`'s use-count map (\`BTreeMap<ValueId, usize>\` → \`FxHashMap\`). \`rebuild_with_sinking\`'s \`remove_at\` set + \`insert_before\` map (BTreeSet/BTreeMap of \`usize\` → Fx). All pre-sized from outer plans/op counts. | | \`const_fold.rs\` | DCE's \`used\` set + cross-block \`cross_block_refs\` (\`BTreeSet<ValueId>\` → \`FxHashSet\`). \`collect_uses\` helper signature updated. | | \`fusion.rs\` | \`pinned_per_block\` inner value (\`BTreeSet<ValueId>\` → \`FxHashSet\`); outer was already \`FxHashMap\`. \`fuse_block\` only does \`.contains()\` on the inner set — no iteration order. | | \`register_estimate.rs\` | \`block_max_live\`'s \`live\` set (\`BTreeSet<ValueId>\` → \`FxHashSet\`), pre-sized to \`block.ops.len()\`. | \`type_check.rs\` **intentionally NOT touched** — overlaps with your in-flight PR #58 scope (introduces a new typed-value-environment model). ## Measured (M5 Max, release, 2000-op block, 10K iters side-by-side) \`perf_block_max_live_btree_vs_fxhash\` — BTreeSet impl kept inline in the test module: \`\`\` === block_max_live: 2000-op live-set walk × 10000 iters === BTreeSet (old): 40110.2 ns/call FxHashSet (new): 17000.2 ns/call → speedup : 2.36× (+57.6%) \`\`\` Pinned by \`assert!(fx_ns_per * 1.05 <= bt_ns_per)\`. \`cargo test -p metaltile-codegen --release perf_block_max_live_btree_vs_fxhash -- --ignored --nocapture\` ## Test plan - [x] \`cargo test -p metaltile-codegen --lib\` — 161/161 pass - [x] \`cargo clippy --workspace --all-targets --all-features -- -D warnings\` — clean - [x] \`cargo fmt --all --check\` — clean - [x] \`cargo test -p metaltile-codegen --release perf_block_max_live_btree_vs_fxhash -- --ignored --nocapture\` — +57.6%
Collaborator
Author
|
Closing as superseded — current Nothing lost; the optimization is live on dev. |
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.
Playbook §"FxHashMap wins on the codegen pipeline". The LICM hoist-fixpoint inner check
op_refs.iter().all(|v| invariant.contains(v))is the densest membership-test loop in the codegen. BTreeSet is log(n) per probe; FxHashSet is single-cycle.Measured win
crates/metaltile-codegen/src/passes/licm.rs::perf::perf_licm_invariant_contains—#[ignore]-gated, playbook §"Measurement infrastructure". Replays the inner check shape: 256-element invariant set, 10K op-refs at 4 refs/op, 200 pass iters per variant. 80% in-set / 20% out-of-set mix matches the empirical hoist hit rate.Run:
Includes
fx_ns_per <= bt_ns_per * 1.05regression assertion +bt_acc == fx_acchit-count equality so future changes that break either correctness or perf fail CI loudly.Diff
licm.rs:53read_only: BTreeSet<String>→FxHashSet<String>licm.rs:61def_block: BTreeMap<ValueId, BlockId>→FxHashMappre-sized withfind_max_vid(kernel)(half the dead_store_elim win per PR perf(codegen): FxHashMap in fusion + pre-sized FxHash in dead_store_elim #38)licm.rs:129invariant: BTreeSet<ValueId>→FxHashSet<ValueId>— the load-bearing swap; what the microbench measureslicm.rs:267skip: BTreeSet<usize>→FxHashSet<usize>(pure membership probe insideremove_ops_from_block)licm.rs:283is_pure_opsignature follows theread_onlyswapinsert_at(rebuild map at line ~227) staysBTreeMap<usize, ...>— the parent-block rebuild walksinsert_atin ascending key order, so iteration order is load-bearing here per playbook §"Counter-indication: keep BTreeMap when iteration order matters"Iteration-order audit
Every swap is
.containsmembership-only or.getkeyed lookup. None of the swapped collections has its iteration order leak into MSL emit or any downstream content-addressed artifact. Verified by re-reading every use ofdef_block,invariant,skip,read_only.Verification
cargo test -p metaltile-codegen --lib passes::licm— 6 passed (all hoist-correctness tests green)cargo check --workspace --all-targetscleancargo clippy --workspace --all-targets --all-features -- -D warningscleancargo fmt --all --checkclean