Skip to content

perf(arguments): keep strict callee descriptors in object slots (#10509) - #11711

Open
proggeramlug wants to merge 3 commits into
PerryTS:mainfrom
proggeramlug:perf/arguments-strict-descriptors
Open

proggeramlug wants to merge 3 commits into
PerryTS:mainfrom
proggeramlug:perf/arguments-strict-descriptors

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Advances #10509 for strict escaping arguments objects.

  • Use the existing canonical key layout for length and callee attributes instead of installing their descriptors on every allocation.
  • Store the restricted callee getter/setter pair in the object's own accessor slot, with the accessor read gate enabled.
  • Assert that construction creates no address-keyed property or accessor descriptor entries and that the accessor survives a moving GC.
  • Remove a stale scoped test name from the current main CI map so the required e2e-scoped job can run. perf: GC scope contexts and captured-binding fixes (#10500, #10703, #10520) #11710 independently carries the same correction.

This removes per-call descriptor side-table work; it adds no side tables and makes no version bump. The remaining cost of escaping arguments and the issue's performance target still need measurement and follow-up. #11710 covers the separate box registry and capture-count machinery.

Verification

  • cargo check -p perry-runtime passed.
  • python3 scripts/ci_e2e_scope.py --self-test passed.
  • git diff --check passed.
  • Focused runtime unit test compilation was stopped before linking when this workstation ran out of disk space; CI is running the tests.

Summary by CodeRabbit

  • Bug Fixes
    • Strict-mode arguments objects now retain the expected length and restricted callee properties without separate per-object descriptor entries.
    • The restricted callee remains non-writable, non-enumerable, and non-configurable after garbage collection moves the object.
  • Tests
    • Added coverage for descriptor handling and moving garbage collection of restricted arguments objects.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: fe882f73-1c7e-443f-b833-71010a7db225

📥 Commits

Reviewing files that changed from the base of the PR and between 5fbc2c3 and 953e34c.

📒 Files selected for processing (5)
  • changelog.d/11711-strict-arguments-descriptors.md
  • crates/perry-runtime/src/gc/tests/arguments_objects.rs
  • crates/perry-runtime/src/object/arguments.rs
  • crates/perry-runtime/src/object/descriptor_state.rs
  • scripts/ci_e2e_scope.py
💤 Files with no reviewable changes (1)
  • scripts/ci_e2e_scope.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Restricted arguments-object allocation now records the callee accessor through the canonical key layout instead of adding per-object descriptor entries. Tests cover descriptor-table entries and accessor attributes after moving GC. The codegen source-to-suite map no longer includes the typed-shape allocation suite.

Changes

Restricted arguments objects

Layer / File(s) Summary
Initialize and validate the callee accessor
crates/perry-runtime/src/object/descriptor_state.rs, crates/perry-runtime/src/object/arguments.rs, crates/perry-runtime/src/gc/tests/arguments_objects.rs, changelog.d/11711-strict-arguments-descriptors.md
Restricted arguments-object allocation stores the thrower getter and setter in the callee slot and records accessor bookkeeping for canonical keys. Tests check descriptor-table entries and callee attributes after moving GC. The changelog describes the layout and removal of per-object descriptor entries.

CI codegen suite mapping

Layer / File(s) Summary
Update the codegen suite map
scripts/ci_e2e_scope.py
The source-to-suite map no longer selects the typed-shape allocation suite for codegen source changes. If the test target remains without an exclusion, the map-coverage check treats it as unclassified.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to 953e3

The restricted arguments layout has matching descriptor and moving-GC checks, and the CI change removes a stale suite reference. No actionable merge-blocking risk remains after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 953e3

The inspected paths preserve restricted callee access and keep accessor values private to each object. No introduced security issue was identified, but focused runtime tests have not completed and security coverage remains incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The directly supported exposure is property access and descriptor mutation of restricted arguments objects within the runtime. The supplied evidence does not establish a tenant, service, credential, or deployment-level boundary change.

Trust Boundaries and Controls

  • observed — Restricted callee has explicit throwing read/write hooks and a deletion rejection. Generic non-configurable redefinition enforcement obtains attributes and accessors through the existing lookup functions; its validator rejects configurable or enumerability changes and conversion between accessor and data properties.

Resilience and Maintainability Implications

  • observed — Construction roots the receiver and input values. Accessor installation runs in a no-move window; pair allocation roots both closures, rebuilds the array's GC slot layout, and applies old-generation write barriers. Existing cleanup handles key-resident accessors separately from table-backed descriptor ownership.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the performance change for strict arguments objects and matches the main changeset.
Description check ✅ Passed The description explains the purpose, implementation changes, related issues, and verification results. It omits some template headings and checklist items, but it provides the required information an…
Docstring Coverage ✅ Passed Docstring coverage is 87.50% which is sufficient. 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 u…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🧪 Generate unit tests (beta)
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

This branch has not been deployed

No deployments
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