Skip to content

refactor: reduce tracked query monomorphization - #1150

Draft
MichaReiser wants to merge 8 commits into
salsa-rs:masterfrom
MichaReiser:erase-query-configurations
Draft

refactor: reduce tracked query monomorphization#1150
MichaReiser wants to merge 8 commits into
salsa-rs:masterfrom
MichaReiser:erase-query-configurations

Conversation

@MichaReiser

@MichaReiser MichaReiser commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

This reduces tracked-query monomorphization, keeps fetch, shallow validation, and panic execution typed, and shares recoverable-cycle execution. Interner erasure is intentionally split into #1204.

Measured against #1206 (c89537c4) on pinned ty_python_semantic, total LLVM IR falls 109,056 lines (3.59%), Salsa function-framework IR falls 82,310 lines (25.63%), and a stripped maturin-built ty shrinks 303,200 bytes (1.16%). The matched release DateType simulation instruction count regresses 0.47%; full and incremental compile-time differences remain within run-to-run noise.

Testing: cargo fmt --check; cargo check --workspace --all-features; cargo test --workspace --quiet; matched pinned ty/DateType LLVM, instruction, compile-time, and binary-size measurements. Strict workspace Clippy reports only #1206's existing result_large_err warning.

@netlify

netlify Bot commented Jun 19, 2026

Copy link
Copy Markdown

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit 34bf6e2
🔍 Latest deploy log https://app.netlify.com/projects/salsa-rs/deploys/6a6de75426d26a000812b46c

@codspeed-hq

codspeed-hq Bot commented Jun 19, 2026

Copy link
Copy Markdown

Merging this PR will regress 2 benchmarks

⚡ 7 improved benchmarks
❌ 2 regressed benchmarks
✅ 65 untouched benchmarks
⏩ 1 skipped benchmark1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime project_check_then_incremental[NoEviction] 6.1 ms 7.7 ms -21.31%
Simulation deep_verify_memo 648.3 µs 679.9 µs -4.64%
WallTime parallel_fast_path[NoEviction] 507.9 µs 363 µs +39.91%
Simulation disabled_eviction[NoEviction] 177.4 µs 161.9 µs +9.57%
Simulation fast_path[NoEviction] 177.5 µs 162 µs +9.57%
Simulation fast_path_and_sweep[NoEviction] 178.5 µs 163 µs +9.53%
Simulation disabled_eviction[Lru] 178.3 µs 162.8 µs +9.47%
Simulation fast_path_and_sweep[Lru] 242.3 µs 229.4 µs +5.63%
Simulation fast_path[Lru] 241.4 µs 228.6 µs +5.62%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing MichaReiser:erase-query-configurations (34bf6e2) with master (59ab5b0)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@MichaReiser
MichaReiser force-pushed the erase-query-configurations branch 2 times, most recently from e1ed6be to 9e486e7 Compare June 27, 2026 15:46
@MichaReiser MichaReiser changed the title refactor: erase interned and tracked query configurations refactor: erase configuration from interned storage Jun 27, 2026
@MichaReiser
MichaReiser force-pushed the erase-query-configurations branch from 9e486e7 to e1ed6be Compare June 27, 2026 15:51
@MichaReiser MichaReiser changed the title refactor: erase configuration from interned storage refactor: erase interned and tracked query configurations Jun 27, 2026
@MichaReiser
MichaReiser force-pushed the erase-query-configurations branch from e1ed6be to 4201f31 Compare June 27, 2026 19:00
@MichaReiser MichaReiser changed the title refactor: erase interned and tracked query configurations refactor: reduce tracked query monomorphization Jun 27, 2026
@MichaReiser
MichaReiser force-pushed the erase-query-configurations branch 2 times, most recently from 9e2b7c6 to 9d4e55b Compare June 28, 2026 18:30
@MichaReiser
MichaReiser force-pushed the erase-query-configurations branch 2 times, most recently from 372a630 to cb798e2 Compare July 9, 2026 19:46
@MichaReiser

This comment was marked as resolved.

@codspeed-hq

