Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/6812-w16-builder-identity.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
perf(object): #6812 w16 — builder-pattern objects get a learnable per-site identity. Empty object literals lower to a unique 0-field anon-shape class per source site (content-addressed on file + offset), so the runtime's learned inline sizing and the static-key write PIC admit `{}`-built objects; the compiled-`new` allocator consumes the learned high-water width. A constant-bounded dynamic-key build loop additionally proves its width at compile time (`Class::alloc_width_hint`), right-sizing even the first instance so arrays of builder objects stay uniform and whole-loop clone eligible. w16 (overflow-slot writes): 173× slower than node → beats node.
3 changes: 3 additions & 0 deletions crates/perry-codegen/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,9 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
let stub = perry_hir::Class {
id: 0, // imported — no local ClassId
name: effective_name.to_string(),
// #6812: width hints don't cross module metadata; imported stubs
// fall back to runtime learned sizing.
alloc_width_hint: 0,
type_params: Vec::new(),
extends: None,
extends_name: ic.parent_name.clone(),
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/src/collectors/class_accessors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ mod tests {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
static_accessor_names: Vec::new(),
static_accessor_fn_ids: Vec::new(),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ mod tests {
decorators: Vec::new(),
is_exported: false,
is_nested: false,
alloc_width_hint: 0,
aliases: Vec::new(),
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/src/collectors/this_as_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ mod tests {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
static_accessor_names: Vec::new(),
static_accessor_fn_ids: Vec::new(),
}
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/src/lower_call/buffer_intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,7 @@ mod shadow_scan_tests {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
}
}

Expand Down
10 changes: 10 additions & 0 deletions crates/perry-codegen/src/lower_call/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,16 @@ fn lower_new_impl(
if let Some(&authoritative) = ctx.class_field_counts.get(class_name) {
field_count = authoritative;
}
// #6812 (w16): a per-site empty-literal anon-shape class may carry a
// compile-time proven builder width. Allocate that many inline slots so
// the FIRST instance of the site is as wide as the runtime-learned
// resizes make every later one — a lone under-sized first instance
// permanently vetoes whole-loop clone eligibility for arrays built at
// the site. Capacity only: the keys array stays authoritative for
// enumeration, and the runtime treats header field_count as alloc_limit.
if class.alloc_width_hint > field_count {
field_count = class.alloc_width_hint;
}

// Allocate the object with the per-class id and (if applicable)
// parent class id, so the runtime registers the inheritance
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/type_analysis_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ fn hir_inferred_types_reuse_codegen_contextual_class_facts() {
decorators: Vec::new(),
is_exported: false,
is_nested: false,
alloc_width_hint: 0,
aliases: Vec::new(),
};
let widget = perry_hir::Class {
Expand Down Expand Up @@ -284,6 +285,7 @@ fn hir_inferred_types_reuse_codegen_contextual_class_facts() {
decorators: Vec::new(),
is_exported: false,
is_nested: false,
alloc_width_hint: 0,
aliases: Vec::new(),
};
let classes = HashMap::from([("Base".to_string(), &base), ("Widget".to_string(), &widget)]);
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/tests/class_keys_gc_root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ fn module_with_declared_field_class() -> Module {
decorators: Vec::new(),
is_exported: false,
is_nested: false,
alloc_width_hint: 0,
aliases: Vec::new(),
}],
interfaces: Vec::new(),
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/tests/constructor_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ fn module_with_recursive_constructor_return() -> Module {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
}],
interfaces: Vec::new(),
type_aliases: Vec::new(),
Expand Down
12 changes: 10 additions & 2 deletions crates/perry-codegen/tests/native_proof_buffer_views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ fn class(id: u32, name: &str, fields: Vec<ClassField>) -> Class {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
}
}

Expand Down Expand Up @@ -1554,9 +1555,16 @@ fn native_owned_uint8array_get_fallback_uses_uint8array_helper() {
})),
],
);
// Codegen migrated the disposed-view fallback from `js_uint8array_get`
// to `js_uint8array_index_get_value` (#6088-era JS-value getter), which
// validates the address against the typed-array kind registry before any
// dereference and returns undefined for dead views — the memory-safety
// property this test exists to pin. The suite was not CI-selected when
// that landed, so the old helper name went stale here.
assert!(
ir.contains("call i32 @js_uint8array_get"),
"disposed native Uint8Array fallback should call js_uint8array_get:\n{ir}"
ir.contains("call double @js_uint8array_index_get_value"),
"disposed native Uint8Array fallback should call the registry-validating \
js_uint8array_index_get_value:\n{ir}"
);
assert!(
!ir.contains("call i32 @js_buffer_get"),
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/tests/native_proof_regressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ fn class(id: u32, name: &str, fields: Vec<ClassField>) -> Class {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ fn class(id: u32, name: &str) -> Class {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/tests/static_symbol_hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ fn class_with_static(id: u32, value: f64) -> Class {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
}
}

Expand Down
27 changes: 22 additions & 5 deletions crates/perry-codegen/tests/typed_feedback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ fn class(id: u32, name: &str, fields: Vec<ClassField>) -> Class {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
}
}

Expand Down Expand Up @@ -217,11 +218,27 @@ fn typed_feedback_trace_dump_runs_before_entry_return() {
));

