Skip to content

fix: make immortal interned values cheaper - #1255

Draft
MichaReiser wants to merge 1 commit into
salsa-rs:masterfrom
MichaReiser:perf/cheaper-immortal-interning
Draft

fix: make immortal interned values cheaper#1255
MichaReiser wants to merge 1 commit into
salsa-rs:masterfrom
MichaReiser:perf/cheaper-immortal-interning

Conversation

@MichaReiser

@MichaReiser MichaReiser commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Main benefit; It also reduces the size of each interned value because we avoid storing the unnecessary Lru overhead.

Maps revisions = usize::MAX to a compact compile-time no-eviction policy while keeping LRU as the default and preserving the existing macro API.
Removes unused LRU state and bookkeeping for immortal values; simulation improves hot interning by 32.6% and cold interning by 13.6%, with no LRU regression.
Testing: Added behavioral, persistence, layout, and performance coverage; full suites and lint checks pass.

@netlify

netlify Bot commented Jul 17, 2026

Copy link
Copy Markdown

Deploy Preview for salsa-rs canceled.

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

@codspeed-hq

codspeed-hq Bot commented Jul 17, 2026

Copy link
Copy Markdown

Merging this PR will regress 3 benchmarks

⚡ 1 improved benchmark
❌ 3 regressed benchmarks
✅ 70 untouched benchmarks
🆕 4 new benchmarks
⏩ 1 skipped benchmark1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory intern_same_many 88 B 104 B -15.38%
Simulation input_create_many[32] 5.9 µs 6.3 µs -6.3%
Simulation deep_verify_memo 648.3 µs 677.4 µs -4.3%
WallTime parallel_fast_path[NoEviction] 507.9 µs 365.2 µs +39.08%
🆕 Memory intern_immortal_distinct_many N/A 14.7 KB N/A
🆕 Memory intern_immortal_same_many N/A 96 B N/A
🆕 Simulation intern_immortal_distinct_many N/A 74.4 µs N/A
🆕 Simulation intern_immortal_same_many N/A 20.6 µs N/A

Tip

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


Comparing MichaReiser:perf/cheaper-immortal-interning (d9908d8) 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 perf/cheaper-immortal-interning branch from 75dd835 to d9908d8 Compare August 1, 2026 12:36
@ChayimFriedman2

Copy link
Copy Markdown
Contributor

Does ty have statically immortal interned values? In r-a we use revisions = usize::MAX but that's mostly because we haven't completely figured out the story around the GC yet.

If nobody needs this, it does not make sense to optimize this.

@MichaReiser

Copy link
Copy Markdown
Contributor Author

We currently don't. But it seems wasteful to track all LRU metadata for interned values that have LRU disabled. But we can consider removing revisions = usize::MAX altogether.

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