This comment was marked as resolved.

@MichaReiser
MichaReiser force-pushed the erase-query-configurations branch from cb798e2 to bff0710 Compare July 31, 2026 07:52
@MichaReiser

This comment was marked as duplicate.

1 similar comment
@MichaReiser

This comment was marked as duplicate.

@MichaReiser

Copy link
Copy Markdown
Contributor Author

@codspeedbot what's the reason that the walltime benchamrks regress?

@codspeed-hq

codspeed-hq Bot commented Jul 31, 2026

Copy link
Copy Markdown

@codspeedbot what's the reason that the walltime benchamrks regress?

Here's what I found for the latest report (head bff0710 vs master b631dd8). Short version: the two questions have unrelated answers — the Simulation gains are a real instruction-count cut on the cached-hit path, while the WallTime deltas are not explained by extra work and I could not reproduce them off the CodSpeed aarch64 runner.

1. Why the Simulation benchmarks improve by that much

The improvements are exactly the pure cached-hit loops (fast_path, disabled_eviction, fast_path_and_sweep), and the whole delta sits in the instructions component:

fast_path[NoEviction] BASE HEAD Δ
instructions 93.32 µs 77.96 µs −16.5%
cache misses 4.01 µs 4.01 µs −0.1%
memory access 80.14 µs 80.03 µs −0.1%
total 177.47 µs 161.99 µs −8.7%

The flamegraphs show where those instructions went. On master the fetch path is still a chain of real calls: access_all_withLocalKey::with::<attach…> (own frame, 18.2 µs of pure instruction self-time) → get_memo_from_table_for (own frame). Its callee list includes the full per-call ceremony — try_with/as_ref<Attached>, DbGuard drop glue, try_borrow_mut<QueryStack>, replace<isize>, get_or_try_init<DatabaseDownCaster>, uncancel. On the PR head both of those frames are gone: the entire hot fetch is inlined into access_all_with, and that ceremony largely disappears with it. That is precisely the intended effect of taking the cold/generic bulk out of the fetch path — the hot remainder becomes small enough for the inliner to swallow whole, so ~15 ns of modeled instruction time per cached fetch disappears (~16%).

One thing worth keeping in mind when reading the 9.5%: that benchmark is ~49% modeled memory time (chasing the memo pointer) and ~53% instructions in the base. Simulation adds those components, so a −16% instruction cut becomes −8.7% of the total. On real hardware those loads overlap with execution, so the same saving should be expected to show up much smaller in wall time — and indeed the hit-dominated WallTime benchmarks (parallel_fast_path, project_check_then_incremental) are flat rather than 9% faster.

Also visible on the Simulation side: the recoverable-cycle path pays for the erasure. converge_diverge_nested is +2.47% and converge_diverge +0.97%, driven by memory accesses (+6.0% and +3.9% respectively) rather than instructions — consistent with the erased ExecuteContext/CycleStateImpl/ErasedMemo values being passed through memory instead of registers. Cold execution itself is flat (cold_execute +0.31%, deep_verify_then_execute −0.23%, fill_and_evict −1.0%).

2. Why the WallTime benchmarks regress

All 10 WallTime benchmarks moved in the same direction, and the size of the move tracks how much of the workload is not a cached hit:

WallTime benchmark BASE min HEAD min Δ min Δ median
one_hit_wonders[Lru] 1216.2 µs 1329.3 µs +9.3% +8.9%
scan_resistance[Lru] 1114.8 µs 1178.7 µs +5.7% +7.1%
phase_change[Lru] 374.1 µs 394.5 µs +5.5% +5.6%
one_hit_wonders[NoEviction] 838.0 µs 873.2 µs +4.2% +3.8%
scan_resistance[NoEviction] 842.3 µs 857.6 µs +1.8% +2.0%
phase_change[NoEviction] 315.9 µs 319.9 µs +1.3% +0.7%
parallel_fast_path[Lru] 3810.6 µs 3859.9 µs +1.3% +3.3%
parallel_fast_path[NoEviction] 361.2 µs 361.1 µs −0.0% +1.6%
project_check_then_incremental[Lru] 9959.1 µs 9961.1 µs +0.0% +0.1%
project_check_then_incremental[NoEviction] 6026.2 µs 6040.0 µs +0.2% +0.1%

