perf(graph): store the declined calls' candidate lists once per distinct list - #208
Open
joyful-ii-V-I wants to merge 2 commits into
Open
perf(graph): store the declined calls' candidate lists once per distinct list#208joyful-ii-V-I wants to merge 2 commits into
joyful-ii-V-I wants to merge 2 commits into
Conversation
…nct list The resolver recorded every tier-3 declined call's candidate definitions in a per-call CSR (declinedCandOff/declinedCand), appending the whole same-name candidate list for each call. A common name repeats its full definition list on every declined call to it. On a sparse llvm-project tree, 715,735 declined calls held 27.9 M entries (114 MB, 167 MB of capacity); `get` alone was 7,270 calls over one 424-candidate list. Entries per node grew with the tree: 9.4, 41.8 and 69.2 on three growing subtrees, 73.1 on the whole. The offsets were uint32 and would have wrapped silently past 4.29 B entries, and every declined_calls= answer scanned all of them. The same tree holds only 9,879 distinct lists, 62,359 entries in all. The only reader, declinedCallsNaming, asks of each declined call whether its candidates include a target and counts each call once, so list membership plus a call count answers it exactly. The new internDeclinedList keys a declined call's list by its exact NodeId sequence. FNV-1a (hashutil::fnv1aAbsorb) over the candidate bytes picks a bucket in a HashMap<uint64_t, SmallVec<uint32_t, 1>> reserved to 4096. A hit is confirmed by length and memcmp and adds one to that list's call count; a miss appends the list to a distinct-list CSR (declinedListOff/declinedListCand) with a count of 1. No per-call array is kept. The reader scans each distinct list until its first target, then adds that list's call count. Equal sets meet without a sort. A declined call is never canonical, narrowed or SCIP-pinned, so its candidates come only from the byName fill, in symbol-id order with each id once, and the decl/def collapse and the namespace, JVM, Rust and std:: filters each keep an ordered subset. A temporary check over the stored lists found none that was not strictly ascending, on the llvm tree and on this repository, and no hash bucket held more than one list. Determinism holds: the resolve loop is sequential, list numbers are first-seen, the map is only probed and inserted into, and no list number reaches output. A new list that would carry the uint32 offsets past UINT32_MAX is not recorded and raises DEGRADED_PATH_ALERT. The call stays counted in declinedOut, and declined_calls= stays within the counts_floor="1" its answers already carry. verifyOffsetCsr in src/infra/csrverify.h VERIFYs the new CSR beside verifyCsr. The interner is defined beside its reader at the end of src/graph.h, behind a forward declaration, so no line above that point moves: showcasecapturecheck arm (H) resolves the published seed src/graph.h:3362 to rankGraphTeleport, and a first version that defined the interner above buildGraph turned that arm red. On the llvm tree the structure is now 9,879 lists and 62,359 entries: 368 KB, 512 KB of capacity. The saving applies to each graph held at once. McpIndex keeps one for a whole MCP session, and quality-delta builds a second for its HEAD snapshot. Byte identity against an origin/main (558a2e0) build, both binaries on the same tree with --no-cache: 14 of 14 outputs identical on this repository and 14 of 14 on the llvm tree. They are the default map; --callers=get, --callers=size, --callers=FILE:size (whose next= widens to --uses=size) and --impact=size, each in XML and --json; and over MCP, find_referencing_symbols for get, size and FILE:size, impact, and find_symbol. test/mcpclidiffcheck.sh passes 21 of 21 with each binary, with identical rows. Cold --no-cache map of the llvm tree, /usr/bin/time -l, three interleaved runs per binary, medians: peak memory footprint 1913 → 1768 MiB (−146 MiB), wall 6.65 → 6.49 s, and max RSS 1983 → 2124 MiB (+142 MiB). The machine was shared, with load averages of 16-19 on 18 cores during these runs, and max RSS was dominated by noise: an earlier round on the same logic read max RSS 2235 → 2122 MiB (−113 MiB), footprint 1808 → 1673 MiB (−134 MiB) and wall 7.58 → 6.99 s. Peak footprint fell in both rounds, by 134 and 146 MiB, against the ~160 MiB the per-call CSR had reserved. None of this is gated. test/declinedlistcheck.sh (30 rows) pins that lists are shared by content, never by name. `size` is defined in C++ under a/ and b/ and in Python under c/ and d/, with one declining caller each, so --impact=a/size.cpp:size reads declined_calls="1" where a by-name share reads 2, while bare --callers=size reads 2. A list named by two calls keeps its call count. Two roots each defining `foo` twice give --callers=r1/x.cpp:foo 1. main was already exact (30 of 30 on the origin/main binary), so red was shown by mutation: keying the interner by the called name turns 12 rows red, the (A) definition rows including --json and MCP and both (C) definition rows, while the bare-name controls stay green. Reverted, 30 of 30 pass. The gate is listed in test/regression.sh, and docs/gatecount_build.py wrote 608 to its eight sites. test/declinecheck.sh passes 161 of 161, arms A-H. On an ASan build, declinedlistcheck passes 30 of 30 and declinecheck 161 of 161, and a map of this repository and --callers=size on the llvm tree print no sanitizer report. pargates over declinedlist, decline, callers, packcallershare, impactimport, impactpartition, mcpclidiff, mcpattrparity, relink (the gate that names csrverify.h), det-gate, xmlwellformed, gatecount and manifest: gates=13 pass=13 skip=0 fail=0. docs/limits_build.py --check, docs/gatecount_build.py --check and scripts/formatcheck.sh pass. --quality-delta against git HEAD exits 0 with gating=0 and new-symbol=0. Its three rows are sev="minor" short-horizon-churn with churn="self", on Graph, buildGraph and declinedCallsNaming; that kind is preexisting by construction and gates only on two or more committed rewrites inside its window. api-new-surface=2 counts internDeclinedList and verifyOffsetCsr. stale=75 are acknowledgement-ledger rows that are already stale on main, where the same binary reports 76. --test-gate exits 4 with changed=6, impacted=868, tests=39 and untested=764. buildGraph feeds nearly every verb, so the untested radius is the tree's dispatchers (dispatchMcpLine, dispatchMain, runLint and on), whose outputs the identity runs above show unchanged. Of the 37 gate scripts it names besides test/regression.sh itself, 4 are in the set above and the other 33 ran in a second pargates pass: gates=33 pass=33 skip=0 fail=0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… gate list unioned, count regenerated Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> # Conflicts: # test/regression.sh
|
Warning Review limit reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
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
1 participant
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.
The call-graph build keeps, for every call the resolver declines to bind, the list of candidates it declined between, so
--callersand friends can say how many calls were declined for a symbol. The lists were stored once per call, so the structure grew with calls × candidates: 27.9 M entries (114 MB) on an llvm-sized corpus, where most declined calls repeat a handful of identical lists.Change
internDeclinedListkeys each declined call's list by its exact candidate sequence: an FNV-1a hash picks a bucket, and a hit is confirmed by length andmemcmp. The storage becomes a CSR of distinct lists plus a call count per list; the per-call array is gone.declinedCallsNamingwalks the distinct lists and adds each matching list's call count.DEGRADED_PATH_ALERT, and a newverifyOffsetCsrinsrc/infra/csrverify.hchecks the list CSR next toverifyCsr.Evidence
mcpclidiffcheck21 of 21.test/declinedlistcheck.sh(30 rows): a mutation that shares lists by name instead of by content fails 12 of them; this change and main both pass all 30.declinecheckpasses 161 of 161, and both gates run clean under ASan.🤖 Generated with Claude Code