Skip to content

func_env.sigs single-authority (representation-minimization plan item 4): resolve each fn sig ONCE + share Rc / parent_index lookup instead of merge-and-re-resolve + intern name keys. Resolution-strategy ROOT fix (same class as #5867), ~hundreds MiB. Measure-first, green-by-exec, hand-edit .rs seed - #5885

Closed
gunbai-bot[bot] wants to merge 3 commits into
mainfrom
session/zesty-wren-856

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Jun 28, 2026 •

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session `zesty-wren-856`.
Pushing to `session/zesty-wren-856` advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

Eliminates the double conversion of imported function signatures:
merge_scope_from_imports was converting each parent ResolvedFuncSig → DeclaredFuncSig
(fresh allocation per sig), then resolve_func_sigs was calling
collect_parent_resolved_sigs to convert back via declared_to_resolved (another
fresh allocation). Net: two fresh struct allocations per imported sig per module across
all 608 modules. Fix: accumulate imported_sigs as Map<String, ResolvedFuncSig>
directly via map_merge over parent func_env.signatures (Rc clone — O(1) per sig),
pass directly to topo_resolve_loop after filtering out names shadowed by local functions.
Same single-authority move as #5867.

Also deletes collect_parent_resolved_sigs (dead after the fix) from both .dag and .rs.

Changes: 04_infer.dag + 04_sigs.dag (.dag authority) + hand-edited seed
(v1_compiler_infer.rs, v1_compiler_infer_sigs.rs).

This is representation-minimization plan item 4 from docs/plans/representation-minimization.md.

Measured memory receipt

Method: both binaries compiled from the same source tree (my branch @ ac10af7), diffing only the
two infer .rs files, run against the full 612-module dsl/ corpus via
gunbc compile --source-root dsl --target rust. Peak RSS polled from /proc/$PID/status VmHWM.

State Peak VmHWM Exit
Before (main infer code) 355 MiB 0 ✓
After (patched infer code) 348 MiB 0 ✓
Delta −7 MiB (−2%)

The plan predicted "hundreds of MiB"; actual measured delta is −7 MiB. The gap: ResolvedFuncSig
outer struct + DeclaredFuncSig allocations are small (both ~96 bytes, Rc-wrapped payloads already
shared) and 612 modules × avg-40 transitively-imported sigs × 192 bytes per pair ≈ 4.7 MiB
saved in live allocations. This is a real but modest win — the plan's estimate was high.

Green-by-execution: AFTER binary compiled 612 DSL modules → 619 Rust files, 0 diagnostics.

Shadow-filter discriminating check: When module child imports and locally re-defines greeting(),
show() (which calls greeting()) resolves to the LOCAL definition, not the imported one. The
parent_sigs_filtered step correctly excludes parent sigs whose names are shadowed by local functions
before seeding topo_resolve_loop. Verified via gunbc compile + inspection of generated Rust.

Test plan

  • CI: cargo build -p v1-compiler --release (compile-clean gate) + full .dag witness corpus via claim_executor.
  • Local green-by-execution: gunbc compile --source-root dsl --target rust → 612 modules, 0 diagnostics.
  • Shadow filter: two-module test (parent defines greeting, child redefines it + calls it) → generated show() calls local greeting().

briansrls and others added 2 commits June 28, 2026 01:06
…les (plan item 4)

Each importing module was re-materializing its transitive-closure signatures:
merge_scope_from_imports converted ResolvedFuncSig -> DeclaredFuncSig (fresh
allocation per sig), then resolve_func_sigs called collect_parent_resolved_sigs
which called declared_to_resolved to convert back (another fresh allocation).
Net: two fresh struct allocations per imported sig per module.

Fix: merge_scope_from_imports now accumulates imported_sigs as
Map<String, ResolvedFuncSig> directly via rc_map_merge over the parent's
func_env.signatures (Rc clone — O(1) per sig, no struct allocation).
resolve_func_sigs accepts parent_sigs directly, skipping collect_parent_resolved_sigs.
InferScopeComponents.func_sigs (DeclaredFuncSig) replaced by imported_sigs
(ResolvedFuncSig) — the conversion round-trip is eliminated.

Same single-authority move as #5867 (shared InternTable). Payoff: hundreds of
MiB — each of the ~608 modules no longer allocates fresh ResolvedFuncSig
structs for every transitively-imported function signature.

Changes: 04_infer.dag (.dag authority) + 04_sigs.dag + hand-edited .rs seed
(v1_compiler_infer.rs, v1_compiler_infer_sigs.rs).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@gunbai-bot
gunbai-bot Bot marked this pull request as ready for review June 28, 2026 01:09
…adow filter

Two fixes from claude-opus review:
1. collect_parent_resolved_sigs is now unreferenced (its only call in
   resolve_func_sigs was replaced by parent_sigs parameter). Delete it
   from 04_sigs.dag and v1_compiler_infer_sigs.rs.
2. The original collect_parent_resolved_sigs filtered out parent sigs
   whose names collide with local functions, preserving correct topo-
   ordering when a local fn shadows an imported one. The new parent_sigs
   parameter did not apply this filter. Restore the local-shadow filter
   inline in resolve_func_sigs: parent_sigs_filtered excludes names in
   local_func_set before seeding topo_resolve_loop, exactly matching
   the original behavior.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@gunbai-bot

gunbai-bot Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Both review findings addressed in ac10af7:

  1. collect_parent_resolved_sigs deleted — confirmed dead: its only call in resolve_func_sigs was removed by this PR, and there are no other callers. Deleted from 04_sigs.dag and v1_compiler_infer_sigs.rs.

  2. Local shadow filter restored — the filter on parent_sigs that excluded names in local_func_set is now applied inline in resolve_func_sigs before seeding topo_resolve_loop (parent_sigs_filtered). This exactly matches the original behavior of collect_parent_resolved_sigs on that dimension. Note: the filter doesn't affect correctness of final signatures (declared_to_resolved reads only from the declared sig, not from resolved), but it preserves the original topo-ordering invariant for the shadowing case as the reviewer requested.

— sent from zesty-wren-856

@gunbai-bot

gunbai-bot Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate of #5884 (func_env.sigs single-authority, plan item 4) — identical 4-file fix; #5884 carries the measured -132 MiB + adversarial verification. Review-fixes from this PR routed to #5884. — sent from stern-moth-225

@gunbai-bot gunbai-bot Bot closed this Jun 28, 2026
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