Skip to content

Commit 381a18a

Browse files
committed
Move the call_arena to the page.
The call_arena was previously owned by the js.Context, but it has to exist on the page, and the page is created before the context, so it's set to undefined on the page. While this has never caused an issue, there's no reason for the page not to own this, and the context to simply reference it. Also, renamed the js.Context.context_arena to simply `arena`, which is more consistent with other arena names (e.g. page.arena).
1 parent 207f065 commit 381a18a

File tree

8 files changed

+30
-40
lines changed

8 files changed

+30
-40
lines changed

src/browser/browser.zig

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ pub const Browser = struct {
3939
session: ?Session,
4040
allocator: Allocator,
4141
http_client: *HttpClient,
42+
call_arena: ArenaAllocator,
4243
page_arena: ArenaAllocator,
4344
session_arena: ArenaAllocator,
4445
transfer_arena: ArenaAllocator,
@@ -63,6 +64,7 @@ pub const Browser = struct {
6364
.allocator = allocator,
6465
.notification = notification,
6566
.http_client = app.http.client,
67+
.call_arena = ArenaAllocator.init(allocator),
6668
.page_arena = ArenaAllocator.init(allocator),
6769
.session_arena = ArenaAllocator.init(allocator),
6870
.transfer_arena = ArenaAllocator.init(allocator),
@@ -73,6 +75,7 @@ pub const Browser = struct {
7375
pub fn deinit(self: *Browser) void {
7476
self.closeSession();
7577
self.env.deinit();
78+
self.call_arena.deinit();
7679
self.page_arena.deinit();
7780
self.session_arena.deinit();
7881
self.transfer_arena.deinit();

src/browser/js/Context.zig

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,11 @@ templates: []v8.FunctionTemplate,
3535
// references the Env.meta_lookup
3636
meta_lookup: []types.Meta,
3737

38-
// An arena for the lifetime of a call-group. Gets reset whenever
39-
// call_depth reaches 0.
40-
call_arena: Allocator,
38+
// Arena for the lifetime of the context
39+
arena: Allocator,
4140

42-
// An arena for the lifetime of the context
43-
context_arena: Allocator,
41+
// The page.call_arena
42+
call_arena: Allocator,
4443

4544
// Because calls can be nested (i.e.a function calling a callback),
4645
// we can only reset the call_arena when call_depth == 0. If we were
@@ -179,7 +178,7 @@ pub fn deinit(self: *Context) void {
179178
}
180179

181180
fn trackCallback(self: *Context, pf: PersistentFunction) !void {
182-
return self.callbacks.append(self.context_arena, pf);
181+
return self.callbacks.append(self.arena, pf);
183182
}
184183

185184
// 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:
236235

237236
const m = try compileModule(self.isolate, src, url);
238237

239-
const arena = self.context_arena;
238+
const arena = self.arena;
240239
const owned_url = try arena.dupe(u8, url);
241240

242241
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:
258257
owned_url,
259258
.{ .alloc = .if_needed, .null_terminated = true },
260259
);
261-
const gop = try self.module_cache.getOrPut(self.context_arena, normalized_specifier);
260+
const gop = try self.module_cache.getOrPut(self.arena, normalized_specifier);
262261
if (!gop.found_existing) {
263-
const owned_specifier = try self.context_arena.dupeZ(u8, normalized_specifier);
262+
const owned_specifier = try self.arena.dupeZ(u8, normalized_specifier);
264263
gop.key_ptr.* = owned_specifier;
265264
gop.value_ptr.* = .{};
266265
try self.script_manager.?.getModule(owned_specifier);
@@ -522,26 +521,26 @@ pub fn zigValueToJs(self: *Context, value: anytype) !v8.Value {
522521
// we can just grab it from the identity_map)
523522
pub fn mapZigInstanceToJs(self: *Context, js_obj_or_template: anytype, value: anytype) !PersistentObject {
524523
const v8_context = self.v8_context;
525-
const context_arena = self.context_arena;
524+
const arena = self.arena;
526525

527526
const T = @TypeOf(value);
528527
switch (@typeInfo(T)) {
529528
.@"struct" => {
530529
// Struct, has to be placed on the heap
531-
const heap = try context_arena.create(T);
530+
const heap = try arena.create(T);
532531
heap.* = value;
533532
return self.mapZigInstanceToJs(js_obj_or_template, heap);
534533
},
535534
.pointer => |ptr| {
536-
const gop = try self.identity_map.getOrPut(context_arena, @intFromPtr(value));
535+
const gop = try self.identity_map.getOrPut(arena, @intFromPtr(value));
537536
if (gop.found_existing) {
538537
// we've seen this instance before, return the same
539538
// PersistentObject.
540539
return gop.value_ptr.*;
541540
}
542541

543542
if (comptime @hasDecl(ptr.child, "destructor")) {
544-
try self.destructor_callbacks.append(context_arena, DestructorCallback.init(value));
543+
try self.destructor_callbacks.append(arena, DestructorCallback.init(value));
545544
}
546545

547546
// 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
563562
// The TAO contains the pointer ot our Zig instance as
564563
// well as any meta data we'll need to use it later.
565564
// See the TaggedAnyOpaque struct for more details.
566-
const tao = try context_arena.create(TaggedAnyOpaque);
565+
const tao = try arena.create(TaggedAnyOpaque);
567566
const meta_index = @field(types.LOOKUP, @typeName(ptr.child));
568567
const meta = self.meta_lookup[meta_index];
569568

@@ -768,7 +767,7 @@ fn jsValueToStruct(self: *Context, comptime named_function: NamedFunction, compt
768767
}
769768

770769
if (T == js.String) {
771-
return .{ .string = try self.valueToString(js_value, .{ .allocator = self.context_arena }) };
770+
return .{ .string = try self.valueToString(js_value, .{ .allocator = self.arena }) };
772771
}
773772

774773
const js_obj = js_value.castTo(v8.Object);
@@ -1062,7 +1061,7 @@ pub fn createPromiseResolver(self: *Context, comptime lifetime: PromiseResolverL
10621061
const persisted = v8.Persistent(v8.PromiseResolver).init(self.isolate, resolver);
10631062

10641063
if (comptime lifetime == .page) {
1065-
try self.persisted_promise_resolvers.append(self.context_arena, persisted);
1064+
try self.persisted_promise_resolvers.append(self.arena, persisted);
10661065
}
10671066

10681067
return .{
@@ -1122,7 +1121,7 @@ pub fn dynamicModuleCallback(
11221121
};
11231122

11241123
const normalized_specifier = @import("../../url.zig").stitch(
1125-
self.context_arena, // might need to survive until the module is loaded
1124+
self.arena, // might need to survive until the module is loaded
11261125
specifier,
11271126
resource,
11281127
.{ .alloc = .if_needed, .null_terminated = true },
@@ -1172,7 +1171,7 @@ fn _resolveModuleCallback(self: *Context, referrer: v8.Module, specifier: []cons
11721171
.{ .alloc = .if_needed, .null_terminated = true },
11731172
);
11741173

1175-
const gop = try self.module_cache.getOrPut(self.context_arena, normalized_specifier);
1174+
const gop = try self.module_cache.getOrPut(self.arena, normalized_specifier);
11761175
if (gop.found_existing) {
11771176
if (gop.value_ptr.module) |m| {
11781177
return m.handle;
@@ -1229,7 +1228,7 @@ const DynamicModuleResolveState = struct {
12291228

12301229
fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8) !v8.Promise {
12311230
const isolate = self.isolate;
1232-
const gop = try self.module_cache.getOrPut(self.context_arena, specifier);
1231+
const gop = try self.module_cache.getOrPut(self.arena, specifier);
12331232
if (gop.found_existing and gop.value_ptr.resolver_promise != null) {
12341233
// This is easy, there's already something responsible
12351234
// for loading the module. Maybe it's still loading, maybe
@@ -1238,10 +1237,10 @@ fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8) !v8.Promise {
12381237
}
12391238

12401239
const persistent_resolver = v8.Persistent(v8.PromiseResolver).init(isolate, v8.PromiseResolver.init(self.v8_context));
1241-
try self.persisted_promise_resolvers.append(self.context_arena, persistent_resolver);
1240+
try self.persisted_promise_resolvers.append(self.arena, persistent_resolver);
12421241
var resolver = persistent_resolver.castToPromiseResolver();
12431242

1244-
const state = try self.context_arena.create(DynamicModuleResolveState);
1243+
const state = try self.arena.create(DynamicModuleResolveState);
12451244
state.* = .{
12461245
.module = null,
12471246
.context = self,

src/browser/js/Env.zig

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,6 @@ pub fn newExecutionWorld(self: *Env) !ExecutionWorld {
178178
return .{
179179
.env = self,
180180
.context = null,
181-
.call_arena = ArenaAllocator.init(self.allocator),
182181
.context_arena = ArenaAllocator.init(self.allocator),
183182
};
184183
}

src/browser/js/ExecutionWorld.zig

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,6 @@ const CONTEXT_ARENA_RETAIN = 1024 * 64;
2121
const ExecutionWorld = @This();
2222
env: *Env,
2323

24-
// Arena whose lifetime is for a single getter/setter/function/etc.
25-
// Largely used to get strings out of V8, like a stack trace from
26-
// a TryCatch. The allocator will be owned by the Context, but the
27-
// arena itself is owned by the ExecutionWorld so that we can re-use it
28-
// from context to context.
29-
call_arena: ArenaAllocator,
30-
3124
// Arena whose lifetime is for a single page load. Where
3225
// the call_arena lives for a single function call, the context_arena
3326
// lives for the lifetime of the entire page. The allocator will be
@@ -48,7 +41,6 @@ pub fn deinit(self: *ExecutionWorld) void {
4841
self.removeContext();
4942
}
5043

51-
self.call_arena.deinit();
5244
self.context_arena.deinit();
5345
}
5446

@@ -178,8 +170,8 @@ pub fn createContext(self: *ExecutionWorld, page: *Page, enter: bool, global_cal
178170
.meta_lookup = &env.meta_lookup,
179171
.handle_scope = handle_scope,
180172
.script_manager = &page.script_manager,
181-
.call_arena = self.call_arena.allocator(),
182-
.context_arena = self.context_arena.allocator(),
173+
.call_arena = page.call_arena,
174+
.arena = self.context_arena.allocator(),
183175
.global_callback = global_callback,
184176
};
185177

@@ -191,8 +183,6 @@ pub fn createContext(self: *ExecutionWorld, page: *Page, enter: bool, global_cal
191183
v8_context.setEmbedderData(1, data);
192184
}
193185

194-
page.call_arena = context.call_arena;
195-
196186
// Custom exception
197187
// NOTE: there is no way in v8 to subclass the Error built-in type
198188
// TODO: this is an horrible hack

src/browser/js/Object.zig

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ pub fn setIndex(self: Object, index: u32, value: anytype, opts: SetOpts) !void {
2222
@setEvalBranchQuota(10000);
2323
const key = switch (index) {
2424
inline 0...20 => |i| std.fmt.comptimePrint("{d}", .{i}),
25-
else => try std.fmt.allocPrint(self.context.context_arena, "{d}", .{index}),
25+
else => try std.fmt.allocPrint(self.context.arena, "{d}", .{index}),
2626
};
2727
return self.set(key, value, opts);
2828
}
@@ -76,7 +76,7 @@ pub fn persist(self: Object) !Object {
7676
const js_obj = self.js_obj;
7777

7878
const persisted = PersistentObject.init(context.isolate, js_obj);
79-
try context.js_object_list.append(context.context_arena, persisted);
79+
try context.js_object_list.append(context.arena, persisted);
8080

8181
return .{
8282
.context = context,

src/browser/page.zig

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ pub const Page = struct {
121121
complete,
122122
};
123123

124-
pub fn init(self: *Page, arena: Allocator, session: *Session) !void {
124+
pub fn init(self: *Page, arena: Allocator, call_arena: Allocator, session: *Session) !void {
125125
const browser = session.browser;
126126
const script_manager = ScriptManager.init(browser, self);
127127

@@ -131,7 +131,7 @@ pub const Page = struct {
131131
.window = try Window.create(null, null),
132132
.arena = arena,
133133
.session = session,
134-
.call_arena = undefined,
134+
.call_arena = call_arena,
135135
.renderer = Renderer.init(arena),
136136
.state_pool = &browser.state_pool,
137137
.cookie_jar = &session.cookie_jar,

src/browser/session.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ pub const Session = struct {
106106

107107
self.page = @as(Page, undefined);
108108
const page = &self.page.?;
109-
try Page.init(page, page_arena.allocator(), self);
109+
try Page.init(page, page_arena.allocator(), self.browser.call_arena.allocator(), self);
110110

111111
log.debug(.browser, "create page", .{});
112112
// start JS env

src/http/Http.zig

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,6 @@ const LineWriter = struct {
443443
}
444444
};
445445

446-
447446
pub fn debugCallback(_: *c.CURL, msg_type: c.curl_infotype, raw: [*c]u8, len: usize, _: *anyopaque) callconv(.c) void {
448447
const data = raw[0..len];
449448
switch (msg_type) {

0 commit comments

Comments
 (0)