Skip to content

fix(codegen): require dense storage for static numeric array proofs - #9803

Closed
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9784-module-presized-array-growth
Closed

fix(codegen): require dense storage for static numeric array proofs#9803
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/9784-module-presized-array-growth

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Closes #9784.

A static new Array(1000001) could receive an unchecked numeric-array store even though the runtime allocates only a small initial backing store above one million elements. The compiler's Ptr<NumArray> proof accepted lengths up to 16 million and confused logical length with allocated capacity. Module constants expose that proof; the original function-parameter example does not.

Restrict the proof to the runtime's fresh dense allocation limit. Larger arrays keep the existing guarded paths, which grow their backing storage as needed. No runtime allocation policy or version change.

The regression adds module-scope fills at 1,000,000, 1,000,001, and 1,200,000 elements, plus a function with a literal length. It extends issue_9371_large_presized_array.rs and retains its moving-GC runs, expando, object-element, and genuinely sparse controls. The test has a bounded execution timeout so a regression cannot hang CI. Collector coverage checks literal and module-constant provenance at the boundary.

Evidence and validation:

  • Before the fix, the original module fixture timed out at 30 seconds; the function form completed in 0.45 seconds. Disabling only PERRY_PTR_NUMARRAY_LOCALS made the module fixture complete in 0.51 seconds.
  • The expanded regression crashes on the original compiler after printing the valid 1,000,000-element control. Saved LLVM IR shows the unchecked store with no capacity test.
  • With the fix, three native runs of the original function/module cases match Node byte for byte: function 0.477/0.037/0.045 seconds; module 0.473/0.031/0.045 seconds.
  • The expanded fixture passes the repository Node parity runner.
  • Full codegen suite: 1,411 passed, 1 ignored. Quick pre-tag checks passed.
  • The expanded compiler integration test passed in both normal and moving-GC modes.
  • Canonical affected-crate runner against branch base 12efed1222: runtime 3,137 passed / 4 ignored; CLI 1,083 passed and the existing PERRY_CONCAT_SITE_CACHE audit failure addressed by fix(cache): register concat switch and explain codegen inputs #9748. The codegen suite was run separately because the runner stops at that CLI failure.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue affecting large pre-sized arrays, ensuring indexed writes and reads remain safe and accurate when array lengths exceed the runtime’s dense allocation threshold.
    • Improved handling for both module-level and function-local large array allocations.
    • Added safeguards for arrays with lengths around and above the supported dense-storage limit.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: ebc1bea4-f9e4-41d5-92a2-0f1dcb3d2caa

📥 Commits

Reviewing files that changed from the base of the PR and between 1d63fa9 and 22c257e.

📒 Files selected for processing (4)
  • changelog.d/9803-module-presized-array-growth.md
  • crates/perry-codegen/src/collectors/ptr_numarray.rs
  • crates/perry/tests/issue_9371_large_presized_array.rs
  • test-files/test_gap_9784_module_presized_array.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The compiler now limits static numeric-array promotion to lengths supported by fresh dense storage. Regression coverage adds module-level arrays at and above the 1,000,000-element limit, timeout handling, expected output checks, and a changelog entry.

Changes

Presized array growth

Layer / File(s) Summary
Allocation proof limit
crates/perry-codegen/src/collectors/ptr_numarray.rs
Static allocation proofs now accept lengths through 1,000,000 and reject larger lengths. Tests cover literal and module-level constant allocations at the boundary.
Module-scope regression fixture
test-files/test_gap_9784_module_presized_array.ts
The fixture fills and validates arrays with 1,000,000, 1,000,001, and 1,200,000 slots, plus a function-local literal allocation above the limit.
Compiled-test integration
crates/perry/tests/issue_9371_large_presized_array.rs, changelog.d/9803-module-presized-array-growth.md
The compiled test includes the shared fixture, checks its output, uses a 30-second timeout, and documents the fix.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 22c25

Large statically sized numeric arrays now retain guarded growth when their logical length exceeds fresh dense capacity, preventing the reported module-scope slowdown and invalid unchecked stores. Threshold and runtime regression coverage support merge readiness.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: requiring dense storage for static numeric-array proofs in code generation.
Description check ✅ Passed The description is substantially complete. It explains the defect, the fix, related issue #9784, regression coverage, and validation results. It does not use the template headings or checklist format,…
Linked Issues check ✅ Passed The changes satisfy issue #9784. They restrict static numeric-array proofs to the 1,000,000-element dense allocation limit, preserve guarded growth for larger arrays, add module-scope coverage at the …
Out of Scope Changes check ✅ Passed The code, regression fixture, timeout support, and changelog entry all support the linked issue and stated fix. No unrelated code changes are identified.
Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main via merge train #9817 (rebase-merged, so your commits keep their authorship). Thanks!

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.

new Array(n) above 1M is still quadratic when the array is a module-level binding

1 participant