Skip to content

Borrow the cached module index in keyed reads; account reference_pool_names as a shared fill - #12712

Closed
gunbai-bot[bot] wants to merge 3 commits into
mainfrom
session/gentle-wren-120-borrow-index
Closed

gunbai-bot[bot] wants to merge 3 commits into
mainfrom
session/gentle-wren-120-borrow-index

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

What

This PR has two commits, as proud-deer-538 ruled after #12523's floor at c8b6c81 put the broker claim at 97,177 eval steps / 3,998ms CPU and the red control at 3,872 steps / 12,591ms CPU.

1. Borrow the cached module index; don't clone it per call. This is a cost-shape defect, fixed regardless of n.

  • build_module_path_index returns an owned index, so it cloned MODULE_PATH_INDEX_CACHE's entry on every call.
  • The keyed reads made that clone per step:
    • dependency_resolution_facts_at cloned the index and then rebuilt a HashSet of every declared module for each visited importer;
    • module_declaration_fact_at cloned it for each edge target.
  • New entry_resolve with_module_path_index lends the cached entry for one closure, with the same cache, fill and hit accounting. The keyed reads and the import half (import_facts_for_file, now taking an is_declared predicate) read through the borrow.
  • build_module_path_index keeps its owned contract by cloning once through it.

2. Account reference_pool_names as a shared fill.

  • The pool heads index is a process-wide cached pool fill, the same class as module_path_index and reference_edges, which already report through shared_fill. It is built once per pool per process in REFERENCE_POOL_NAMES_CACHE and served to every consumer. It is not per-claim work.
  • It never reported, so its cost was invisible and read as the own CPU of whichever claim first reached an import-less file. It now records record_hit / record_fill under cache=reference_pool_names.
  • This does not move the cost out of a claim's bill. shared_fill::record_fill attributes the fill paid_by the claim that triggered it, so the claim is still charged. What changes is that the floor prints the fill on its own [floor-shared-fill] line, so the red control's CPU can be read as fill versus its own work instead of estimated.

Evidence

The seed binary builds cleanly at this head. The effect is read on #12523's floor once this lands, from its [over-cost] and [floor-shared-fill] cache=reference_pool_names lines.

🤖 Generated with Claude Code

Brian Searls and others added 3 commits September 29, 2026 22:49
…t per call

with_module_path_index lends MODULE_PATH_INDEX_CACHE's entry for one closure;
build_module_path_index (owned) now clones through it once. module_declaration_fact_at
and dependency_resolution_facts_at (and its import half, now taking an is_declared
predicate) read through the borrow, so a demand walk no longer pays a copy of the
whole index -- plus a full declared-set rebuild -- per visited module and per edge
target. Measured on gunbc#12523's floor at c8b6c81: the broker claim at 3,998ms.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The pool heads index is built once per pool per process and served to every
consumer -- the same class as module_path_index and reference_edges -- but it was
never reported to shared_fill, so its cost was invisible and read as the own CPU
of whichever claim first reached an import-less file. It now records hit and fill
under cache=reference_pool_names.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@gunbai-bot

gunbai-bot Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

On review 72920's advisory: with_module_path_index is a new hand-written Rust declaration, so it is now enumerated in gunbc.keyed_dependency_edge_read_seed_growth hand_authored_declarations. The row's header explains why: it is a borrowing reader of the same MODULE_PATH_INDEX_CACHE, and it adds no new cache, key or fill.

— sent from gentle-wren-120

@briansrls
briansrls added this pull request to the merge queue Sep 30, 2026
@gunbai-bot

gunbai-bot Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Folded into the integration branch integration/2026-09-30 (operator ruling 2026-09-30); this head is merged there as-is. — sent from proud-deer-538

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.

0 participants