Our `Arena` from the `ArenaPool` now tracks if it's already been released. On
a subsequent release, it panics then and there. Without this, the code will
almost certainly panic anyways, but it will panic in a seemingly unrelated
place. Hopefully this makes identifying future cases of this easier (since we'll
get the stack trace of the re-releaser).
Fix 3 separate memory issues, all edge cases.
1 - an XHR error handler that re-open/sends would incorrectly null the new
transfer (maybe this isn't that odd, maybe it's a common retry-on-error).
2 - On a curl_easy_pause error (from WebSocket.zig) we now unqueue the just-
queued message, because the error will errdefer the message arena to be
cleaned up.
3 - ScriptManager now cleans up after itself on a failure prior to submit()
being called.
1 - Add a metric to track the number of inlight arenas from the pool
2 - Script now use 2 arenas:
- An initial (small) one for the script
- A sized one for the body
Should result in less pressure on our limited .large arenas
3 - DOMPoint and DOMPointRO are now arena free (they live on the slab only)
4 - TextDecoder no longer accumulate garbage in its arena
5 - Response object is much better at picking its arena size, rather than just
using a .large
Report memory held by arenas which are waiting on v8 finalization to v8. This
acts as a hint to the v8 GC, so that it knows about external memory which is
being held up by it.
This is opt-in, code needs to getArena() -> getPinnedArena() and then needs to
report() where it makes sense (e.g. no point reporting XHR memory if the Zig-
side still needs to instance).
Introduces a ArenaAllocator wrappre (lp.Arena). This is groundwork for better
memory tracking and reporting memory usage to v8. This is almost purely a
mechanical change to lay a foundation for a follow up PR that will address
https://github.com/lightpanda-io/browser/issues/3027
Some code became a bit leaner: a pooled arena can release itself (it has a
reference to the ArenaPool).
Some code became uglier: The Frame has a `_local_arena: *lp.Arena` and a
`local_arena: Allocator` (same with call_arena, and same with a few other types)
so that consumers aren't impacted (they continue to use `frame.local_arena`).
Built against https://github.com/lightpanda-io/zig-v8-fork/tree/zig-0.16 but
it doesn't require a new v8 build.
Built against https://github.com/lightpanda-io/boringssl-zig/tree/zig-0.16
since the current fork we point to isn't updated.
A global std.Io instance, lp.io. Way easier this way and requires 0 changes to
our libcurl integration / event loop.
Network code uses a new layer that does what Zig 0.15's posix package used to
do. Again, quicker migration that way. But, as long as we have the global IO,
and given the half-baked nature of networking in std.Io 0.16, this just makes
sense. Things can be migrated as needed.
The std.time.* -> std.Io.Timestamp/Clock/Duration resulted in _a lot_ of
changes. ArrayList = .{} -> ArrayList -> .empty also resulted in a lot of
changes, but that's obviously superficial. As is the trimLeft/trimRight ->
trimStart/trimEnd rename.
Locking adopt the `Uncancelable` variants, e.g. mutex.lockUncancelable() to
preserve the error-free signature (and, because cancellation would be something
we'd have to put more thought into).
std.json.ObjectMap is now unmanaged, so the allocator had to be passed along.
However, there's still a deprecated managed variant of MemoryPool, so I switched
to it (we can do a small follow up PR to move to the unmanaged after).
I tried use_llvm = false, but it locks my computer, consuming RAM until MacOS
gives me a popup I've never seen before, begging me to start killing processes.
Agent and the networking stuff saw the most significant changes.
Adds a /metrics endpoint in serve mode. Can be disabled via --disable-metrics.
In fetch, dumps the metrics if --metrics is specified (defaults to false).
None of the metrics being collected are on a hot path, so they're just always
collected using atomic operations (i.e. no `if (!enabled) return;`). Because
the operations are cheap and infrequent enough not to matter.
The one place I want to add more metrics is in HttpClient (bytes, status,
counts, ...) but after the pending HttpClient-related PR is merged.
./lightpanda fetch --log-level fatal --metrics "https://lightpanda.io/"
```
build_info{version="1.0.0-dev.7837+70493ce35"} 1
cdp_connections_total 0
cdp_connection_limit_total 0
cdp_active_connections 0
cdp_commands_total 0
cdp_unknown_commands_total 0
js_heap_limits_total 0
script_errors_total 0
arena_hit_total{size="tiny"} 908
arena_hit_total{size="small"} 35
arena_hit_total{size="medium"} 0
arena_hit_total{size="large"} 27
arena_miss_total{size="tiny"} 193
arena_miss_total{size="small"} 31
arena_miss_total{size="medium"} 5
arena_miss_total{size="large"} 23
navigate_total{type="page"} 1
navigate_total{type="iframe"} 0
navigate_total{type="popup"} 0
js_heap_size_bytes_bucket{le="4194304"} 0
js_heap_size_bytes_bucket{le="8388608"} 0
js_heap_size_bytes_bucket{le="16777216"} 1
js_heap_size_bytes_bucket{le="33554432"} 1
js_heap_size_bytes_bucket{le="67108864"} 1
js_heap_size_bytes_bucket{le="134217728"} 1
js_heap_size_bytes_bucket{le="268435456"} 1
js_heap_size_bytes_bucket{le="536870912"} 1
js_heap_size_bytes_bucket{le="+Inf"} 1
js_heap_size_bytes_sum 11223040
js_heap_size_bytes_count 1
```
Arena reuse/retain can hide UAF issues, often resulting in a crash that is more
symptom than cause (far from where the error actually is). Removing this, lets
us better utilize the DebugAllocator's UAF-detection.
Also, when running WPT tests (-Dwpt_extensions) limit console logging to 100
values (a few tests writer millions of values, which is annoying and just
destroys the terminal).
@import("lightpanda") where needed.
Would also like to do this for String, Page, Session and js which all stand out
as types that are use across the codebase.
I know that a few devs are doing this in new work and I haven't heard anyone
voice an objection.
ArenaPool previously maintained up to 512 16KB buckets. The 16KB retention is
small for things like XHR and scripts, but increasing it to something more
reasonably, like 128KB, would use up to 8x more memory.
This commit adds 4 buckets: 1KB, 4KB, 16KB and 128KB. Callers can request a
tiny, small, medium or large bucket. We end up using less memory peak memory
and less allocations.
Furthermore, callers can request a specific size. This is particularly useful
for WebSocket or Blob where the size could vary greatly (so we'd likely default
to a large bucket), but that could needlessly use up a large arena.
The bucket sizes were derived from analyzing allocations. A significant number
of allocations were very small. Things like ScheduleCallback and
FinalizerCallback are always less than 1K and can be generated in the thousands.
The 16KB retention was wasteful in these cases...better to have a large number
of 1K pools, so that we can have a handful of very large buffers.
The Context's call_arena should be based on the source, e.g. the IsolateWorld
or the Page, not always the page. There's no rule that says all Contexts have
to be a subset of the Page, and thus some might live longer and by doing so
outlive the page_arena.
Also, on context cleanup, isolate worlds now cleanup their identity.
History: We started with 1 context and thus only had 1 identity map. Frames
were added, and we tried to stick with 1 identity map per context. That didn't
work - it breaks cross-frame scripting. We introduced "Origin" so that all
frames on the same origin share the same objects. That almost worked, by
the v8::Inspector isn't bound by a Context's SecurityToken. So we tried 1 global
identity map. But that doesn't work. CDP IsolateWorlds do, in fact, need some
isolation. They need new v8::Objects created in their context, even if the
object already exists in the main context.
In the end, you end up with something like this: A page (and all its frames)
needs 1 view of the data. And each IsolateWorld needs it own view. This commit
introduces a js.Identity which is referenced by the context. The Session has a
js.Identity (used by all pages), and each IsolateWorld has its own js.Identity.
As a bonus, the arena pool memory-leak detection has been moved out of the
session and into the ArenaPool. This means _all_ arena pool access is audited
(in debug mode). This seems superfluous, but it's actually necessary since
IsolateWorlds (which now own their own identity) can outlive the Page so there's
no clear place to "check" for leaks - except on ArenaPool deinit.
This removes the browser-specific arenas (session, transfer, page, call) in
favor of the arena pool.
This is a bit of a win-lose commit. It exists as (the last?) step before I can
really start working on frames. Frames will require their own "page" and "call"
arenas, so there isn't just 1 per browser now, but rather N, where N is the
number of frames + 1 page. This change was already done for Contexts when
ExecutionWorld was removed, and the idea is the same: making these units more
self contained so to support cases where we break out of the "1" model we
currently have (1 browser, 1 session, 1 page, 1 context, ...).
But it's a bit of a step backwards because the ArenaPool is dumb and just resets
everything to a single hard-coded (for now) value: 16KB. But in my mind, an
arena that's used for 1 thing (e.g. the page or call arenas) is more likely to
be well-sized for that specific role in the future, even on a different
page/navigate.
I think ultimately, we'll move to an ArenaPool that has different levels, e.g.
acquire() and acquireLarge() which can reset to different sizes, so that a page
arena can use acquireLarge() and retain a larger amount of memory between use.
Any object we return from Zig to V8 becomes a v8::Global that we track in our
`ctx.identity_map`. V8 will not free such objects. On the flip side, on its own,
our Zig code never knows if the underlying v8::Object of a global can still be
used from JS. Imagine an XHR request where we fire the last readyStateChange
event..we might think we no longer need that XHR instance, but nothing stops
the JavaScript code from holding a reference to it and calling a property on it,
e.g. `xhr.status`.
What we can do is tell v8 that we're done with the global and register a callback.
We make our reference to the global weak. When v8 determines that this object
cannot be reached from JavaScript, it _may_ call our registered callback. We can
then clean things up on our side and free the global (we actually _have_ to
free the global).
v8 makes no guarantee that our callback will ever be called, so we need to track
these finalizable objects and free them ourselves on context shutdown. Furthermore
there appears to be some possible timing issues, especially during context shutdown,
so we need to be defensive and make sure we don't double-free (we can use the
existing identity_map for this).
An type like XMLHttpRequest can be re-used. After a request succeeds or fails,
it can be re-opened and a new request sent. So we also need a way to revert a
"weak" reference back into a "strong" reference. These are simple v8 calls on
the v8::Global, but it highlights how sensitive all this is. We need to mark
it as weak when we're 100% sure we're done with it, and we need to switch it to
strong under any circumstances where we might need it again on our side.
Finally, none of this makes sense if there isn't something to free. Of course,
the finalizer lets us release the v8::Global, and we can free the memory for the
object itself (i.e. the `*XMLHttpRequest`). This PR also adds an ArenaPool. This
allows the XMLHTTPRequest to be self-contained and not need the `page.arena`.
On init, the `XMLHTTPRequest` acquires an arena from the pool. On finalization
it releases it back to the pool. So we now have:
- page.call_arena: short, guaranteed for 1 v8 -> zig -> v8 flow
- page.arena long: lives for the duration of the entire page
- page.arena_pool: ideally lives for as long as needed by its instance (but no
guarantees from v8 about this, or the script might leak a lot of global, so worst
case, same as page.arena)
Karl Seguin