From 996936ee206c89e7a5bd53b86ab7579fd1bc19ee Mon Sep 17 00:00:00 2001 From: bing Date: Fri, 22 May 2026 14:01:33 +0800 Subject: [PATCH 1/2] 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 7b10cef..4e5f6d0 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 41d228516c9a2275c7615d53d90205bfa4f2424e Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Tue, 26 May 2026 18:03:24 +0200 Subject: [PATCH 2/2] 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 505bd95..128d389 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -407,6 +407,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 4e5f6d0..b694f84 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] };