Skip to content

perf(graph): fast-reject FileNode Eq and hash interned node strings by content id - #832

Merged
jonathanong merged 2 commits into
execute-plan/79dd0742-pr-1-cheap-concatenated-nodesortkey-comparisonfrom
execute-plan/79dd0742-pr-2-faster-filenode-eq-and-interned-string-hashing
Sep 5, 2026
Merged

perf(graph): fast-reject FileNode Eq and hash interned node strings by content id#832
jonathanong merged 2 commits into
execute-plan/79dd0742-pr-1-cheap-concatenated-nodesortkey-comparisonfrom
execute-plan/79dd0742-pr-2-faster-filenode-eq-and-interned-string-hashing

Conversation

@jonathanong

@jonathanong jonathanong commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

Reject FileNode equality on path-hash mismatch without reading OsStr bytes (still compare paths when ids match, for hash-collision safety). Wrap symbol/job/module/procedure names in InternedStr so HashMap probes hash a stored Fx content id, never an Arc pointer. Standalone NodeId::symbol and NodeId::symbol_in stay Hash/Eq compatible.

Local Criterion (source of truth)

Machine: macOS aarch64, rustc 1.96.0, --sample-size 20. Same machine before/after. CodSpeed is not used as a gate.

bench before (mean) after (mean) change
graph_production_finalization/node_id_finalization 26.986 ms 27.067 ms +0.51% (n.s.)
graph/all_domains_build 32.424 ms 31.349 ms −4.23% (n.s.)

No solo win on these benches; the change is Hash/Eq correctness plus cheaper probes that compound with PR 1 on real maps.

Shepherd Journal
  • Fixed rust-no-inline-tests: moved FileNode/InternedStr from_parts to interned_identity_tests.rs sibling module (a21059f). CodSpeed regressions reported different runtime environments; local Criterion showed no solo win, not treated as a same-environment regression.
  • codecov/project/rust FAILURE: no coverage on head while Rust tests are still inProgress after the rust-no-inline-tests fix; not a code defect.
  • First-look CodSpeed comment already minimized; different runtime environments; local Criterion n.s. solo win.
  • Codex quota comments IC_kwDOSaSRXM8AAAABStKfwA/IC_kwDOSaSRXM8AAAABStKhBA and Sourcery budget review PRR_kwDOSaSRXM8AAAABMTHoCw are bot-limit notices, not code findings; minimize only.
  • Codex quota comments IC_kwDOSaSRXM8AAAABStOdow/IC_kwDOSaSRXM8AAAABStQZFw are bot-limit notices, not code findings; minimize only.
  • codecov/project/rust "No coverage information found on base report" is stacked-PR upload lag against perf(graph): memcmp concatenated NodeSortKey instead of flatten iterators #831, not a FileNode/InternedStr defect.
  • codecov/project/rust "No coverage information found on base report" remains stacked-PR upload lag against perf(graph): memcmp concatenated NodeSortKey instead of flatten iterators #831 after tests finished; no FileNode code change.
  • codecov/project/rust "No coverage information found on base report" persists after perf(graph): memcmp concatenated NodeSortKey instead of flatten iterators #831 ready-delay CANCEL; stacked PRs have no Codecov base until perf(graph): memcmp concatenated NodeSortKey instead of flatten iterators #831 coverage is the comparison target. Not a FileNode/InternedStr defect.

@coderabbitai

This comment has been minimized.

@sourcery-ai

This comment has been minimized.

@codecov

This comment has been minimized.

@codspeed-hq

This comment has been minimized.

@jonathanong
jonathanong marked this pull request as ready for review September 5, 2026 07:27
@chatgpt-codex-connector

This comment has been minimized.

sourcery-ai[bot]

This comment was marked as resolved.

@chatgpt-codex-connector

This comment has been minimized.

@jonathanong
jonathanong force-pushed the execute-plan/79dd0742-pr-2-faster-filenode-eq-and-interned-string-hashing branch from a21059f to d4d41b9 Compare September 5, 2026 07:40
@chatgpt-codex-connector

This comment has been minimized.

…y content id

Reject FileNode equality on path-hash mismatch and hash symbol/job strings
by a stored Fx content id so map probes do not rehash OsStr/str bytes.
rust-no-inline-tests forbids inline #[cfg(test)] items in production
source; keep from_parts on interned_identity_tests.
@jonathanong
jonathanong force-pushed the execute-plan/79dd0742-pr-2-faster-filenode-eq-and-interned-string-hashing branch from d4d41b9 to 03ccd35 Compare September 5, 2026 07:46
@chatgpt-codex-connector

This comment has been minimized.

@jonathanong
jonathanong merged commit b6b0b34 into main Sep 5, 2026
38 of 41 checks passed
@jonathanong
jonathanong deleted the execute-plan/79dd0742-pr-2-faster-filenode-eq-and-interned-string-hashing branch September 5, 2026 09:18
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