Skip to content

Re-introduce call_arena #547

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

Merged
merged 2 commits into from
Apr 22, 2025
Merged

Re-introduce call_arena #547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3277d1b
Re-introduce call_arena
karlseguin Apr 19, 2025
114e11f
Partially revert some changes.
karlseguin Apr 22, 2025
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
2 changes: 1 addition & 1 deletion src/browser/dom/mutation_observer.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ const EventHandler = struct {
const muevt = parser.eventToMutationEvent(evt.?);

// TODO get the allocator by another way?
const alloc = data.cbk.executor.call_arena.allocator();
const alloc = data.cbk.executor.scope_arena;

if (std.mem.eql(u8, t, "DOMAttrModified")) {
mrs.first = .{
Expand Down
99 changes: 72 additions & 27 deletions src/runtime/js.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ const ArenaAllocator = std.heap.ArenaAllocator;

const log = std.log.scoped(.js);

const CALL_ARENA_RETAIN = 1024 * 16;
const SCOPE_ARENA_RETAIN = 1024 * 64;

// Global, should only be initialized once.
pub const Platform = struct {
inner: v8.Platform,
Expand Down Expand Up @@ -177,6 +180,9 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
// Send a lowMemoryNotification whenever we stop an executor
gc_hints: bool,

// Used in debug mode to assert that we only have one executor at a time
has_executor: if (builtin.mode == .Debug) bool else void = if (builtin.mode == .Debug) false else {},

const Self = @This();

const State = S;
Expand Down Expand Up @@ -260,6 +266,10 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
}

pub fn startExecutor(self: *Self, comptime Global: type, state: State, module_loader: anytype) !*Executor {
if (comptime builtin.mode == .Debug) {
std.debug.assert(self.has_executor == false);
self.has_executor = true;
}
const isolate = self.isolate;
const templates = &self.templates;

Expand Down Expand Up @@ -332,22 +342,28 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
context.setEmbedderData(1, data);
}

const allocator = self.allocator;

executor.* = .{
.state = state,
.context = context,
.isolate = isolate,
.templates = templates,
.handle_scope = handle_scope,
.call_arena = ArenaAllocator.init(allocator),
.scope_arena = ArenaAllocator.init(allocator),
.call_arena = undefined,
.scope_arena = undefined,
._call_arena_instance = std.heap.ArenaAllocator.init(self.allocator),
._scope_arena_instance = std.heap.ArenaAllocator.init(self.allocator),
.module_loader = .{
.ptr = @ptrCast(module_loader),
.func = @TypeOf(module_loader.*).fetchModuleSource,
},
};

// We do it this way, just to present a slightly nicer API. Many
// things want one of these two allocator from the executor. Now they
// can do `executor.call_arena` instead of `executor.call_arena.allocator`.
executor.call_arena = executor._call_arena_instance.allocator();
executor.scope_arena = executor._scope_arena_instance.allocator();

errdefer self.stopExecutor(executor);

// Custom exception
Expand Down Expand Up @@ -375,6 +391,11 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
if (self.gc_hints) {
self.isolate.lowMemoryNotification();
}

if (comptime builtin.mode == .Debug) {
std.debug.assert(self.has_executor == true);
self.has_executor = false;
}
}

// Give it a Zig struct, get back a v8.FunctionTemplate.
Expand Down Expand Up @@ -755,19 +776,27 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
// a context, we can always get the Executor back.
context: v8.Context,

// Because calls can be nested (i.e.a function calling a callback),
// we can only reset the call_arena when call_depth == 0. If we were
// to reset it within a callback, it would invalidate the data of
// the call which is calling the callback.
call_depth: usize = 0,

// Arena whose lifetime is for a single getter/setter/function/etc.
// Largely used to get strings out of V8, like a stack trace from
// a TryCatch. The allocator will be owned by the Scope, but the
// arena itself is owned by the Executor so that we can re-use it
// from scope to scope.
call_arena: ArenaAllocator,
call_arena: Allocator,
_call_arena_instance: std.heap.ArenaAllocator,

// Arena whose lifetime is for a single page load, aka a Scope. Where
// the call_arena lives for a single function call, the scope_arena
// lives for the lifetime of the entire page. The allocator will be
// owned by the Scope, but the arena itself is owned by the Executor
// so that we can re-use it from scope to scope.
scope_arena: ArenaAllocator,
scope_arena: Allocator,
_scope_arena_instance: std.heap.ArenaAllocator,

// When we need to load a resource (i.e. an external script), we call
// this function to get the source. This is always a reference to the
Expand All @@ -790,13 +819,14 @@ pub fn Env(comptime S: type, comptime types: anytype) type {

// not public, must be destroyed via env.stopExecutor()
fn deinit(self: *Executor) void {
if (self.scope) |*s| {
s.deinit();
if (self.scope != null) {
self.endScope();
}
self.context.exit();
self.handle_scope.deinit();
self.call_arena.deinit();
self.scope_arena.deinit();

self._call_arena_instance.deinit();
self._scope_arena_instance.deinit();
}

// Executes the src
Expand Down Expand Up @@ -889,16 +919,15 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
v8.HandleScope.init(&handle_scope, self.isolate);
self.scope = Scope{
.handle_scope = handle_scope,
.arena = self.scope_arena.allocator(),
.call_arena = self.scope_arena.allocator(),
.arena = self.scope_arena,
};
_ = try self._mapZigInstanceToJs(self.context.getGlobal(), global);
}

pub fn endScope(self: *Executor) void {
self.scope.?.deinit();
self.scope = null;
_ = self.scope_arena.reset(.{ .retain_with_limit = 1024 * 64 });
_ = self._scope_arena_instance.reset(.{ .retain_with_limit = SCOPE_ARENA_RETAIN });
}

// Given an anytype, turns it into a v8.Object. The anytype could be:
Expand Down Expand Up @@ -1090,7 +1119,6 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
// in executor, rather than having one for each of these.
pub const Scope = struct {
arena: Allocator,
call_arena: Allocator,
handle_scope: v8.HandleScope,
callbacks: std.ArrayListUnmanaged(v8.Persistent(v8.Function)) = .{},
identity_map: std.AutoHashMapUnmanaged(usize, PersistentObject) = .{},
Expand Down Expand Up @@ -1148,7 +1176,7 @@ pub fn Env(comptime S: type, comptime types: anytype) type {

self.callWithThis(this, args) catch |err| {
if (try_catch.hasCaught()) {
const allocator = self.executor.scope.?.call_arena;
const allocator = self.executor.call_arena;
result.stack = try_catch.stack(allocator) catch null;
result.exception = (try_catch.exception(allocator) catch @errorName(err)) orelse @errorName(err);
} else {
Expand Down Expand Up @@ -1185,7 +1213,7 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
fn printFunc(self: Callback) !void {
const executor = self.executor;
const value = self.func.castToFunction().toValue();
const src = try valueToString(executor.call_arena.allocator(), value, executor.isolate, executor.context);
const src = try valueToString(executor.call_arena, value, executor.isolate, executor.context);
std.debug.print("{s}\n", .{src});
}
};
Expand All @@ -1209,7 +1237,7 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
pub fn setIndex(self: JsObject, index: usize, value: anytype) !void {
const key = switch (index) {
inline 0...1000 => |i| std.fmt.comptimePrint("{d}", .{i}),
else => try std.fmt.allocPrint(self.executor.scope_arena.allocator(), "{d}", .{index}),
else => try std.fmt.allocPrint(self.executor.scope_arena, "{d}", .{index}),
};
return self.set(key, value);
}
Expand Down Expand Up @@ -1491,7 +1519,7 @@ fn Caller(comptime E: type) type {
context: v8.Context,
isolate: v8.Isolate,
executor: *E.Executor,
call_allocator: Allocator,
call_arena: Allocator,

const Self = @This();

Expand All @@ -1503,17 +1531,34 @@ fn Caller(comptime E: type) type {
const context = isolate.getCurrentContext();
const executor: *E.Executor = @ptrFromInt(context.getEmbedderData(1).castTo(v8.BigInt).getUint64());

executor.call_depth += 1;
return .{
.isolate = isolate,
.context = context,
.executor = executor,
.call_allocator = executor.scope.?.call_arena,
.call_arena = executor.call_arena,
};
}

fn deinit(self: *Self) void {
_ = self;
// _ = self.executor.call_arena.reset(.{ .retain_with_limit = 4096 });
const executor = self.executor;
const call_depth = executor.call_depth - 1;
executor.call_depth = call_depth;

// Because of callbacks, calls can be nested. Because of this, we
// can't clear the call_arena after _every_ call. Imagine we have
// arr.forEach((i) => { console.log(i); }
//
// First we call forEach. Inside of our forEach call,
// we call console.log. If we reset the call_arena after this call,
// it'll reset it for the `forEach` call after, which might still
// need the data.
//
// Therefore, we keep a call_depth, and only reset the call_arena
// when a top-level (call_depth == 0) function ends.
if (call_depth == 0) {
_ = executor._call_arena_instance.reset(.{ .retain_with_limit = CALL_ARENA_RETAIN });
}
}

fn constructor(self: *Self, comptime named_function: anytype, info: v8.FunctionCallbackInfo) !void {
Expand Down Expand Up @@ -1681,7 +1726,7 @@ fn Caller(comptime E: type) type {
}

fn nameToString(self: *Self, name: v8.Name) ![]const u8 {
return valueToString(self.call_allocator, .{ .handle = name.handle }, self.isolate, self.context);
return valueToString(self.call_arena, .{ .handle = name.handle }, self.isolate, self.context);
}

fn assertSelfReceiver(comptime named_function: anytype) void {
Expand Down Expand Up @@ -1730,7 +1775,7 @@ fn Caller(comptime E: type) type {

const Exception = comptime getCustomException(named_function.S) orelse break :blk null;
if (function_error_set == Exception or isErrorSetException(Exception, err)) {
const custom_exception = Exception.init(self.call_allocator, err, named_function.js_name) catch |init_err| {
const custom_exception = Exception.init(self.call_arena, err, named_function.js_name) catch |init_err| {
switch (init_err) {
// if a custom exceptions' init wants to return a
// different error, we need to think about how to
Expand Down Expand Up @@ -1863,7 +1908,7 @@ fn Caller(comptime E: type) type {
if (js_parameter_count == 0) {
@field(args, tupleFieldName(params_to_map.len + offset - 1)) = &.{};
} else {
const arr = try self.call_allocator.alloc(last_parameter_type_info.pointer.child, js_parameter_count - params_to_map.len + 1);
const arr = try self.call_arena.alloc(last_parameter_type_info.pointer.child, js_parameter_count - params_to_map.len + 1);
for (arr, last_js_parameter..) |*a, i| {
const js_value = info.getArg(@as(u32, @intCast(i)));
a.* = try self.jsValueToZig(named_function, slice_type, js_value);
Expand Down Expand Up @@ -1934,10 +1979,10 @@ fn Caller(comptime E: type) type {
if (ptr.child == u8) {
if (ptr.sentinel()) |s| {
if (comptime s == 0) {
return valueToStringZ(self.call_allocator, js_value, self.isolate, self.context);
return valueToStringZ(self.call_arena, js_value, self.isolate, self.context);
}
} else {
return valueToString(self.call_allocator, js_value, self.isolate, self.context);
return valueToString(self.call_arena, js_value, self.isolate, self.context);
}
}

Expand All @@ -1963,7 +2008,7 @@ fn Caller(comptime E: type) type {

// Newer version of V8 appear to have an optimized way
// to do this (V8::Array has an iterate method on it)
const arr = try self.call_allocator.alloc(ptr.child, js_arr.length());
const arr = try self.call_arena.alloc(ptr.child, js_arr.length());
for (arr, 0..) |*a, i| {
a.* = try self.jsValueToZig(named_function, ptr.child, try js_obj.getAtIndex(context, @intCast(i)));
}
Expand Down
Loading