Merge pull request #1125 from lightpanda-io/call_arena

Move the call_arena to the page.

Author: karlseguin

src/browser/browser.zig

@@ -39,6 +39,7 @@ pub const Browser = struct {
     session: ?Session,
     allocator: Allocator,
     http_client: *HttpClient,
+    call_arena: ArenaAllocator,
     page_arena: ArenaAllocator,
     session_arena: ArenaAllocator,
     transfer_arena: ArenaAllocator,
@@ -63,6 +64,7 @@ pub const Browser = struct {
             .allocator = allocator,
             .notification = notification,
             .http_client = app.http.client,
+            .call_arena = ArenaAllocator.init(allocator),
             .page_arena = ArenaAllocator.init(allocator),
             .session_arena = ArenaAllocator.init(allocator),
             .transfer_arena = ArenaAllocator.init(allocator),
@@ -73,6 +75,7 @@ pub const Browser = struct {
     pub fn deinit(self: *Browser) void {
         self.closeSession();
         self.env.deinit();
+        self.call_arena.deinit();
         self.page_arena.deinit();
         self.session_arena.deinit();
         self.transfer_arena.deinit();

src/browser/js/Context.zig

@@ -35,12 +35,11 @@ templates: []v8.FunctionTemplate,
 // references the Env.meta_lookup
 meta_lookup: []types.Meta,
 
-// An arena for the lifetime of a call-group. Gets reset whenever
-// call_depth reaches 0.
-call_arena: Allocator,
+// Arena for the lifetime of the context
+arena: Allocator,
 
-// An arena for the lifetime of the context
-context_arena: Allocator,
+// The page.call_arena
+call_arena: Allocator,
 
 // 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
@@ -179,7 +178,7 @@ pub fn deinit(self: *Context) void {
 }
 
 fn trackCallback(self: *Context, pf: PersistentFunction) !void {
-    return self.callbacks.append(self.context_arena, pf);
+    return self.callbacks.append(self.arena, pf);
 }
 
 // Given an anytype, turns it into a v8.Object. The anytype could be:
@@ -236,7 +235,7 @@ pub fn module(self: *Context, comptime want_result: bool, src: []const u8, url:
 
     const m = try compileModule(self.isolate, src, url);
 
-    const arena = self.context_arena;
+    const arena = self.arena;
     const owned_url = try arena.dupe(u8, url);
 
     try self.module_identifier.putNoClobber(arena, m.getIdentityHash(), owned_url);
@@ -258,9 +257,9 @@ pub fn module(self: *Context, comptime want_result: bool, src: []const u8, url:
                 owned_url,
                 .{ .alloc = .if_needed, .null_terminated = true },
             );
-            const gop = try self.module_cache.getOrPut(self.context_arena, normalized_specifier);
+            const gop = try self.module_cache.getOrPut(self.arena, normalized_specifier);
             if (!gop.found_existing) {
-                const owned_specifier = try self.context_arena.dupeZ(u8, normalized_specifier);
+                const owned_specifier = try self.arena.dupeZ(u8, normalized_specifier);
                 gop.key_ptr.* = owned_specifier;
                 gop.value_ptr.* = .{};
                 try self.script_manager.?.getModule(owned_specifier);
@@ -522,26 +521,26 @@ pub fn zigValueToJs(self: *Context, value: anytype) !v8.Value {
 //      we can just grab it from the identity_map)
 pub fn mapZigInstanceToJs(self: *Context, js_obj_or_template: anytype, value: anytype) !PersistentObject {
     const v8_context = self.v8_context;
-    const context_arena = self.context_arena;
+    const arena = self.arena;
 
     const T = @TypeOf(value);
     switch (@typeInfo(T)) {
         .@"struct" => {
             // Struct, has to be placed on the heap
-            const heap = try context_arena.create(T);
+            const heap = try arena.create(T);
             heap.* = value;
             return self.mapZigInstanceToJs(js_obj_or_template, heap);
         },
         .pointer => |ptr| {
-            const gop = try self.identity_map.getOrPut(context_arena, @intFromPtr(value));
+            const gop = try self.identity_map.getOrPut(arena, @intFromPtr(value));
             if (gop.found_existing) {
                 // we've seen this instance before, return the same
                 // PersistentObject.
                 return gop.value_ptr.*;
             }
 
             if (comptime @hasDecl(ptr.child, "destructor")) {
-                try self.destructor_callbacks.append(context_arena, DestructorCallback.init(value));
+                try self.destructor_callbacks.append(arena, DestructorCallback.init(value));
             }
 
             // Sometimes we're creating a new v8.Object, like when
@@ -563,7 +562,7 @@ pub fn mapZigInstanceToJs(self: *Context, js_obj_or_template: anytype, value: an
                 // The TAO contains the pointer ot our Zig instance as
                 // well as any meta data we'll need to use it later.
                 // See the TaggedAnyOpaque struct for more details.
-                const tao = try context_arena.create(TaggedAnyOpaque);
+                const tao = try arena.create(TaggedAnyOpaque);
                 const meta_index = @field(types.LOOKUP, @typeName(ptr.child));
                 const meta = self.meta_lookup[meta_index];
 
@@ -768,7 +767,7 @@ fn jsValueToStruct(self: *Context, comptime named_function: NamedFunction, compt
     }
 
     if (T == js.String) {
-        return .{ .string = try self.valueToString(js_value, .{ .allocator = self.context_arena }) };
+        return .{ .string = try self.valueToString(js_value, .{ .allocator = self.arena }) };
     }
 
     const js_obj = js_value.castTo(v8.Object);
@@ -1062,7 +1061,7 @@ pub fn createPromiseResolver(self: *Context, comptime lifetime: PromiseResolverL
     const persisted = v8.Persistent(v8.PromiseResolver).init(self.isolate, resolver);
 
     if (comptime lifetime == .page) {
-        try self.persisted_promise_resolvers.append(self.context_arena, persisted);
+        try self.persisted_promise_resolvers.append(self.arena, persisted);
     }
 
     return .{
@@ -1122,7 +1121,7 @@ pub fn dynamicModuleCallback(
     };
 
     const normalized_specifier = @import("../../url.zig").stitch(
-        self.context_arena, // might need to survive until the module is loaded
+        self.arena, // might need to survive until the module is loaded
         specifier,
         resource,
         .{ .alloc = .if_needed, .null_terminated = true },
@@ -1172,7 +1171,7 @@ fn _resolveModuleCallback(self: *Context, referrer: v8.Module, specifier: []cons
         .{ .alloc = .if_needed, .null_terminated = true },
     );
 
-    const gop = try self.module_cache.getOrPut(self.context_arena, normalized_specifier);
+    const gop = try self.module_cache.getOrPut(self.arena, normalized_specifier);
     if (gop.found_existing) {
         if (gop.value_ptr.module) |m| {
             return m.handle;
@@ -1229,7 +1228,7 @@ const DynamicModuleResolveState = struct {
 
 fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8) !v8.Promise {
     const isolate = self.isolate;
-    const gop = try self.module_cache.getOrPut(self.context_arena, specifier);
+    const gop = try self.module_cache.getOrPut(self.arena, specifier);
     if (gop.found_existing and gop.value_ptr.resolver_promise != null) {
         // This is easy, there's already something responsible
         // for loading the module. Maybe it's still loading, maybe
@@ -1238,10 +1237,10 @@ fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8) !v8.Promise {
     }
 
     const persistent_resolver = v8.Persistent(v8.PromiseResolver).init(isolate, v8.PromiseResolver.init(self.v8_context));
-    try self.persisted_promise_resolvers.append(self.context_arena, persistent_resolver);
+    try self.persisted_promise_resolvers.append(self.arena, persistent_resolver);
     var resolver = persistent_resolver.castToPromiseResolver();
 
-    const state = try self.context_arena.create(DynamicModuleResolveState);
+    const state = try self.arena.create(DynamicModuleResolveState);
     state.* = .{
         .module = null,
         .context = self,

src/browser/js/Env.zig

@@ -178,7 +178,6 @@ pub fn newExecutionWorld(self: *Env) !ExecutionWorld {
     return .{
         .env = self,
         .context = null,
-        .call_arena = ArenaAllocator.init(self.allocator),
         .context_arena = ArenaAllocator.init(self.allocator),
     };
 }

src/browser/js/ExecutionWorld.zig

@@ -21,13 +21,6 @@ const CONTEXT_ARENA_RETAIN = 1024 * 64;
 const ExecutionWorld = @This();
 env: *Env,
 
-// 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 Context, but the
-// arena itself is owned by the ExecutionWorld so that we can re-use it
-// from context to context.
-call_arena: ArenaAllocator,
-
 // Arena whose lifetime is for a single page load. Where
 // the call_arena lives for a single function call, the context_arena
 // lives for the lifetime of the entire page. The allocator will be
@@ -48,7 +41,6 @@ pub fn deinit(self: *ExecutionWorld) void {
         self.removeContext();
     }
 
-    self.call_arena.deinit();
     self.context_arena.deinit();
 }
 
@@ -178,8 +170,8 @@ pub fn createContext(self: *ExecutionWorld, page: *Page, enter: bool, global_cal
         .meta_lookup = &env.meta_lookup,
         .handle_scope = handle_scope,
         .script_manager = &page.script_manager,
-        .call_arena = self.call_arena.allocator(),
-        .context_arena = self.context_arena.allocator(),
+        .call_arena = page.call_arena,
+        .arena = self.context_arena.allocator(),
         .global_callback = global_callback,
     };
 
@@ -191,8 +183,6 @@ pub fn createContext(self: *ExecutionWorld, page: *Page, enter: bool, global_cal
         v8_context.setEmbedderData(1, data);
     }
 
-    page.call_arena = context.call_arena;
-
     // Custom exception
     // NOTE: there is no way in v8 to subclass the Error built-in type
     // TODO: this is an horrible hack

src/browser/js/Object.zig

@@ -22,7 +22,7 @@ pub fn setIndex(self: Object, index: u32, value: anytype, opts: SetOpts) !void {
     @setEvalBranchQuota(10000);
     const key = switch (index) {
         inline 0...20 => |i| std.fmt.comptimePrint("{d}", .{i}),
-        else => try std.fmt.allocPrint(self.context.context_arena, "{d}", .{index}),
+        else => try std.fmt.allocPrint(self.context.arena, "{d}", .{index}),
     };
     return self.set(key, value, opts);
 }
@@ -76,7 +76,7 @@ pub fn persist(self: Object) !Object {
     const js_obj = self.js_obj;
 
     const persisted = PersistentObject.init(context.isolate, js_obj);
-    try context.js_object_list.append(context.context_arena, persisted);
+    try context.js_object_list.append(context.arena, persisted);
 
     return .{
         .context = context,

src/browser/page.zig

@@ -121,7 +121,7 @@ pub const Page = struct {
         complete,
     };
 
-    pub fn init(self: *Page, arena: Allocator, session: *Session) !void {
+    pub fn init(self: *Page, arena: Allocator, call_arena: Allocator, session: *Session) !void {
         const browser = session.browser;
         const script_manager = ScriptManager.init(browser, self);
 
@@ -131,7 +131,7 @@ pub const Page = struct {
             .window = try Window.create(null, null),
             .arena = arena,
             .session = session,
-            .call_arena = undefined,
+            .call_arena = call_arena,
             .renderer = Renderer.init(arena),
             .state_pool = &browser.state_pool,
             .cookie_jar = &session.cookie_jar,

src/browser/session.zig

@@ -106,7 +106,7 @@ pub const Session = struct {
 
         self.page = @as(Page, undefined);
         const page = &self.page.?;
-        try Page.init(page, page_arena.allocator(), self);
+        try Page.init(page, page_arena.allocator(), self.browser.call_arena.allocator(), self);
 
         log.debug(.browser, "create page", .{});
         // start JS env

src/http/Http.zig

@@ -443,7 +443,6 @@ const LineWriter = struct {
     }
 };
 
-
 pub fn debugCallback(_: *c.CURL, msg_type: c.curl_infotype, raw: [*c]u8, len: usize, _: *anyopaque) callconv(.c) void {
     const data = raw[0..len];
     switch (msg_type) {