Skip to content

perf(checker): size declaration storage by unique entries - #46

Closed
Unclip1843 wants to merge 1 commit into
pingdotgg:mainfrom
Unclip1843:perf-unique-declaration-storage
Closed

Unclip1843 wants to merge 1 commit into
pingdotgg:mainfrom
Unclip1843:perf-unique-declaration-storage

Conversation

@Unclip1843

@Unclip1843 Unclip1843 commented Oct 11, 2026 •

Copy link
Copy Markdown

create_union_or_intersection_property reserves declaration storage using the sum of declarations across constituent properties, before deduplication. When many constituents repeat the same declarations, the resulting symbol can retain a buffer sized for all occurrences rather than the unique list.

Start the inline SmallVec empty and pass zero as the existing DeclarationSet size hint. Both structures grow with the collected unique declarations instead of the occurrence estimate. Rust scope: +5 / -6 lines in one file.

Why behavior stays the same

The same DeclarationSet::add calls keep the same append-if-unique order. Its existing growth path supports underestimated hints: declaration_set_grows_past_its_hint explicitly tests hint 0 with 300 distinct nodes, duplicates and nil entries. Nil handling and conversion to the retained Declarations are unchanged. Mostly unique inputs can cause additional growth/rehashing; an isolated performance comparison is pending.

Lean source proves ordered unique collection and a unique-count capacity bound in the retained-buffer model. Fresh Lean 4.33.1 verification passes. The temporary hash table can still grow on a duplicate at its load threshold; this PR does not claim zero allocation for every duplicate operation.

Validation and evidence

  • Applied independently to bf34f21ae40b221ea9def389778d2f90ec950398; changed files pass rustfmt with Rust 1.93.0. Independent static review is attached.
  • Evidence package, review, and audit, with source/binary hashes and complete per-case results.
  • Historical combined candidate: 133,390 compiler/conformance checks passed; 1,728 explicit skips. The default suite had 1,529 passes plus two API-session failures that passed on the same binary after a macOS socket-path correction. npm: 879 passes on each side. Workspace: 629 passes and one unchanged watch failure. Original failures and skips remain in the attached records.
  • Those results used the earlier parent 9f6ee6d147de1f8216c967e2a966cacbac295984 and included other optimizations. They are not test results for this independent PR head. No standalone speedup or memory percentage is claimed.

Draft pending the light-path checks in docs/typechecker-accountability.md: existing focused tests, protected goport comparison against the latest accepted revision with no lost passes, and byte-equal Query/Hono/zod/effect output. Exact-head Cargo build/runtime checks, clippy and CI remain pending. This follows the validation pattern in #31; the attached evidence preserves what has actually run.

Note

Size Checker::create_union_or_intersection_property declaration storage by unique entries

Removes the aggregate declaration-count calculation in checker_p24.rs that summed declaration occurrences across constituent properties. The declaration buffer now grows based on the unique-declaration collection, and DeclarationSet::add receives zero instead of the occurrence count. Declaration ordering and uniqueness logic is unchanged.

Macroscope summarized fe33363.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@t3dotgg

t3dotgg commented Oct 11, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Thank you for this PR and for the proof. The dedupe order is unchanged, and we confirmed byte-equal output and no lost goport tests on current main. But the old size hint is there for one known case. On blueprint, 42,921 calls each collect 178 unique declarations from 181. Without the hint, the table and the buffer grow step by step, and the kept buffer rounds up to 256 entries. On that project this change measured +4.4 percent instructions and +7.1 percent peak memory. On Query, Hono, zod, Effect and the T3 Code server it had no measurable effect. So we will close it. If you find a project where duplicate declarations keep a lot of slack, please share it. A shrink only in that case could be worth a look.

@t3dotgg t3dotgg closed this Oct 11, 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.

2 participants