project_check_then_incremental hits each file 64× in a row (≈99% hits) and is flat; the regressed ones have a high share of first-execution and re-validation work, and the Lru variants — which recompute what they evicted — regress 2–4× more than their NoEviction twins.

That pattern points at the execute/validate path, so I checked whether it actually does more work. It does not:

  • Cachegrind on the exact same workloads (same binaries, deterministic 20 rounds): phase_change 159.23M → 159.11M instructions (−0.07%), D refs +0.34%, D1 misses +0.4%, LL misses flat. one_hit_wonders 256.48M → 256.14M (−0.13%), D refs +0.57%, D1 misses +0.15%. Only L1i misses tick up (+2–5%, tiny in absolute terms).
  • Native wall-clock runs, 3 alternating repetitions each pinned to one core: the head is equal or slightly faster everywhere, e.g. one_hit_wonders[NoEviction] medians 232.1/225.4/226.1 µs on master vs 224.0/224.1/222.9 µs on head; phase_change[Lru] 100.8/99.7/100.0 vs 98.6/98.9/103.4.
  • CodSpeed's own Memory instrument is byte-identical on the eviction benchmarks (fill_and_evict[Lru] 35.5 KB on both), so no extra allocations either.

So on identical inputs the head executes the same instructions, touches the same memory, and allocates the same bytes. What did change is the code layout on exactly the paths those benchmarks stress. execute lost its #[inline(never)] (and fetch_cold_cycle too), so on x86_64 the cold body is now inlined into fetch_cold (607 → 831 bytes), insert_memo is inlined into the new execute_panic (1457 bytes, and it no longer exists as a separate symbol), and maybe_changed_after grew 1498 → 1613 bytes. On a different micro-architecture, front-end/branch behaviour on those cold and validation paths can plausibly cost a few percent of wall time at constant instruction count — and that is exactly the dimension Simulation does not model (it accounts for instruction counts plus D-cache/memory, not branch prediction or front-end pressure), which is why the two instruments disagree here.

I want to be explicit about what is not established: I could not reproduce the regression outside the runner, so the layout explanation stays a hypothesis. Two caveats also argue for confirming the signal first: these divan benchmarks use sample_size = 1 with no warmup and a fresh database per round, which makes every measured round sensitive to cold-code effects; and because eviction_walltime is new, this is the only base/head pair where these benchmarks exist, i.e. a single observation of a 2–4σ shift.

Suggested next steps, in order:

  1. Rebase and re-trigger so you get a second base/head pair. If the four deltas reproduce at the same magnitude, it is a genuine aarch64 codegen effect and not a one-off.
  2. If it does reproduce, the cheapest targeted lever is restoring #[inline(never)] on the execute entry point (and keeping insert_memo/finish_memo out of line) so the cold body stays out of fetch_cold, as on master. That keeps all of the monomorphization win — the hit path, where the Simulation gains come from, is untouched by it.
  3. Independently of the WallTime question, the converge_diverge* +1–2.5% memory-access increase is a real, measurable cost of the erased cycle path if you want to chase it.
Summary
  • Warming up the performance wizard...
  • Analyzing walltime regressions and simulation improvements — Compared runs, flamegraphs and instrument breakdowns, reproduced the workloads locally (native + cachegrind) and inspected the compiled code layout. No repository changes were needed.
  • Handling comment — Comment handling finished successfully

Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.

@SuperMuel

Copy link
Copy Markdown

Hi @MichaReiser, I’m Samuel from CodSpeed. I work on the CodSpeed AI agent 👋

Sorry about the failed requests. I fixed the bug that prevented the agent from replying here and restarted the latest request.

Feel free to tag me, @SuperMuel, if you run into any other issues with it!

@MichaReiser
MichaReiser force-pushed the erase-query-configurations branch from bff0710 to 34bf6e2 Compare August 1, 2026 12:32
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