Skip to content

feat(bindings/bls): use external buffers for blst operations #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
spiral-ladder wants to merge 1 commit into from
Closed

feat(bindings/bls): use external buffers for blst operations #356

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
65 changes: 40 additions & 25 deletions bindings/napi/blst.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,41 @@ fn coerceToBool(boolish: napi.Value) napi.status.NapiError!bool {
return b.getValueBool();
}

/// Wraps `bytes` as a JS `Uint8Array` backed by an *external* (native-heap) ArrayBuffer.

@wemeetagain wemeetagain May 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd change the comment here to mentioning duping, copying, etc. Current comment sounds like bytes is being captured.

///
/// V8 holds the pointer to manage the buffer's lifetime, but the buffer must still
/// be freed natively via a finalizer.
///
/// ## Details
///
/// Heap-allocation rate & GC frequency increases when using `napi_create_arraybuffer`
/// since V8 owns both the pointer and the payload. We rely on external buffers here
/// to lighten the pressure on V8's GC engine.
fn createExternalUint8Array(env: napi.Env, bytes: []const u8) !napi.Value {
const buf = try allocator.alloc(u8, bytes.len);
@memcpy(buf, bytes);
Comment on lines +151 to +152

@wemeetagain wemeetagain May 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can use allocator.dupe as a shorthand


const len_hint: ?*anyopaque = @ptrFromInt(bytes.len);
const arraybuffer = env.createExternalArrayBuffer(buf, externalUint8Finalize, len_hint) catch |err| {
allocator.free(buf);

@wemeetagain wemeetagain May 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you should use an errdefer allocator.free(buf) above, and call env.createExternalArrayBuffer with try.

return err;
};

_ = try env.adjustExternalMemory(@intCast(bytes.len));
return try env.createTypedarray(.uint8, bytes.len, arraybuffer, 0);
}
Comment on lines +150 to +162

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To adhere to the repository style guide (Lines 51-55), this function should include assertions for its arguments and maintain a minimum assertion density of two per function. Additionally, the return value of adjustExternalMemory should be asserted rather than ignored (Line 51). The callback name in the createExternalArrayBuffer call should also be updated to createExternalUint8ArrayFinalize to match the renamed finalizer for better clarity (Line 246).

fn createExternalUint8Array(env: napi.Env, bytes: []const u8) !napi.Value {
    std.debug.assert(bytes.len > 0);
    std.debug.assert(bytes.len <= std.math.maxInt(i64));

    const buf = try allocator.alloc(u8, bytes.len);
    @memcpy(buf, bytes);

    const len_hint: ?*anyopaque = @ptrFromInt(bytes.len);
    const arraybuffer = env.createExternalArrayBuffer(buf, createExternalUint8ArrayFinalize, len_hint) catch |err| {
        allocator.free(buf);
        return err;
    };

    const adjusted = try env.adjustExternalMemory(@intCast(bytes.len));
    std.debug.assert(adjusted >= @as(i64, @intCast(bytes.len)));

    return try env.createTypedarray(.uint8, bytes.len, arraybuffer, 0);
}
References
  1. Assert all function arguments and return values, pre/postconditions and invariants. The assertion density of the code must average a minimum of two assertions per function. (link)
  2. When a single function calls out to a helper function or callback, prefix the name of the helper function with the name of the calling function to show the call history. (link)


fn externalUint8Finalize(env: napi.c.napi_env, data: ?*anyopaque, hint: ?*anyopaque) callconv(.c) void {
const len: usize = @intFromPtr(hint);
if (data) |d| {
const ptr: [*]u8 = @ptrCast(d);
allocator.free(ptr[0..len]);
}
var result: i64 = undefined;

_ = napi.status.check(napi.c.napi_adjust_external_memory(env, -@as(i64, @intCast(len)), &result)) catch return {};
Comment on lines +170 to +172

@wemeetagain wemeetagain May 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

don't need result or napi.status.check since neither are intended to be used.

}
Comment on lines +164 to +173

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The finalizer should be renamed to createExternalUint8ArrayFinalize to clearly associate it with its caller (Line 246). It also requires assertions for all arguments (Line 51). Furthermore, architecture-specific usize should be avoided in favor of explicitly-sized types like u64 (Line 44), and the long line calling napi_adjust_external_memory should be wrapped to stay within the 100-column limit (Line 400). The error handling should also be more robust than a silent catch (Line 46, 160).

