diff --git a/src/browser/dom/mutation_observer.zig b/src/browser/dom/mutation_observer.zig index c77be1d8..38da1bd8 100644 --- a/src/browser/dom/mutation_observer.zig +++ b/src/browser/dom/mutation_observer.zig @@ -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 = .{ diff --git a/src/runtime/js.zig b/src/runtime/js.zig index 5d52536c..0c1907c3 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -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, @@ -177,6 +180,14 @@ 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 {}, + + // See the Executor's call_arena and scope_arena allocators. Having these + // here allows the arena to be re-used by different executors. + call_arena: ArenaAllocator, + scope_arena: ArenaAllocator, + const Self = @This(); const State = S; @@ -212,6 +223,8 @@ pub fn Env(comptime S: type, comptime types: anytype) type { .gc_hints = opts.gc_hints, .global_scope = global_scope, .prototype_lookup = undefined, + .call_arena = ArenaAllocator.init(allocator), + .scope_arena = ArenaAllocator.init(allocator), .executor_pool = std.heap.MemoryPool(Executor).init(allocator), }; @@ -247,6 +260,8 @@ pub fn Env(comptime S: type, comptime types: anytype) type { } pub fn deinit(self: *Self) void { + self.call_arena.deinit(); + self.scope_arena.deinit(); self.global_scope.deinit(); self.isolate.exit(); self.isolate.deinit(); @@ -260,6 +275,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; @@ -332,16 +351,14 @@ 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 = self.call_arena.allocator(), + .scope_arena = self.scope_arena.allocator(), .module_loader = .{ .ptr = @ptrCast(module_loader), .func = @TypeOf(module_loader.*).fetchModuleSource, @@ -375,6 +392,13 @@ 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; + } + _ = self.call_arena.reset(.{ .retain_with_limit = CALL_ARENA_RETAIN }); + _ = self.scope_arena.reset(.{ .retain_with_limit = SCOPE_ARENA_RETAIN }); } // Give it a Zig struct, get back a v8.FunctionTemplate. @@ -755,19 +779,25 @@ 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, // 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, // 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 @@ -790,13 +820,11 @@ 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(); } // Executes the src @@ -889,8 +917,7 @@ 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); } @@ -898,7 +925,8 @@ pub fn Env(comptime S: type, comptime types: anytype) type { pub fn endScope(self: *Executor) void { self.scope.?.deinit(); self.scope = null; - _ = self.scope_arena.reset(.{ .retain_with_limit = 1024 * 64 }); + const scope_arena: *std.heap.ArenaAllocator = @ptrCast(@alignCast(self.scope_arena.ptr)); + _ = scope_arena.reset(.{ .retain_with_limit = SCOPE_ARENA_RETAIN }); } // Given an anytype, turns it into a v8.Object. The anytype could be: @@ -1090,7 +1118,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) = .{}, @@ -1148,7 +1175,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 { @@ -1185,7 +1212,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}); } }; @@ -1209,7 +1236,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); } @@ -1491,7 +1518,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(); @@ -1503,17 +1530,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 call_depth = self.executor.call_depth - 1; + self.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) { + const call_arena: *std.heap.ArenaAllocator = @ptrCast(@alignCast(self.call_arena.ptr)); + _ = call_arena.reset(.{ .retain_with_limit = CALL_ARENA_RETAIN }); + } } fn constructor(self: *Self, comptime named_function: anytype, info: v8.FunctionCallbackInfo) !void { @@ -1681,7 +1725,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 { @@ -1730,7 +1774,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 @@ -1863,7 +1907,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); @@ -1934,10 +1978,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); } } @@ -1963,7 +2007,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))); }