Skip to content

Inline input field access helper#1227

Draft
MichaReiser wants to merge 1 commit into
salsa-rs:masterfrom
MichaReiser:micha/input-interned-field-access-opt
Draft

Inline input field access helper#1227
MichaReiser wants to merge 1 commit into
salsa-rs:masterfrom
MichaReiser:micha/input-interned-field-access-opt

Conversation

@MichaReiser

@MichaReiser MichaReiser commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add an inline hint to the input field access helper

Notes

Local instruction-count inspection showed that inlining input::IngredientImpl::field removes the hot call from the input field read query body. Marking data / data_raw alone does not help, because the query body still calls field; interned field reads were already inlined in the benchmark body without additional hints.

Verification

  • cargo check

@netlify

netlify Bot commented Jul 2, 2026

Copy link
Copy Markdown

👷 Deploy Preview for salsa-rs processing.

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

@netlify

netlify Bot commented Jul 2, 2026

Copy link
Copy Markdown

Deploy Preview for salsa-rs canceled.

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

@MichaReiser MichaReiser force-pushed the micha/input-interned-field-access-opt branch from 2603cca to 6f1fe98 Compare July 2, 2026 19:13
@MichaReiser MichaReiser changed the title Inline field access helpers perf: Inline field accessors Jul 2, 2026
@MichaReiser MichaReiser force-pushed the micha/input-interned-field-access-opt branch from 6f1fe98 to 93535ed Compare July 2, 2026 19:16
@MichaReiser MichaReiser changed the title perf: Inline field accessors Inline input field access helper Jul 2, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 2, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 98.32%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 4 regressed benchmarks
✅ 46 untouched benchmarks
⏩ 12 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation untracked_field_read_many 219.6 µs 340,581.1 µs -99.94%
Simulation interned_field_read_many 210.7 µs 212,160.4 µs -99.9%
Simulation cold_execute 1.6 ms 460.4 ms -99.65%
Simulation tracked_field_read_many 270.6 µs 480.8 µs -43.71%
WallTime hot_cache_hits[1024] 163.2 µs 156.5 µs +4.3%

Tip

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


Comparing MichaReiser:micha/input-interned-field-access-opt (93535ed) with master (9447e2f)

Open in CodSpeed

Footnotes

  1. 12 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@MichaReiser

Copy link
Copy Markdown
Contributor Author

@codspeedbot analyze this regression

@codspeed-hq

codspeed-hq Bot commented Jul 3, 2026

Copy link
Copy Markdown

@codspeedbot analyze this regression

Regression analysis

Conclusion: these regressions are not caused by your #[inline] change — they are artifacts of the base and head runs executing in different runtime environments.

CodSpeed flagged an Environment Differences warning on this comparison: the head (93535ed) and base (9447e2f, master) runs used different libc builds:

  • libc.so.6 build_id 034c9554…9f32d43c…
  • ld-linux-aarch64.so.1 build_id 51e41459…99b08204…

The reported deltas are also physically impossible for a single inline hint — e.g. untracked_field_read_many going from 219.6 µs to 340,581 µs (~1500×). A change that only marks input::IngredientImpl::field inline cannot inflate a hot loop by three orders of magnitude.

What the flamegraph shows

For the worst "regression" (untracked_field_read_many, 340.6 ms total), 74.5% of self time (253.6 ms) is __aarch64_cas8_acq_rel — an atomic compare-and-swap — reached through malloc in libc.so.6. That is precisely the library whose build_id differs between the two runs. The salsa functions themselves (fetch_cold, execute, insert_memo, …) each show only single-digit-µs self time; the multi-hundred-ms totals come entirely from the differing libc's allocator path being scored very differently by the simulator.

In other words, the simulation is comparing two different C libraries, not two versions of your code.

Recommendation

To get a trustworthy signal, re-run base and head in the same environment:

  1. Rebase this branch onto the latest master and re-trigger CI so a fresh base run is produced on the same runner image.
  2. Re-evaluate the comparison — same-environment results are the only meaningful ones here.

The only same-environment signal in the current report is the WallTime hot_cache_hits[1024] improvement (+4.3%). The four Simulation "regressions" should be disregarded until the base is re-measured on matching libc/ld builds.

No code changes are needed for this — it's purely a measurement-environment issue.

Summary
  • Warming up the performance wizard...
  • Analyzing regression — Determined the reported regressions are artifacts of base and head running in different runtime environments (different libc build). No code change required.
  • Handling comment — Comment handling finished successfully

Learnings updated. See what CodSpeed remembers about this repository.

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

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.

1 participant