Repository navigation
perf(obc-pack): the content steps find sources and facts by key - #2728
Merged
Merged
Conversation
The landmark and peak content steps scanned a whole collection once per item. A large region spent most of its time in these scans. - read_pinned finds a source in a map by path and canonicalizes the snapshot root once. - The shared view reads and parses each fact file once, and finds an asset fact by its digest. - The peak compiler groups resolutions by node and finds a place by its identity. The output bytes do not change, except policy_sha256, which hashes the compiler source. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Summary
The in-process steps
device/landmark-contentanddevice/peak-contentscanned a whole collection once per item. On a large region these scans were quadratic. This change builds each lookup once.read_pinned(landmarks/mod.rs): a newSourcestype holds the pinned sources in a map by path, under the snapshot root. The root is canonicalized once.Sources::newalso rejects a duplicate path, soload_snapshotdoes not do that check separately. The(root, sources)parameter pair is now one&Sources.shared::view: a privatereadreads and parses each fact file once. It returns the facts and a pin (key, digest, length) for each file.landmark_viewandpeak_viewgive these toview_of, so they do not read the files again. An asset fact is found by its digest in a map. A set holds the paths that can repeat (canonical aliases and proofs). The old code did a scan ofsourcesfor this.peaks::compile: resolutions are grouped by node ID, and places are put in a map by identity. The first place of an identity wins, asfinddid before.The public signatures of
shared::{facts, view, landmark_view, peak_view}do not change.Output
The output bytes do not change. A synthetic bake of 2,000 places ran with the old code and with the new code, from the same inputs:
manifest.jsonfor landmarks and for peaks is byte-identical.content.jsonandpeaks.jsonare different inpolicy_sha256only. That field hashes the compiler source (include_bytes!("mod.rs")and the other files), so it changes with any edit. All other fields are equal.The existing landmark and peak tests also pin the output.
Measurement
I did not have a local data store, so I could not do a DACH bake. I used a synthetic region instead: N places, each with one entity, one article, one Commons file, one asset, and one link fact. The landmark view and compile ran on it, then the peak view and compile. Release build on the dev laptop (15 GB RAM). Times are in seconds.
The time is now linear in the number of places. Before, three times the places took about ten times as long. I did not run the 20,000 baseline. If the trend continues, it would take approximately 10 minutes for each view.
A remaining scan:
shared::subjectfalls back to a scan of all facts when an ID is not an entity key. In a peak view this occurs for a summit whose link resolves to a non-QID identity (wiki-<lang>-<page>). That case is rare, so this change does not address it.Checks
obc test -p obc-pack: 95 passed.cargo clippy -p obc-pack --all-targets -- -D warnings: clean.cargo fmt -p obc-pack --check: clean.obc test affected, and any check outsideobc-pack. CI is the gate.Public docs did not change.
Requirements: none
🤖 Generated with Claude Code