Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Big PR description, but 2 important things to highlight:
1 -
The build process has changed a lot. You'll need to run
zig build get-v8
followed by
zig build build-v8
first. It might be better to test this branchin a separate / clean folder (it'll leave a large v8 folder in the root of the
project).
2 -
If you run this and notice that memory usage is high, try running it with the new
--gc_hints
command line argument. Note that without the--gc_hints
memory usagedoes level off at some point, and it's faster. With
--gc_hints
, the memoryconsumption should be lower than before and it might still be a bit faster.
Overview
This PR aims to replace
zig-js-runtime
with the main goal of:by the http client rewrite).
No memory layout guarantee with zig structs zig-js-runtime#205)
The intention wasn't to improve performance or memory usage, but, at least on
my tests, both have been improved.
The changes can be broken down into two parts, changes to Browser and reworking
of zig-js-runtime.
Browser Changes
Most of the changes to browser were made to accommodate the new js runtime.
However, other changes were also made:
All browser-related code was moved under the browser folder. For example,
src/css/
->src/browser/css/
zig-v8-fork
andtigerbeetle-io
are now Zig packages. Both projects werechanged slightly to be proper Zig libraries. They both have a
jsruntime
branchwhich this branch references.
netsurf.zig
andmimalloc.zig
are no longer separate modules and can be found in:src/browser/netsurf.zig
andsrc/browser/mimalloc.zig
.zig build unittest
has been removed.zig build test
is now more idiomatic:the root source file is src/main.zig and tests are automatically discovered.
zig build test
is, as before, slow and something to be looked into.All existing web API tests are passing, but the test runner has changed a little
so every single line of test has been superficially touched. Also,
getter can't return a Callback zig-js-runtime#200 is now fixed, so
the XHR test no long has commented out tests cases.
The rest of the changes exist specifically to accommodate the new jsruntime,
which will be covered next, but briefly:
Browser
/Session
/Page
, the main integration points with the js runtime,have changed slightly.
UserContext
has been renamed toSessionState
and, while the same in spirit,is also quite different.
Loop
andAllocator
are no longer "internal" or special types. These are nowaccessed through the SessionState.
Variadic
no longer exists, they map directly to plain Zig slicespostAttach
has been replaced withindexed_get
andnamed_get
whichcapture live changes.
mem_guarantied
is no longer needed and has been removed where declared.get-v8
which gets the tools to build v8as well as the v8 source, and
build-v8
which builds v8. All of this is placedin a
v8
folder at the root of the project. Also, on mac, we always build a releasebuild (since the dev build segfaults).
js runtime
The zig-js-runtime project has been replaced with two files
src/runtime/loop.zig
andsrc/runtime/js.zig
.The loop is unchanged except that
cancel
on Mac no longer allocates(and leaks) memory (it's a complete no-op, versus the current partial no-op).
Architecture
The js runtime architecture matches the Browser structure:
Browser -> js.Env -> v8.Isolate
Session -> js.Executor -> v8.Context
Page -> js.Scope -> v8.HandleScope
As you can see, the v8.Isolate has been elevated to the Browser and is re-used
across multiple sessions. To make this possible, most (maybe all??) leaks have
been fixed. However, v8.Context are garbage collected by v8 at its own discretion.
Thus, this new approach, while faster, is likely to use more memory. With the new
--gc_hints
command line argument, we callisolate.lowMemoryNotification()
wheneach executor ends - encouraging v8 to garbage collect the just-released
context. Note that this is still just a hint. I think it's safe to say that this
approach gives more control to the v8 GC, making memory usage less predictable.
Code Generation
The v8 bindings are created in a single-pass, directly from Zig's type system.
There is no longer a
reflect
stage nor an internal type representation. Themain advantage is that you don't need to learn two type systems.
Generics
The global types have been replaced by making most js.* types (i.e. js.Env)
generics. I believe this significantly simplifies our build process, thus the
ability to have the tests' root source file be
src/main.zig
.The only downside that I've seen is that you will get an awful type description
in an error message involving the js.Env(T) or any of its child types.
UserContext / SessionState / State
UserContext has been reworked. From the js runtime's point of view, i.e.
js.Env(T)
, this isState
generic. From the rest of Browser's point of view,it is now called
SessionState
.The
SessionState
belongs to theSession
and is referenced by theExecutor
.Rather than calling
setUserContext
, thesession.state
object ismutated directly.
Most importantly, the js runtime no longer has the concept of special/internal
types. Thus,
*Loop
andAllocator
have been added to the SessionState.For example, the
XMLHttpRequest
constructor was:And is now:
Compatibility with other engines
Without knowing anything about other JS engines, I feel that some of the changes
could make integrating a different JS engine harder and some of the changes
could make it simpler. I think this new approaches exposes a few less types and
presents more of a self-contained, but far from perfect, interface.
Personally, I think it would be great if the abstraction could be built under this layer, so that instead of having a v8.Context, v8.Value and v8.Object, we have a js.Context, js.Value and js.Object, so that binding code can be re-used, but that might be a stretch.