Repository navigation
perf(storage): clone pending facts straight into all_facts (#543) - #587
Merged
Merged
Conversation
Snapshot::all_facts collected the pending facts with pending_range, which copied them into a temporary vector before moving them into the result. Recursive rules read every fact of an in-memory store holding the whole database as pending facts, so each read allocated and copied it twice: recursive_ancestors at 1M facts went from 45.9 s to 56.4 s (+24%) after #580. The pending facts are now cloned onto the committed facts' vector. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyLscCUuJ1GQpdAeX79EdR
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
7 tasks
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.
Fixes a regression from #580 (unreleased,
v3only). Refs #543.Snapshot::all_factsbuilt its pending part withpending_range, which cloned the pending facts into a temporary vector before they were moved into the result. Recursive rules read every fact of their in-memory derived store, which holds the whole database as pending facts, so each read did an extra allocation and copy of the full set. The perf envelope'srecursive_ancestorsat 1M facts went up 24%.all_factsnow starts from the committed facts' vector and clones the pending facts onto its end under the pending lock.pending_range(used byFactLog) shares the same helper and keeps its clamping. Ordering and locking are unchanged.Measurements
recursive_ancestors, 1M-fact envelope fixture, envelope profile, 10 runs per sample, median. Run on an idle machine, with each pair run in parallel and then repeated with the sides swapped.27951e3)v3(771fe4e)The PR's two numbers per run are its two pairings, one against base and one against
v3.A profile of
v3showed ato_vecframe at 3.6% that base does not have.hot_currentandhot_as_ofwere re-measured the same way and show no regression from #580; the +12% in the earlier envelope run was noise.Tests
cargo test: 1463 passed, 10 ignored. No new test, because behaviour is unchanged: the existing snapshot and fact-log tests cover the ordering and the clamping.🤖 Generated with Claude Code
https://claude.ai/code/session_01HyLscCUuJ1GQpdAeX79EdR