assert!(ir.contains("declare void @js_typed_feedback_maybe_dump_trace()"));
let dump_pos = ir
.rfind("call void @js_typed_feedback_maybe_dump_trace()")
.expect("entry should call typed-feedback trace dump");
let ret_pos = ir.rfind("ret i32 0").expect("entry should return i32 0");
assert!(dump_pos < ret_pos);
// The entry epilogue now has TWO exit paths: the host-return path
// (`ret i32 0`) and the event-loop exit path, which returns the dynamic
// `js_process_pending_exit_code` result (`ret i32 %rN`). The old
// rfind(dump) < rfind("ret i32 0") comparison broke the day the second
// path appeared — its dump call sits after the literal `ret i32 0`. The
// real invariant is stronger: EVERY entry return must be immediately
// preceded by the trace dump, so no exit path can skip the dump.
let mut ret_count = 0;
let mut search_from = 0;
while let Some(rel) = ir[search_from..].find("ret i32") {
let ret_pos = search_from + rel;
ret_count += 1;
let preceding = &ir[ret_pos.saturating_sub(200)..ret_pos];
assert!(
preceding.contains("call void @js_typed_feedback_maybe_dump_trace()"),
"entry return at byte {ret_pos} is not preceded by the typed-feedback \
trace dump:\n...{preceding}"
);
search_from = ret_pos + 1;
}
assert!(ret_count >= 1, "entry should return i32");
Comment on lines +221 to +241

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert adjacency instead of searching a 200-byte window.

preceding.contains(...) can match a dump belonging to an earlier exit path, allowing a later ret i32 without its own dump to pass. It also makes the test depend on unrelated IR size. Check the immediately preceding non-empty instruction, or parse the entry-function control flow, for each return.

