Skip to content

Commit 54a7df8

Browse files
committed
Move call/control of gc_hint to browser.
It has more context than the env about when this should be called. Specifically it can be called once per session, whereas, in the env, we can only call it once per context - which could be too often.
1 parent 17ed502 commit 54a7df8

File tree

2 files changed

+18
-23
lines changed

2 files changed

+18
-23
lines changed

src/browser/browser.zig

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ pub const Browser = struct {
6060
pub fn init(app: *App) !Browser {
6161
const allocator = app.allocator;
6262

63-
const env = try Env.init(allocator, .{
64-
.gc_hints = app.config.gc_hints,
65-
});
63+
const env = try Env.init(allocator, .{});
6664
errdefer env.deinit();
6765

6866
const notification = try Notification.init(allocator, app.notification);
@@ -98,6 +96,9 @@ pub const Browser = struct {
9896
if (self.session) |*session| {
9997
session.deinit();
10098
self.session = null;
99+
if (self.app.config.gc_hints) {
100+
self.env.lowMemoryNotification();
101+
}
101102
}
102103
}
103104

src/runtime/js.zig

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -174,18 +174,13 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
174174
// index.
175175
prototype_lookup: [Types.len]u16,
176176

177-
// Send a lowMemoryNotification whenever we stop an executor
178-
gc_hints: bool,
179-
180177
const Self = @This();
181178

182179
const TYPE_LOOKUP = TypeLookup{};
183180

184-
const Opts = struct {
185-
gc_hints: bool = false,
186-
};
181+
const Opts = struct {};
187182

188-
pub fn init(allocator: Allocator, opts: Opts) !*Self {
183+
pub fn init(allocator: Allocator, _: Opts) !*Self {
189184
// var params = v8.initCreateParams();
190185
var params = try allocator.create(v8.CreateParams);
191186
errdefer allocator.destroy(params);
@@ -213,7 +208,6 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
213208
.templates = undefined,
214209
.allocator = allocator,
215210
.isolate_params = params,
216-
.gc_hints = opts.gc_hints,
217211
.prototype_lookup = undefined,
218212
};
219213

@@ -274,6 +268,18 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
274268
};
275269
}
276270

271+
// V8 doesn't immediately free memory associated with
272+
// a Context, it's managed by the garbage collector. So, when the
273+
// `gc_hints` option is enabled, we'll use the `lowMemoryNotification`
274+
// call on the isolate to encourage v8 to free any contexts which
275+
// have been freed.
276+
pub fn lowMemoryNotification(self: *Self) void {
277+
var handle_scope: v8.HandleScope = undefined;
278+
v8.HandleScope.init(&handle_scope, self.isolate);
279+
defer handle_scope.deinit();
280+
self.isolate.lowMemoryNotification();
281+
}
282+
277283
pub const Executor = struct {
278284
env: *Self,
279285

@@ -304,18 +310,6 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
304310
self.endScope();
305311
}
306312

307-
// V8 doesn't immediately free memory associated with
308-
// a Context, it's managed by the garbage collector. So, when the
309-
// `gc_hints` option is enabled, we'll use the `lowMemoryNotification`
310-
// call on the isolate to encourage v8 to free any contexts which
311-
// have been freed.
312-
if (self.env.gc_hints) {
313-
var handle_scope: v8.HandleScope = undefined;
314-
v8.HandleScope.init(&handle_scope, self.env.isolate);
315-
defer handle_scope.deinit();
316-
317-
self.env.isolate.lowMemoryNotification(); // TODO we only need to call this for the main World Executor
318-
}
319313
self.call_arena.deinit();
320314
self.scope_arena.deinit();
321315
}

0 commit comments

Comments
 (0)