perf(runtime): every function object carries a shape (every-shape part 1) - #11580
Merged
Merged
Conversation
|
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 configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughFunction closures now carry ShapeIds in an expanded header. The runtime assigns base function shapes and moves closures with additional own state to a dictionary shape. Closure checks use GC headers. Function-method dispatch handles ChangesFunction Object Runtime
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant NativeCallMethod
participant FunctionShapeDispatch
participant CommonMethodDispatch
participant ClosureAllocator
NativeCallMethod->>FunctionShapeDispatch: pass receiver, method name, and arguments
FunctionShapeDispatch->>CommonMethodDispatch: dispatch bind, call, or apply when the prototype slot matches
CommonMethodDispatch->>ClosureAllocator: allocate bound closure for bind
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable risk remains in the reviewed changes; the PR is mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The change spans several core runtime behaviors. The reviewed dispatch path retains checks on object ownership, function shape, and prototype method identity, but the breadth of the change and incomplete comparison with the stacked base warrant design review. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
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. Comment |
proggeramlug
pushed a commit
that referenced
this pull request
Sep 27, 2026
proggeramlug
pushed a commit
that referenced
this pull request
Sep 27, 2026
proggeramlug
force-pushed
the
perf-function-shapes
branch
from
September 27, 2026 20:09
bcbd8ec to
e18e175
Compare
added 13 commits
September 27, 2026 23:56
A closure is now {capture_count u32, ShapeId u32, func_ptr, props} with the
ShapeId at payload +4, the word an ObjectHeader keeps its ShapeId in. The
CLOSURE_MAGIC payload word is gone: what a cell IS comes from its GcHeader
type byte, and is_closure_ptr proves ownership (tracked header) behind a
one-load exotic-band prefilter on the +4 word.
Function shapes are minted in a new EXOTIC ShapeId band that no own-slot
site word accepts. A closure is born with the base Function shape of its
body kind (proto_id = the intrinsic serial of Function.prototype /
%AsyncFunction.prototype% / %(Async)GeneratorFunction.prototype%, reserved
1..4 and assigned at creation) and leaves it for the shared
FunctionDictionary shape at every funnel that installs something the base
shape does not describe (a non-intrinsic key, a symbol key, an accessor, a
delete, a recorded [[Prototype]]).
…he shape names A method call on a function object that is still on its base Function shape compares the receiver's +4 word with this agent's base ShapeId (ownership then proven by the tracked GC header), reads the key's data slot on the Function.prototype that shape names, and — when that slot's VALUE is still the intrinsic bind/call/apply closure — runs exactly what the by-name tower runs for those names (now one unit, dispatch_function_proto_method), before any of the tower's probes. A patched slot, an accessor, a missing key or a FunctionDictionary receiver declines to the unchanged full path.
…r, audit markers, holder verdicts)
… in a capture js_function_bind wrote its four captures through the per-slot setters, so every bind installed a per-object layout mask (4 >= the mask threshold) and a builtin-closure-length metadata entry, both of which every minor then pruned. The bound closure now has a fifth capture holding its .length (read back through builtin_closure_length, the one reader every .length path uses), its captures are installed in one step (closure_install_boxed_captures: raw stores, GC_LAYOUT_UNKNOWN when any word can be a pointer — the #7630 state for a payload a mask cannot improve on — and one newborn barrier), and the length is resolved from the rooted target before the closure exists. A non-u32 length stays an own property, as before.
…-class table The tracked resolver range search measured 17% of a fn.length read. An arena cell is proven by classify_heap_generation (the page-class table, the proof the pre-shape predicate used); only a cell in no arena takes the malloc-registry resolver. builtin_closure_length consults its table before the bind-capture check, so a non-bound function pays no extra probe.
…d-key probes from its shape Every accessor installer (set_accessor_descriptor, install_fresh_accessor_property, set_builtin_accessor_descriptor) and every delete now moves a closure to FunctionDictionary, so a closure still on its base Function shape provably has neither: closure_get_dynamic_prop skips both side-table probes for it. The bound length re-check is the header byte only (callers proved the closure).
…e method arm on one compare A function now shows a ShapeId at +4, so the inherited-read cache hashed and probed for it before failing; it declines an exotic-band id up front. The function-shape method arm tests the band before loading the agent base id, so a Map/array/object receiver leaves on one compare. closure_on_base_shape is one compare against the FunctionDictionary id.
- a test's unsafe block around a now-safe call;
- trusted_box_callback_tests reads the capture offset from perry-abi
(CLOSURE_HEADER_SIZE, 24) instead of the old literal 16;
- the two JSON-reviver copied-minor tests register the object model's
production scanners. The copying-nursery guard takes the thread's registry
away; without the shape-table scanner a copied minor left the parse's shape
families under from-space addresses, and once the nursery reused one the
post-minor prune retired the live object's shape ("b" vanished from
{"a":[{"b":"c"}]}). Which address is reused depends on cell sizes, so the
24-byte closure header exposed a gap that was already there.
…ut of its cell Every closure birth and every function-receiver test asks for one of the agent's base Function ShapeIds (or the FunctionDictionary id). base_slot copied the whole five-id array out of its thread-local cell per call, and showed up at ~3% of Zod's instructions. It now reads the one element in place and keeps the mint on a cold path. Zod (perrymaster, real release profile, this series' part 2): 1,160.9M -> 1,145.9M instructions (-1.3%); fn.bind per op 3,732 -> 3,605.
proggeramlug
force-pushed
the
perf-function-shapes
branch
from
September 27, 2026 21:57
e18e175 to
a86c2bc
Compare
proggeramlug
pushed a commit
that referenced
this pull request
Sep 28, 2026
…ure births) Since the last regeneration, closure birth mints a shape descriptor (#11580/#11581): js_closure_alloc -> birth_shape_for_body -> mint -> shape_descriptor_ensure_with_holes -> keys_attrs, and keys_attrs' forwarded-keys arm calls clean_arr_ptr, whose full resolver force-materializes a lazy JSON array (a reparse that reaches js_object_set_field_by_name's indirect call). The graph therefore demotes js_closure_alloc{,_init,_singleton,_with_captures_singleton}, js_closure_unbind_this, js_console_log_as_closure, js_domain_{bind,intercept} and js_v8_promise_hook_register from AllocOnly, and (macOS) four ThrowOnly callers, to Reenters. No symbol lost Leaf; the S2 fast hits stay Leaf. In default codegen AllocOnly and ThrowOnly are already non-leaf, so emitted code is unchanged. Twelve new js_stdlib_install_* registration exports come out Leaf. Tables are the gc-call-effects jobs' own regenerations (run 36380097760) from refs/pull/11565/merge c9a053f, which is tree-identical to this branch's parent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #11489. Review after it merges; the diff here is this PR's 12 commits only. Refs #10502, #11394.
What
Every function object now carries a real ShapeId, like any other object. The function header is
capcount | ShapeId @+4 | code @+8 | props @+16, a 24-byte header.CLOSURE_MAGICis gone: a function's kind comes from its GC header type byte, and the method site recognizes a closure by that byte plus the code pointer at +8.perry-abicrate: the closure layout, GC type constants, andMETHOD_SITE_FUNCTION_BAG(bit 61).With a shape, the questions the runtime used to answer by walking side tables become one compare:
fn.bind/call/applyare decided by the prototype slot the shape names.bindresult is born with one tag-checked allocation, with its length in a capture.is_closure_ptrproves arena ownership through the page-class table.Evidence
Measured n=5 interleaved on a release build, each arm linking its own runtime, against base
d548daaaf(#11489). Spread was at most 0.38%, and output matches node in every arm.fn.bindper callbindcalls take the new path.--test-threads=1: 4,676 passing vs 4,665 on base. Both arms share one failure, a DNS test that fails on the build host's network.run_lint_gates.sh: 2 of 108 fail, the Windows xwin check (not installed on the host) and public-benchmark freshness, which fails on main too. This includes-D warnings, clippy, API docs and GC root-dominance.cargo fmt --checkis clean.Summary by CodeRabbit
bind,call, andapply.lengthwhen it falls within the supported range.fn.bind.