From a70c3cefb885a3a3d2341b8d9500b383a7ed90ff Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 11 Jun 2026 14:19:31 +0800 Subject: [PATCH] perf, http: Preload modules Similar to https://github.com/lightpanda-io/browser/pull/2675. That PR added support for rel=preload, this one adds it for rel=modulepreload. The impact here is generally less significant. Modules were already asynchronously preloaded as part of the module-loading flow. What this does is potentially start the preload earlier (i.e. when the link is encountered). You can imagine: // pseudo-html If s1.js imports mod1 and mod2, then there won't be a huge difference...I mean mod1 and mod2 can start loading before s1.js is complete, but it won't be huge. If s1.js doesn't import mod1 and/or mod2, but s2.js does, then the gain is a little more (because now we have to wait for both s1 and s2 to load). So it really depends. This commit also better manages preloaded scripts. There were common cases where a preloaded script might remain preloaded until page teardown..well past its actual usage. This happened because, while every preload increments the waiters count, only the first to fetch it decrements it. Once cached, waiters was never decremented. This is now fixed. --- src/browser/Frame.zig | 17 ++++ src/browser/ScriptManagerBase.zig | 83 +++++++++++++++++-- src/browser/js/Context.zig | 11 ++- src/browser/tests/element/html/link.html | 6 +- .../element/html/script/modulepreload.html | 57 +++++++++++++ .../element/html/script/modulepreload.js | 1 + .../html/script/modulepreload_dynamic.js | 1 + .../html/script/modulepreload_dynamic2.js | 1 + .../html/script/modulepreload_unused.js | 5 ++ src/browser/tests/page/module.html | 12 +++ src/browser/tests/page/modules/diamond-a.js | 1 + src/browser/tests/page/modules/diamond-b.js | 1 + .../tests/page/modules/diamond-shared.js | 1 + src/browser/webapi/element/html/Link.zig | 28 +++---- 14 files changed, 199 insertions(+), 26 deletions(-) create mode 100644 src/browser/tests/element/html/script/modulepreload.html create mode 100644 src/browser/tests/element/html/script/modulepreload.js create mode 100644 src/browser/tests/element/html/script/modulepreload_dynamic.js create mode 100644 src/browser/tests/element/html/script/modulepreload_dynamic2.js create mode 100644 src/browser/tests/element/html/script/modulepreload_unused.js create mode 100644 src/browser/tests/page/modules/diamond-a.js create mode 100644 src/browser/tests/page/modules/diamond-b.js create mode 100644 src/browser/tests/page/modules/diamond-shared.js diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index bbe669fc2..a9a437f5e 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -1714,6 +1714,23 @@ pub fn preloadScriptHint(self: *Frame, href: []const u8) void { self._script_manager.preloadScript(resolved) catch {}; } +// start prefetching +pub fn preloadModuleHint(self: *Frame, href: []const u8) void { + if (self.isGoingAway() or self._parse_mode == .fragment) { + return; + } + + // The url becomes the imported_modules key, which must outlive the fetch + // so it lives on the frame arena + const resolved = URL.resolve(self.arena, self.base(), href, .{ .encoding = self.charset }) catch return; + if (!std.ascii.startsWithIgnoreCase(resolved, "http:") and !std.ascii.startsWithIgnoreCase(resolved, "https:")) { + // data:/blob: are synthesized locally — no round-trip to hide. + return; + } + + self._script_manager.base.preloadModuleHint(resolved, self.url) catch {}; +} + // Synchronously fetch and parse an external ``. // href is passed in as an optimization since the [currently] only callsite has // it, so why look it up again? diff --git a/src/browser/ScriptManagerBase.zig b/src/browser/ScriptManagerBase.zig index f917ced60..a1969501f 100644 --- a/src/browser/ScriptManagerBase.zig +++ b/src/browser/ScriptManagerBase.zig @@ -217,10 +217,19 @@ pub fn resolveSpecifier(self: *ScriptManagerBase, arena: Allocator, base: [:0]co return error.SpecifierResolutionFailed; } -pub fn preloadImport(self: *ScriptManagerBase, url: [:0]const u8, referrer: []const u8) !void { +const PreloadOpts = struct { + hint: bool = false, +}; +pub fn preloadImport(self: *ScriptManagerBase, url: [:0]const u8, referrer: []const u8, opts: PreloadOpts) !void { const gop = try self.imported_modules.getOrPut(self.allocator, url); if (gop.found_existing) { - gop.value_ptr.waiters += 1; + if (gop.value_ptr.hint) { + // A never calls waitForImport, so the + // first real import adopts its waiter slot rather than adding one. + gop.value_ptr.hint = false; + } else { + gop.value_ptr.waiters += 1; + } return; } errdefer _ = self.imported_modules.remove(url); @@ -239,7 +248,7 @@ pub fn preloadImport(self: *ScriptManagerBase, url: [:0]const u8, referrer: []co .extra = .import, }; - gop.value_ptr.* = ImportedModule{}; + gop.value_ptr.* = .{ .state = .{ .loading = script }, .hint = opts.hint }; if (comptime IS_DEBUG) { var ls: js.Local.Scope = undefined; @@ -284,6 +293,15 @@ pub fn preloadImport(self: *ScriptManagerBase, url: [:0]const u8, referrer: []co }; } +// — start fetching a module before import +// resolution discovers it. +pub fn preloadModuleHint(self: *ScriptManagerBase, url: [:0]const u8, referrer: []const u8) !void { + if (self.imported_modules.contains(url)) { + return; + } + try self.preloadImport(url, referrer, .{ .hint = true }); +} + pub fn waitForImport(self: *ScriptManagerBase, url: [:0]const u8) !ModuleSource { const entry = self.imported_modules.getEntry(url) orelse { // It shouldn't be possible for v8 to ask for a module that we didn't @@ -324,7 +342,57 @@ pub fn waitForImport(self: *ScriptManagerBase, url: [:0]const u8) !ModuleSource } } +pub fn releaseImport(self: *ScriptManagerBase, url: [:0]const u8) void { + const entry = self.imported_modules.getEntry(url) orelse { + return; + }; + if (entry.value_ptr.waiters > 1) { + entry.value_ptr.waiters -= 1; + return; + } + switch (entry.value_ptr.state) { + .done => |script| script.deinit(), + .loading, .err => return, + } + self.imported_modules.removeByPtr(entry.key_ptr); +} + pub fn getAsyncImport(self: *ScriptManagerBase, url: [:0]const u8, cb: ImportAsync.Callback, cb_data: *anyopaque, referrer: []const u8) !void { + // A hint may already be fetching/fetched this module + if (self.imported_modules.getEntry(url)) |entry| { + if (entry.value_ptr.hint) { + switch (entry.value_ptr.state) { + .loading => |script| { + // fetch is in flight, take the script and turn it into + // our normal getAsyncImport flow (e.g. what we do at the + // end of this file as-if imported_modules didn't have this script) + if (comptime IS_DEBUG) { + log.debug(.http, "script adopt", .{ .url = url, .ctx = "dynamic module", .state = "loading" }); + } + script.extra = .{ .import_async = .{ .callback = cb, .data = cb_data } }; + self.imported_modules.removeByPtr(entry.key_ptr); + return; + }, + .done => |script| { + // fetch is complete; deliver through the normal + // ready_scripts flow. evaluate() runs it now, or — if an + // evaluation window is open — evaluate_pending runs it + // when that window closes. + if (comptime IS_DEBUG) { + log.debug(.http, "script adopt", .{ .url = url, .ctx = "dynamic module", .state = "done" }); + } + script.extra = .{ .import_async = .{ .callback = cb, .data = cb_data } }; + self.imported_modules.removeByPtr(entry.key_ptr); + self.ready_scripts.append(&script.node); + self.evaluate(); + return; + }, + // The hint's fetch failed; give the import its own attempt. + .err => {}, + } + } + } + const arena = try self.acquireArena(.large, "SM.getAsyncImport"); errdefer self.releaseArena(arena); @@ -890,12 +958,17 @@ pub const ModuleSource = struct { pub const ImportedModule = struct { waiters: u16 = 1, - state: State = .loading, + // Created by a hint and not yet claimed by a real + // import. While set, the single waiter slot belongs to the hint, which + // will never collect it (see preloadModuleHint). A dynamic import may + // adopt a hint entry outright (see getAsyncImport). + hint: bool = false, + state: State, buffer: std.ArrayList(u8) = .{}, pub const State = union(enum) { err, - loading, + loading: *Script, done: *Script, }; }; diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index af39fe0a8..572e12741 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -516,7 +516,7 @@ fn postCompileModule(self: *Context, mod: js.Module, url: [:0]const u8, local: * const owned_specifier = try self.arena.dupeZ(u8, normalized_specifier); nested_gop.key_ptr.* = owned_specifier; nested_gop.value_ptr.* = .{}; - try script_manager.preloadImport(owned_specifier, url); + try script_manager.preloadImport(owned_specifier, url, .{}); } else if (nested_gop.value_ptr.module == null) { // Entry exists but module failed to compile previously. // The imported_modules entry may have been consumed, so @@ -524,7 +524,7 @@ fn postCompileModule(self: *Context, mod: js.Module, url: [:0]const u8, local: * // Key was stored via dupeZ so it has a sentinel in memory. const key = nested_gop.key_ptr.*; const key_z: [:0]const u8 = key.ptr[0..key.len :0]; - try script_manager.preloadImport(key_z, url); + try script_manager.preloadImport(key_z, url, .{}); } } } @@ -733,6 +733,11 @@ fn _resolveModuleCallback(self: *Context, referrer: js.Module, specifier: [:0]co const entry = self.module_cache.getPtr(normalized_specifier).?; if (entry.module) |m| { + // This import registered a waiter via preloadImport when it was discovered + // but the compiled module is already cached so we don't have to call + // waitForImport. Release our waiter so we no longer hold on waiter on + // the resource. + self.script_manager.releaseImport(normalized_specifier); return local.toLocal(m).handle; } @@ -740,7 +745,7 @@ fn _resolveModuleCallback(self: *Context, referrer: js.Module, specifier: [:0]co error.UnknownModule => blk: { // Module is in cache but was consumed from imported_modules // (e.g., by a previous failed resolution). Re-preload and retry. - try self.script_manager.preloadImport(normalized_specifier, referrer_path); + try self.script_manager.preloadImport(normalized_specifier, referrer_path, .{}); break :blk try self.script_manager.waitForImport(normalized_specifier); }, else => return err, diff --git a/src/browser/tests/element/html/link.html b/src/browser/tests/element/html/link.html index d3f2faa0f..754fe27e7 100644 --- a/src/browser/tests/element/html/link.html +++ b/src/browser/tests/element/html/link.html @@ -120,11 +120,15 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/src/browser/tests/element/html/script/modulepreload.js b/src/browser/tests/element/html/script/modulepreload.js new file mode 100644 index 000000000..fed413f5b --- /dev/null +++ b/src/browser/tests/element/html/script/modulepreload.js @@ -0,0 +1 @@ +export const val = 'preloaded-module'; diff --git a/src/browser/tests/element/html/script/modulepreload_dynamic.js b/src/browser/tests/element/html/script/modulepreload_dynamic.js new file mode 100644 index 000000000..d8c3e9858 --- /dev/null +++ b/src/browser/tests/element/html/script/modulepreload_dynamic.js @@ -0,0 +1 @@ +export const dval = 'dynamic-preloaded'; diff --git a/src/browser/tests/element/html/script/modulepreload_dynamic2.js b/src/browser/tests/element/html/script/modulepreload_dynamic2.js new file mode 100644 index 000000000..e9983ba8f --- /dev/null +++ b/src/browser/tests/element/html/script/modulepreload_dynamic2.js @@ -0,0 +1 @@ +export const dval = 'dynamic-preloaded-2'; diff --git a/src/browser/tests/element/html/script/modulepreload_unused.js b/src/browser/tests/element/html/script/modulepreload_unused.js new file mode 100644 index 000000000..3535633b2 --- /dev/null +++ b/src/browser/tests/element/html/script/modulepreload_unused.js @@ -0,0 +1,5 @@ +// Nothing imports this module, so it must never be evaluated. If it runs, the +// flag trips the assertion in modulepreload.html (and this fail() fires +// directly). +window.modulepreload_unused_ran = true; +testing.fail('an unconsumed must not evaluate'); diff --git a/src/browser/tests/page/module.html b/src/browser/tests/page/module.html index bb820332b..bba90ebe5 100644 --- a/src/browser/tests/page/module.html +++ b/src/browser/tests/page/module.html @@ -58,6 +58,18 @@ testing.expectEqual('a', getFromA()); + +