Proposed assertion tightening
-    let mut search_from = 0;
-    while let Some(rel) = ir[search_from..].find("ret i32") {
-        let ret_pos = search_from + rel;
+    let mut previous_nonempty = None;
+    for line in ir.lines() {
+        if line.trim_start().starts_with("ret i32") {
         ret_count += 1;
-        let preceding = &ir[ret_pos.saturating_sub(200)..ret_pos];
-        assert!(
-            preceding.contains("call void `@js_typed_feedback_maybe_dump_trace`()"),
-            "entry return at byte {ret_pos} is not preceded by the typed-feedback \
-             trace dump:\n...{preceding}"
-        );
-        search_from = ret_pos + 1;
+            assert_eq!(
+                previous_nonempty,
+                Some("call void `@js_typed_feedback_maybe_dump_trace`()"),
+                "entry return is not immediately preceded by the typed-feedback trace dump"
+            );
+        }
+        if !line.trim().is_empty() {
+            previous_nonempty = Some(line.trim());
+        }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// The entry epilogue now has TWO exit paths: the host-return path
// (`ret i32 0`) and the event-loop exit path, which returns the dynamic
// `js_process_pending_exit_code` result (`ret i32 %rN`). The old
// rfind(dump) < rfind("ret i32 0") comparison broke the day the second
// path appeared — its dump call sits after the literal `ret i32 0`. The
// real invariant is stronger: EVERY entry return must be immediately
// preceded by the trace dump, so no exit path can skip the dump.
let mut ret_count = 0;
let mut search_from = 0;
while let Some(rel) = ir[search_from..].find("ret i32") {
let ret_pos = search_from + rel;
ret_count += 1;
let preceding = &ir[ret_pos.saturating_sub(200)..ret_pos];
assert!(
preceding.contains("call void @js_typed_feedback_maybe_dump_trace()"),
"entry return at byte {ret_pos} is not preceded by the typed-feedback \
trace dump:\n...{preceding}"
);
search_from = ret_pos + 1;
}
assert!(ret_count >= 1, "entry should return i32");
// The entry epilogue now has TWO exit paths: the host-return path
// (`ret i32 0`) and the event-loop exit path, which returns the dynamic
// `js_process_pending_exit_code` result (`ret i32 %rN`). The old
// rfind(dump) < rfind("ret i32 0") comparison broke the day the second
// path appeared — its dump call sits after the literal `ret i32 0`. The
// real invariant is stronger: EVERY entry return must be immediately
// preceded by the trace dump, so no exit path can skip the dump.
let mut ret_count = 0;
let mut previous_nonempty = None;
for line in ir.lines() {
if line.trim_start().starts_with("ret i32") {
ret_count += 1;
assert_eq!(
previous_nonempty,
Some("call void `@js_typed_feedback_maybe_dump_trace`()"),
"entry return is not immediately preceded by the typed-feedback trace dump"
);
}
if !line.trim().is_empty() {
previous_nonempty = Some(line.trim());
}
}
assert!(ret_count >= 1, "entry should return i32");
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/tests/typed_feedback.rs` around lines 221 - 241, Update
the return validation loop in the typed-feedback test to verify that each `ret
i32` is immediately preceded by the trace-dump call, ignoring only whitespace or
empty lines. Remove the fixed 200-byte `preceding.contains` window so an earlier
exit path’s dump cannot satisfy a later return; preserve the existing
requirement that the entry contains at least one return.

}

#[test]
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/tests/typed_shape_descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ fn class(id: u32, name: &str, fields: Vec<ClassField>) -> Class {
is_exported: false,
aliases: Vec::new(),
is_nested: false,
alloc_width_hint: 0,
}
}

Expand Down
5 changes: 5 additions & 0 deletions crates/perry-hir/src/analysis/value_types_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@ fn seeds_contextual_class_and_enum_facts_from_module() {
decorators: Vec::new(),
is_exported: false,
is_nested: false,
alloc_width_hint: 0,
aliases: Vec::new(),
});

Expand Down Expand Up @@ -694,6 +695,7 @@ fn infers_named_class_and_interface_property_facts() {
decorators: Vec::new(),
is_exported: false,
is_nested: false,
alloc_width_hint: 0,
aliases: Vec::new(),
});
module.classes.push(Class {
Expand All @@ -718,6 +720,7 @@ fn infers_named_class_and_interface_property_facts() {
decorators: Vec::new(),
is_exported: false,
is_nested: false,
alloc_width_hint: 0,
aliases: Vec::new(),
});
module.interfaces.push(Interface {
Expand Down Expand Up @@ -1661,6 +1664,7 @@ fn resolves_this_and_super_in_class_context() {
decorators: Vec::new(),
is_exported: false,
is_nested: false,
alloc_width_hint: 0,
aliases: Vec::new(),
});
module.classes.push(Class {
Expand All @@ -1685,6 +1689,7 @@ fn resolves_this_and_super_in_class_context() {
decorators: Vec::new(),
is_exported: false,
is_nested: false,
alloc_width_hint: 0,
aliases: Vec::new(),
});

Expand Down
10 changes: 10 additions & 0 deletions crates/perry-hir/src/ir/decl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,16 @@ pub struct Class {
/// inside a turbopack factory threw at module init). Codegen
/// (`init_static_fields_*`) skips module-init static init for these.
pub is_nested: bool,
/// #6812 (w16): minimum inline slot count to allocate for instances,
/// beyond `fields.len()`. Set only on per-site empty-literal anon-shape
/// classes when the lowering can prove the builder's final width (e.g. a
/// `{}` declarator followed by a constant-bounded single-write build
/// loop), so even the FIRST instance allocates every field inline
/// instead of spilling to the overflow side-table and permanently
/// poisoning whole-loop clone eligibility for arrays built at that
/// site. Pure capacity: does not add fields, keys, or enumeration
/// entries. 0 = no hint.
pub alloc_width_hint: u32,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand Down
Loading
Loading