fn createExternalUint8ArrayFinalize(env: napi.c.napi_env, data: ?*anyopaque, hint: ?*anyopaque) callconv(.c) void {
    std.debug.assert(env != null);
    std.debug.assert(data != null);
    std.debug.assert(hint != null);

    const len: u64 = @intCast(@intFromPtr(hint));
    if (data) |d| {
        const ptr: [*]u8 = @ptrCast(d);
        allocator.free(ptr[0..@intCast(len)]);
    }
    var result: i64 = undefined;

    _ = napi.status.check(napi.c.napi_adjust_external_memory(
        env,
        -@as(i64, @intCast(len)),
        &result,
    )) catch {
        std.debug.assert(false);
    };
}
References
  1. Use explicitly-sized types like u32 for everything, avoid architecture-specific usize. (link)
  2. Hard limit all line lengths, without exception, to at most 100 columns. (link)
  3. Assertions detect programmer errors. The only correct way to handle corrupt code is to crash. (link)


pub fn PublicKey_finalize(_: napi.Env, pk: *PublicKey, _: ?*anyopaque) void {
allocator.destroy(pk);
}
Expand Down Expand Up @@ -219,18 +254,10 @@ pub fn PublicKey_toBytes(env: napi.Env, cb: napi.CallbackInfo(1)) !napi.Value {

if (compress) {
const bytes = pk.compress();

var arraybuffer_bytes: [*]u8 = undefined;
const arraybuffer = try env.createArrayBuffer(PublicKey.COMPRESS_SIZE, &arraybuffer_bytes);
@memcpy(arraybuffer_bytes[0..PublicKey.COMPRESS_SIZE], &bytes);
return try env.createTypedarray(.uint8, PublicKey.COMPRESS_SIZE, arraybuffer, 0);
return try createExternalUint8Array(env, &bytes);
} else {
const bytes = pk.serialize();

var arraybuffer_bytes: [*]u8 = undefined;
const arraybuffer = try env.createArrayBuffer(PublicKey.SERIALIZE_SIZE, &arraybuffer_bytes);
@memcpy(arraybuffer_bytes[0..PublicKey.SERIALIZE_SIZE], &bytes);
return try env.createTypedarray(.uint8, PublicKey.SERIALIZE_SIZE, arraybuffer, 0);
return try createExternalUint8Array(env, &bytes);
}
}

Expand Down Expand Up @@ -336,18 +363,10 @@ pub fn Signature_toBytes(env: napi.Env, cb: napi.CallbackInfo(1)) !napi.Value {

if (compress) {
const bytes = sig.compress();

var arraybuffer_bytes: [*]u8 = undefined;
const arraybuffer = try env.createArrayBuffer(Signature.COMPRESS_SIZE, &arraybuffer_bytes);
@memcpy(arraybuffer_bytes[0..Signature.COMPRESS_SIZE], &bytes);
return try env.createTypedarray(.uint8, Signature.COMPRESS_SIZE, arraybuffer, 0);
return try createExternalUint8Array(env, &bytes);
} else {
const bytes = sig.serialize();

var arraybuffer_bytes: [*]u8 = undefined;
const arraybuffer = try env.createArrayBuffer(Signature.SERIALIZE_SIZE, &arraybuffer_bytes);
@memcpy(arraybuffer_bytes[0..Signature.SERIALIZE_SIZE], &bytes);
return try env.createTypedarray(.uint8, Signature.SERIALIZE_SIZE, arraybuffer, 0);
return try createExternalUint8Array(env, &bytes);
}
}

Expand Down Expand Up @@ -484,11 +503,7 @@ pub fn SecretKey_toPublicKey(env: napi.Env, cb: napi.CallbackInfo(0)) !napi.Valu
pub fn SecretKey_toBytes(env: napi.Env, cb: napi.CallbackInfo(0)) !napi.Value {
const sk = try env.unwrap(SecretKey, cb.this());
const bytes = sk.serialize();

var arraybuffer_bytes: [*]u8 = undefined;
const arraybuffer = try env.createArrayBuffer(SecretKey.serialize_size, &arraybuffer_bytes);
@memcpy(arraybuffer_bytes[0..SecretKey.serialize_size], &bytes);
return try env.createTypedarray(.uint8, SecretKey.serialize_size, arraybuffer, 0);
return try createExternalUint8Array(env, &bytes);
}

/// Aggregates multiple Signature objects into one.
Expand Down
Loading