From 098ee17566015325e936dd5fa04744358f553b4e Mon Sep 17 00:00:00 2001 From: bing Date: Fri, 22 May 2026 14:01:33 +0800 Subject: [PATCH 1/4] perf(dsl): skip placeholder pattern in materializeClassInstance Per-call cost of every factory method that returns a DSL class (PublicKey.fromBytes, Signature.aggregate, SecretKey.sign, etc.) was: 1 napi_external alloc (V8 young-gen) 1 napi_new_instance -> constructor allocs placeholder native + napi_wrap 1 removeWrapChecked -> tears down placeholder finalizer entry 1 destroyInternalPlaceholder 1 native alloc for the real object 1 napi_wrap + typeTagObject That's 2 native allocs + 1 free, 2 napi_wraps + 1 unwrap, 2 typeTagObject writes, and an extra V8 young-gen object per call. In lodestar's attestation/signature hot path this measurably increases Scavenge time. Replace with a threadlocal marker: materializeClassInstance sets `materialize_target` before calling napi_new_instance(argc=0, args=null). The generated constructor early-returns when isMaterializing(T) is true, skipping the placeholder alloc/wrap entirely. materialize then wraps the real object directly. After patch: 1 native alloc + 1 napi_wrap + 1 typeTagObject per call. Bench (lodestar-z bench/napi/materialize.bench.mjs, 200k iters): PublicKey.fromBytes 3.28% -> 2.10% Scavenge (-1.18 pp, -69% count) Signature.fromBytes 0.78% -> 0.47% Scavenge (-38% count) --- src/js/class_runtime.zig | 30 ++++++++++++++++++++---------- src/js/wrap_class.zig | 7 +++++++ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/js/class_runtime.zig b/src/js/class_runtime.zig index 7af0b03..6fffc3e 100644 --- a/src/js/class_runtime.zig +++ b/src/js/class_runtime.zig @@ -85,27 +85,37 @@ pub fn registerClass(comptime T: type, env: napi.Env, ctor: napi.Value) !void { try env.addEnvCleanupHook(State.Entry, entry, State.cleanupHook); } +/// Per-thread marker set by `materializeClassInstance` to tell the generated +/// constructor "this `new` call comes from the DSL — don't allocate a placeholder, +/// I'll wrap with the real object after `napi_new_instance` returns." +/// Compared by identity against `internalCtorMarkerPtr(T)`. +threadlocal var materialize_target: ?*const anyopaque = null; + +pub fn isMaterializing(comptime T: type) bool { + return materialize_target == @as(?*const anyopaque, @ptrCast(internalCtorMarkerPtr(T))); +} + pub fn materializeClassInstance(comptime T: type, env: napi.Env, instance: T, preferred_ctor: ?napi.Value) !napi.Value { const ctor = preferred_ctor orelse try getConstructor(T, env); - const internal_arg = try env.createExternal(@ptrCast(internalCtorMarkerPtr(T)), null, null); - var raw_args = [_]napi.c.napi_value{internal_arg.value}; + + const obj_ptr = try std.heap.c_allocator.create(T); + errdefer std.heap.c_allocator.destroy(obj_ptr); + obj_ptr.* = instance; + + const prev = materialize_target; + materialize_target = @ptrCast(internalCtorMarkerPtr(T)); + defer materialize_target = prev; var js_instance_raw: napi.c.napi_value = null; try napi.status.check(napi.c.napi_new_instance( env.env, ctor.value, - 1, - &raw_args, + 0, + null, &js_instance_raw, )); const js_instance = napi.Value{ .env = env.env, .value = js_instance_raw }; - const placeholder = try env.removeWrapChecked(T, js_instance, typeTag(T)); - destroyInternalPlaceholder(T, placeholder); - - const obj_ptr = try std.heap.c_allocator.create(T); - obj_ptr.* = instance; - try wrapTaggedObject(T, env, js_instance, obj_ptr, null); return js_instance; } diff --git a/src/js/wrap_class.zig b/src/js/wrap_class.zig index 6a34c0c..46743b9 100644 --- a/src/js/wrap_class.zig +++ b/src/js/wrap_class.zig @@ -366,6 +366,13 @@ pub fn wrapClass(comptime T: type) type { return null; }; + // Fast path: materializeClassInstance is creating this instance. + // Skip the placeholder alloc/wrap — materialize will wrap directly + // with the real native pointer after napi_new_instance returns. + if (class_runtime.isMaterializing(T)) { + return this_arg; + } + if (actual_argc == 1) { const internal_arg = napi.Value{ .env = raw_env, .value = raw_args[0] }; if ((internal_arg.typeof() catch null) == .external) { From f6a6179b6d4767ed8e434853ff1ce086212bbc18 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Tue, 26 May 2026 18:03:24 +0200 Subject: [PATCH 2/4] fix(dsl): scope class materialization marker Consume the marker only in the intended constructor so nested normal construction cannot skip native wrapping. Clean up returned native resources when materialization fails. --- examples/js_dsl/mod.test.ts | 45 +++++++++++++++++++++++++++++++++++++ src/js/class_runtime.zig | 14 +++++++++++- src/js/wrap_class.zig | 6 ++++- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index 9df6556..43e15a8 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -425,6 +425,51 @@ describe("class materialization", () => { it("rejects cross-class static factory binding during materialization", () => { expect(() => mod.Point.create.call(mod.Buffer, 1, 2)).toThrow(); }); + + it("preserves normal nested construction inside subclass constructors", () => { + // Same-class materialization may call a JS subclass constructor via the preferred + // receiver constructor. A nested normal `new` inside that constructor must not + // inherit the internal materialization marker, otherwise it skips native wrapping. + let nested: InstanceType | undefined; + class DerivedFactoryResource extends mod.FactoryResource { + constructor() { + super(); + nested = new mod.FactoryResource(); + } + } + + const resource = DerivedFactoryResource.withByte(5); + + expect(resource).toBeInstanceOf(DerivedFactoryResource); + expect(resource.getByte()).toEqual(5); + expect(nested?.getByte()).toEqual(0); + }); + + it("does not run cross-class constructors during failed materialization", () => { + const initBefore = mod.getFactoryResourceInitCount(); + const deinitBefore = mod.getFactoryResourceDeinitCount(); + + expect(() => mod.Point.create.call(mod.FactoryResource, 1, 2)).toThrow(); + + expect(mod.getFactoryResourceInitCount()).toEqual(initBefore); + expect(mod.getFactoryResourceDeinitCount()).toEqual(deinitBefore); + }); + + it("rejects non-zapi constructors during materialization", () => { + function FakePoint() {} + + expect(() => mod.Point.create.call(FakePoint, 1, 2)).toThrow(); + }); + + it("deinitializes returned native resources when materialization fails", () => { + const initBefore = mod.getFactoryResourceInitCount(); + const deinitBefore = mod.getFactoryResourceDeinitCount(); + + expect(() => mod.FactoryResource.withByte.call(mod.Point, 7)).toThrow(); + + expect(mod.getFactoryResourceInitCount()).toEqual(initBefore + 1); + expect(mod.getFactoryResourceDeinitCount()).toEqual(deinitBefore + 1); + }); }); // Section 15: Getters and Setters diff --git a/src/js/class_runtime.zig b/src/js/class_runtime.zig index 6fffc3e..7e1373f 100644 --- a/src/js/class_runtime.zig +++ b/src/js/class_runtime.zig @@ -95,11 +95,21 @@ pub fn isMaterializing(comptime T: type) bool { return materialize_target == @as(?*const anyopaque, @ptrCast(internalCtorMarkerPtr(T))); } +pub fn hasPendingMaterialization() bool { + return materialize_target != null; +} + +pub fn consumeMaterialization(comptime T: type) bool { + if (!isMaterializing(T)) return false; + materialize_target = null; + return true; +} + pub fn materializeClassInstance(comptime T: type, env: napi.Env, instance: T, preferred_ctor: ?napi.Value) !napi.Value { const ctor = preferred_ctor orelse try getConstructor(T, env); const obj_ptr = try std.heap.c_allocator.create(T); - errdefer std.heap.c_allocator.destroy(obj_ptr); + errdefer destroyNativeObject(T, obj_ptr); obj_ptr.* = instance; const prev = materialize_target; @@ -116,6 +126,8 @@ pub fn materializeClassInstance(comptime T: type, env: napi.Env, instance: T, pr )); const js_instance = napi.Value{ .env = env.env, .value = js_instance_raw }; + if (materialize_target != null) return error.InvalidMaterializationConstructor; + try wrapTaggedObject(T, env, js_instance, obj_ptr, null); return js_instance; } diff --git a/src/js/wrap_class.zig b/src/js/wrap_class.zig index 46743b9..ca124eb 100644 --- a/src/js/wrap_class.zig +++ b/src/js/wrap_class.zig @@ -369,9 +369,13 @@ pub fn wrapClass(comptime T: type) type { // Fast path: materializeClassInstance is creating this instance. // Skip the placeholder alloc/wrap — materialize will wrap directly // with the real native pointer after napi_new_instance returns. - if (class_runtime.isMaterializing(T)) { + if (class_runtime.consumeMaterialization(T)) { return this_arg; } + if (class_runtime.hasPendingMaterialization()) { + e.throwTypeError("", "Invalid materialization constructor") catch {}; + return null; + } if (actual_argc == 1) { const internal_arg = napi.Value{ .env = raw_env, .value = raw_args[0] }; From 8ce78ffc9f586e9a646c151ed1e4962b6bd6e5ad Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Thu, 28 May 2026 11:12:48 +0200 Subject: [PATCH 3/4] fix: use class constructor to materialize a class --- examples/js_dsl/mod.test.ts | 11 +++++++++++ src/js/class_runtime.zig | 27 ++++++++++++++++++++++++++- src/js/wrap_class.zig | 6 +++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index 43e15a8..cdeb643 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -445,6 +445,17 @@ describe("class materialization", () => { expect(nested?.getByte()).toEqual(0); }); + it("rejects subclass constructors that return a different object", () => { + class ReplacingPoint extends mod.Point { + constructor() { + super(); + return {}; + } + } + + expect(() => ReplacingPoint.create(3, 4)).toThrow(); + }); + it("does not run cross-class constructors during failed materialization", () => { const initBefore = mod.getFactoryResourceInitCount(); const deinitBefore = mod.getFactoryResourceDeinitCount(); diff --git a/src/js/class_runtime.zig b/src/js/class_runtime.zig index 7e1373f..e81d084 100644 --- a/src/js/class_runtime.zig +++ b/src/js/class_runtime.zig @@ -91,6 +91,17 @@ pub fn registerClass(comptime T: type, env: napi.Env, ctor: napi.Value) !void { /// Compared by identity against `internalCtorMarkerPtr(T)`. threadlocal var materialize_target: ?*const anyopaque = null; +/// Captures the exact `this` object whose generated base constructor consumed +/// `materialize_target`. JS derived constructors are allowed to `return {}` +/// after `super()`, causing `napi_new_instance` to return that replacement +/// object. Materialization must reject that case instead of wrapping native +/// state onto an unrelated object with the wrong prototype. +/// +/// Stored as a temporary N-API reference because nested JS construction can run +/// before `napi_new_instance` returns; keeping only the raw constructor callback +/// handle is not stable enough across that nested call stack. +threadlocal var materialized_instance: ?napi.Ref = null; + pub fn isMaterializing(comptime T: type) bool { return materialize_target == @as(?*const anyopaque, @ptrCast(internalCtorMarkerPtr(T))); } @@ -99,9 +110,12 @@ pub fn hasPendingMaterialization() bool { return materialize_target != null; } -pub fn consumeMaterialization(comptime T: type) bool { +pub fn consumeMaterialization(comptime T: type, env: napi.Env, this_arg: napi.c.napi_value) !bool { if (!isMaterializing(T)) return false; + const this_val = napi.Value{ .env = env.env, .value = this_arg }; + const this_ref = try env.createReference(this_val, 1); materialize_target = null; + materialized_instance = this_ref; return true; } @@ -113,8 +127,14 @@ pub fn materializeClassInstance(comptime T: type, env: napi.Env, instance: T, pr obj_ptr.* = instance; const prev = materialize_target; + const prev_instance = materialized_instance; materialize_target = @ptrCast(internalCtorMarkerPtr(T)); + materialized_instance = null; defer materialize_target = prev; + defer { + if (materialized_instance) |ref| ref.delete() catch {}; + materialized_instance = prev_instance; + } var js_instance_raw: napi.c.napi_value = null; try napi.status.check(napi.c.napi_new_instance( @@ -127,6 +147,11 @@ pub fn materializeClassInstance(comptime T: type, env: napi.Env, instance: T, pr const js_instance = napi.Value{ .env = env.env, .value = js_instance_raw }; if (materialize_target != null) return error.InvalidMaterializationConstructor; + const expected_instance_ref = materialized_instance orelse return error.InvalidMaterializationConstructor; + const expected_instance = try expected_instance_ref.getValue(); + // The generated constructor must be the object that comes back from + // `napi_new_instance`; otherwise a subclass returned a replacement object. + if (!(try expected_instance.strictEquals(js_instance))) return error.InvalidMaterializationConstructor; try wrapTaggedObject(T, env, js_instance, obj_ptr, null); return js_instance; diff --git a/src/js/wrap_class.zig b/src/js/wrap_class.zig index ca124eb..fed936d 100644 --- a/src/js/wrap_class.zig +++ b/src/js/wrap_class.zig @@ -369,7 +369,11 @@ pub fn wrapClass(comptime T: type) type { // Fast path: materializeClassInstance is creating this instance. // Skip the placeholder alloc/wrap — materialize will wrap directly // with the real native pointer after napi_new_instance returns. - if (class_runtime.consumeMaterialization(T)) { + const did_consume_materialization = class_runtime.consumeMaterialization(T, e, this_arg) catch { + e.throwError("", "Failed to record materialized instance") catch {}; + return null; + }; + if (did_consume_materialization) { return this_arg; } if (class_runtime.hasPendingMaterialization()) { From ba7c8c747efb49ff3344ecb9dc27047600a00079 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Thu, 28 May 2026 11:50:48 +0200 Subject: [PATCH 4/4] chore: remove the orphan reference to placeholder --- examples/js_dsl/mod.test.ts | 4 ++-- examples/js_dsl/mod.zig | 2 +- src/js/class_runtime.zig | 44 ++++++------------------------------- src/js/wrap_class.zig | 34 +++++----------------------- 4 files changed, 15 insertions(+), 69 deletions(-) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index cdeb643..9886585 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -391,7 +391,7 @@ describe("optional parameters", () => { }); describe("class materialization", () => { - it("static class return avoids constructor placeholder allocation", () => { + it("static class return avoids constructor init allocation", () => { const initBefore = mod.getFactoryResourceInitCount(); const deinitBefore = mod.getFactoryResourceDeinitCount(); @@ -402,7 +402,7 @@ describe("class materialization", () => { expect(mod.getFactoryResourceDeinitCount()).toEqual(deinitBefore); }); - it("instance class return avoids constructor placeholder allocation", () => { + it("instance class return avoids constructor init allocation", () => { const base = mod.FactoryResource.withByte(1); const initBefore = mod.getFactoryResourceInitCount(); const deinitBefore = mod.getFactoryResourceDeinitCount(); diff --git a/examples/js_dsl/mod.zig b/examples/js_dsl/mod.zig index 1eef6cd..59ee1c8 100644 --- a/examples/js_dsl/mod.zig +++ b/examples/js_dsl/mod.zig @@ -426,7 +426,7 @@ pub const Point = struct { } }; -/// A resource-owning class used to verify placeholder cleanup in factory paths. +/// A resource-owning class used to verify cleanup in class materialization paths. pub const FactoryResource = struct { pub const js_meta = js.class(.{}); data: []u8, diff --git a/src/js/class_runtime.zig b/src/js/class_runtime.zig index e81d084..edf52ef 100644 --- a/src/js/class_runtime.zig +++ b/src/js/class_runtime.zig @@ -8,15 +8,11 @@ pub fn typeTag(comptime T: type) napi.c.napi_type_tag { }; } -pub fn wrapTaggedObject(comptime T: type, env: napi.Env, object: napi.Value, native_object: *T, finalize_hint: ?*anyopaque) !void { +pub fn wrapTaggedObject(comptime T: type, env: napi.Env, object: napi.Value, native_object: *T) !void { const tag = typeTag(T); - try env.wrap(object, T, native_object, defaultFinalize(T), finalize_hint, null); + try env.wrap(object, T, native_object, defaultFinalize(T), null, null); errdefer if (env.removeWrap(T, object)) |removed| { - if (isInternalPlaceholderHint(T, finalize_hint)) { - destroyInternalPlaceholder(T, removed); - } else { - destroyNativeObject(T, removed); - } + destroyNativeObject(T, removed); } else |_| {}; if (!(try env.checkObjectTypeTag(object, tag))) { try env.typeTagObject(object, tag); @@ -48,17 +44,9 @@ pub fn destroyNativeObject(comptime T: type, obj: *T) void { std.heap.c_allocator.destroy(obj); } -pub fn destroyInternalPlaceholder(comptime T: type, obj: *T) void { - std.heap.c_allocator.destroy(obj); -} - pub fn defaultFinalize(comptime T: type) napi.FinalizeCallback(T) { return struct { - fn f(_: napi.Env, obj: *T, hint: ?*anyopaque) void { - if (isInternalPlaceholderHint(T, hint)) { - destroyInternalPlaceholder(T, obj); - return; - } + fn f(_: napi.Env, obj: *T, _: ?*anyopaque) void { destroyNativeObject(T, obj); } }.f; @@ -86,8 +74,8 @@ pub fn registerClass(comptime T: type, env: napi.Env, ctor: napi.Value) !void { } /// Per-thread marker set by `materializeClassInstance` to tell the generated -/// constructor "this `new` call comes from the DSL — don't allocate a placeholder, -/// I'll wrap with the real object after `napi_new_instance` returns." +/// constructor "this `new` call comes from the DSL; return the JS instance +/// without running `init`, and materialization will wrap the native object." /// Compared by identity against `internalCtorMarkerPtr(T)`. threadlocal var materialize_target: ?*const anyopaque = null; @@ -153,7 +141,7 @@ pub fn materializeClassInstance(comptime T: type, env: napi.Env, instance: T, pr // `napi_new_instance`; otherwise a subclass returned a replacement object. if (!(try expected_instance.strictEquals(js_instance))) return error.InvalidMaterializationConstructor; - try wrapTaggedObject(T, env, js_instance, obj_ptr, null); + try wrapTaggedObject(T, env, js_instance, obj_ptr); return js_instance; } @@ -167,19 +155,6 @@ fn getConstructor(comptime T: type, env: napi.Env) !napi.Value { return try entry.ctor_ref.getValue(); } -pub fn isInternalCtorArg(comptime T: type, value: napi.Value) bool { - const raw = value.getValueExternal() catch return false; - return raw == @as(*anyopaque, @ptrCast(internalCtorMarkerPtr(T))); -} - -pub fn internalPlaceholderHint(comptime T: type) ?*anyopaque { - return @ptrCast(&markers(T).placeholder_hint); -} - -pub fn isInternalPlaceholderHint(comptime T: type, hint: ?*anyopaque) bool { - return hint == internalPlaceholderHint(T); -} - fn state(comptime T: type) type { return struct { const Class = T; @@ -240,14 +215,9 @@ fn markers(comptime T: type) type { } var ctor_marker: u8 = 0; - var placeholder_hint: u8 = 0; }; } -fn internalCtorMarker(comptime T: type) [*]const u8 { - return internalCtorMarkerPtr(T); -} - fn internalCtorMarkerPtr(comptime T: type) *u8 { return &markers(T).ctor_marker; } diff --git a/src/js/wrap_class.zig b/src/js/wrap_class.zig index fed936d..bb20974 100644 --- a/src/js/wrap_class.zig +++ b/src/js/wrap_class.zig @@ -68,21 +68,15 @@ pub fn wrapClass(comptime T: type) type { /// This callback is invoked when `new Class(...)` is called in JavaScript. /// It handles argument conversion, calls the `pub fn init(...)` method of /// the Zig struct `T`, and wraps the resulting native object in the JS instance. - /// It also handles internal placeholder creation for factory methods. + /// It also supports direct wrapping for DSL-returned class instances. pub const constructor: napi.c.napi_callback = genConstructor(); /// The default N-API finalizer callback for native objects wrapped by instances of `T`. /// /// This function is registered with `napi.Env.wrap()` and is called by the /// JavaScript garbage collector when the wrapped JS object is finalized. - /// It handles cleanup for internal placeholder objects during class - /// materialization and calls the `deinit()` method (if present) and frees - /// the native memory for regular class instances. - pub fn defaultFinalize(_: napi.Env, obj: *T, hint: ?*anyopaque) void { - if (class_runtime.isInternalPlaceholderHint(T, hint)) { - class_runtime.destroyInternalPlaceholder(T, obj); - return; - } + /// It calls the `deinit()` method (if present) and frees the native memory. + pub fn defaultFinalize(_: napi.Env, obj: *T, _: ?*anyopaque) void { class_runtime.destroyNativeObject(T, obj); } @@ -367,7 +361,7 @@ pub fn wrapClass(comptime T: type) type { }; // Fast path: materializeClassInstance is creating this instance. - // Skip the placeholder alloc/wrap — materialize will wrap directly + // Skip normal init; materialize will wrap the returned JS instance // with the real native pointer after napi_new_instance returns. const did_consume_materialization = class_runtime.consumeMaterialization(T, e, this_arg) catch { e.throwError("", "Failed to record materialized instance") catch {}; @@ -381,24 +375,6 @@ pub fn wrapClass(comptime T: type) type { return null; } - if (actual_argc == 1) { - const internal_arg = napi.Value{ .env = raw_env, .value = raw_args[0] }; - if ((internal_arg.typeof() catch null) == .external) { - const obj_ptr = std.heap.c_allocator.create(T) catch { - e.throwError("", "Out of memory allocating internal placeholder") catch {}; - return null; - }; - obj_ptr.* = std.mem.zeroes(T); - - const this_val = napi.Value{ .env = raw_env, .value = this_arg }; - class_runtime.wrapTaggedObject(T, e, this_val, obj_ptr, class_runtime.internalPlaceholderHint(T)) catch { - e.throwError("", "Failed to wrap internal placeholder") catch {}; - return null; - }; - return this_arg; - } - } - const required_init_argc = comptime wrap_function.requiredArgCount(init_params); if (required_init_argc > 0 and actual_argc < required_init_argc) { e.throwTypeError("", "Constructor expects at least " ++ std.fmt.comptimePrint("{d}", .{required_init_argc}) ++ " arguments") catch {}; @@ -423,7 +399,7 @@ pub fn wrapClass(comptime T: type) type { obj_ptr.* = init_result; const this_val = napi.Value{ .env = raw_env, .value = this_arg }; - class_runtime.wrapTaggedObject(T, e, this_val, obj_ptr, null) catch { + class_runtime.wrapTaggedObject(T, e, this_val, obj_ptr) catch { e.throwError("", "Failed to wrap native object") catch {}; return null; };