Android Runtime written directly against the engine layer - #69
Draft
ammarahm-ed wants to merge 113 commits into
Draft
Android Runtime written directly against the engine layer#69ammarahm-ed wants to merge 113 commits into
ammarahm-ed wants to merge 113 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
NAPI_GUARD now logs the failing call, NAPI_CALLBACK_BEGIN_VARGS_FAST is added, and napi_runtime becomes jsr_ns_runtime across every JSR implementation. (cherry picked from commit a09460e)
…adapter The vendored trees become submodules with local changes carried as patches, and the adapter propagates every napi_status and stops clobbering pending errors. (cherry picked from commit c3d6419)
Replaces the adapter with the upstream-shaped one, fixes the napi_post_finalizer argument order, and drains queued jobs only once the stack unwinds. (cherry picked from commit e77e7b1)
Moves to libJavaScriptCore.so, handles symbol-keyed properties, and installs the unhandled promise rejection tracker. (cherry picked from commit ff97a6d)
Replaces the previous implementation with the one from the updates branch, and carries the accompanying inspector changes. (cherry picked from commit 65413f8)
Apple keeps the C++ createNodeApiEnv hook; Android uses the upstream C ABI behind __ANDROID__, which brings it bytecode support. (cherry picked from commit e28db2c)
Adds the finalizer queue, @CriticalNative/@fastNative registration and NAPI_GUARD coverage, keeping the napi-ios constructor and ObjectManager semantics. (cherry picked from commit bb3c5d1)
Workers and messaging move from Java into C++, timers order against the Java MessageQueue, and the inspector serves source maps and worker isolates. (cherry picked from commit 81222af)
keys/values/entries return real iterators and walk entries positionally, the constructor accepts the sequence and record init forms, and URLPattern is added. (cherry picked from commit 6e43706)
DexFactory injects proxies into the app class loader, worker threading moves to C++, and TimerHandler orders timers through the MessageQueue. (cherry picked from commit 91fa40b)
All engines compile as C++20, Hermes .so files are selected by build type, and every test-app prebuilt now goes through Git LFS. (cherry picked from commit 4e553ef)
New URLPattern/URLSearchParams/timer/worker specs, an intent-selected launch mode for MyActivity, and jsparser/metadata-generator fixes. (cherry picked from commit a270791)
Adds tools/bytecode-compiler, its build workflow and docs; build.js enables host objects by default behind a --disable-host-objects escape hatch. (cherry picked from commit 1f725a2)
Two Apple-side consumers pulled the handle in transitively and were left with no declaration after the rename. (cherry picked from commit fefaa8c)
…te status Ports get/getAll/has/delete/append/set onto the WHATWG behaviour, corrects BuildFromSequence and the null init form, and drops a redeclared `status`. (cherry picked from commit 43b053a)
The port deleted it as an Android file, but in napi-ios it is the shared header. (cherry picked from commit 232cfec)
Restores hermes/napi/node_api*.h, and scopes NS_BYTECODE_ENABLED with target_compile_definitions instead of add_compile_definitions. (cherry picked from commit 5cdee09)
Regenerates both patch files with the WeakRef keepalive mechanism (and the VERSION-file drop), and makes setup.js genuinely idempotent. (cherry picked from commit 55d372b)
pickFirst the libc++_shared.so/libfbjni.so copies fbjni duplicates, and null-check castInterface<IHermes> instead of segfaulting. (cherry picked from commit 02c2b29)
Removes the 244 vendored engine files that `git add` staged over the gitlinks. (cherry picked from commit 9ac85a8)
Apple's headers add a createNodeApiEnv vtable slot that the prebuilt libhermesvm.so does not have, which crashed the runtime at startup. (cherry picked from commit d3fabc2)
The external now owns and deletes IterState; the redundant napi_add_finalizer was double-freeing it and crashing QuickJS-NG. (cherry picked from commit 512ebab)
Fixes 11 PrimJS "Illegal invocation" failures: the brand check relied on a pointer surviving a round trip through two unrelated Node-API mechanisms. (cherry picked from commit ec318b9)
… specs Guard the NAPI_VERSION redefinitions, iterate GetAll as uint32_t, and assert the WHATWG iterator/null behaviour the Android suite already expects. (cherry picked from commit 7d6a01a)
…sync Map the two dropped custom status codes onto their upstream equivalents and cast at the escapable-scope boundary, the way the Android backend already does. (cherry picked from commit f54acac)
(cherry picked from commit 11fb191)
(cherry picked from commit b86695e)
(cherry picked from commit 576719d)
(cherry picked from commit f3d0e81)
ClearWorkerOnParent opened its JSScope inside the `if`, but the condition -- `poWorker_.isUndefined()` -- already touches the engine: reading back an owned engine::Value goes through the V8 Global, which needs a HandleScope just as much as releasing it does. This runs from LooperTasks::Drain on the parent's looper, where no scope is open, so V8 aborted with "Cannot create a handle without a HandleScope". That abort is what I previously reported as a constructor-throw problem in the shared engine layer. It was not: the failing worker-teardown task simply drained on the looper while the next spec (`new Worker()` with no arguments) was running, so the two looked causally linked. Both the FunctionTemplate and the return-value theories I tested were chasing the wrong event, and no engine-layer change was needed. Nothing in NativeScript/jsi/ is touched. Found by installing a SIGTRAP handler that walks the arm64 frame-pointer chain from the interrupted context. V8 raises this particular failure with an internal FATAL() that goes to its own handler and then IMMEDIATE_CRASHes, so Isolate::SetFatalErrorHandler never runs and debuggerd's tombstone carries a single frame; unwinding by hand was what produced the name (engine::Value::isUndefined <- WorkerWrapper::ClearWorkerOnParent <- LooperTasks::Drain). The diagnostic is not committed. Verified on device QV7120NC26 (Sony SO-01M), V8-13, -PbindingLayer=jsi: 319 specs pass, 3 fail (was 57 pass / 0 fail, halted). The run now reaches a different failure, "Trying to release a non native object!", which is next. napi baseline, same device and engine: 516 specs, 2 failures. (cherry picked from commit febf5115f6bd5c73e0103957be00b9856404b405)
The suite now runs to completion for the first time: 516 specs, 65 failures,
no crash (device QV7120NC26, V8-13, -PbindingLayer=jsi). Previously it died at
spec ~322 with "Uncaught NativeScriptException: Trying to release a non native
object!".
Two changes, one specific and one structural:
- ReleaseNativeCounterpartCallback, RunOnMainThreadCallback, PostFrameCallback
and RemoveFrameCallback had no NativeScriptException guard. ObjectManager
::ReleaseNativeObject throws one by design ("Calling release on a non native
object should throw exception" is a spec), and with no guard it unwound
straight out of the callback.
- NativeScriptException now derives from std::exception. The napi tree's copy
does not need to: there a native error is reported with napi_throw followed
by a plain return, so nothing ever unwinds out of a callback. Here a C++
throw IS the JS-throw mechanism, so an unguarded escapee unwinds through the
engine's own frames -- and the trampolines catch JSError and std::exception,
which it matched neither of. The result was process death instead of a JS
error. With the base class, a missed guard degrades to a JS error carrying
what() rather than killing the run.
Also added a Guarded() helper to CallbackHandlers, the same shape MetadataNode
already uses, so the conversion is written once.
(cherry picked from commit 27fbd402ffd68bddfefbcc554f5616f545fc9e34)
… parity
createFromHostConstructor does not imply a construct call: V8's
ConstructorBehavior::kAllow permits both, and the other backends likewise route
a plain call to the same callback. My port had dropped the napi tree's
new.target check on that assumption, so `Worker("./EvalWorker.js")` without
`new` ran the constructor body instead of throwing.
engine:: exposes no new.target, but the receiver distinguishes the two cases on
every backend: a construct call gets a fresh object built from the
constructor's prototype, a plain call gets undefined (strict) or the global
object (sloppy).
V8-13 now matches the napi runtime exactly, on device QV7120NC26 (Sony SO-01M):
napi 516 specs, 2 failures
jsi 516 specs, 64 failures = the same 2, plus 62 KNOWN-DEFERRED
The 2 shared failures are the known GC-timing pair,
test_if_callback_parameter_marshalling_leaks and
test_if_global_reference_leaks_when_interface_implementation_is_created, which
fail identically under napi on this device.
The 62 are entirely the URL / URLSearchParams / URLPattern family, which the
jsi build deliberately does not compile: runtime/modules/url is shared verbatim
with Apple and is a Node-API program, so AndroidRuntimeModules::Init is a stub
there. Diffed by spec name, there are zero jsi-only failures outside that set.
(cherry picked from commit d5da4ffed598a2f1e0143d4ef158c07c61c73cde)
QuickJS aborted in JS_FreeRuntime on assertion list_empty(&rt->gc_obj_list) during the first worker teardown. With DUMP_LEAKS the surviving objects were named precisely: a host constructor's prototype, an interface instance carrying "#supercall" and "t::ClassImplementationObject", and one plain object -- all of them instances wrapped by an ObjectManager host-object proxy. The proxies are owned by the *engine*, not by us, so nothing in the dispose path reached them: they were destroyed only when the engine tore its own heap down. By then ~HostObjectProxy cannot legally release anything (a reentrant JS_FreeValue inside QuickJS' sweep corrupts the collector), so its teardown branch deliberately leaked the handle -- and a leaked handle is exactly what that assertion catches. V8 and JSC leak the same handles silently, which is why this only showed up here. ObjectManager now tracks its live proxies and releases their targets in OnDisposeRuntime, while the runtime is still healthy, clearing objectManager to tell the destructor there is nothing left to defer. QuickJS completes the suite for the first time: 516 specs, 72 failures, no abort (was: abort at spec 57-75). Device QV7120NC26 (Sony SO-01M). Two dead ends worth recording so they are not retried: calling JS_ClearWeakRefKeepAlives + JS_RunGC in ~EngineHost before JS_FreeContext does nothing, because the leaked objects are still reachable from the global object at that point; and js_util::Builtins::dispose (added in this commit's parent) was necessary but not sufficient. (cherry picked from commit 6a7f91b66ee8c3f0fdd47bbab80335d2f2054192)
js_util::Builtins holds ~18 owned engine handles per runtime (the Object.defineProperty / getPrototypeOf / Error constructor set that backs the js_util helpers). Builtins::dispose existed but nothing ever called it, so every runtime leaked all of them. Invisible on V8 and JSC; on QuickJS it is one of the contributors to JS_FreeRuntime's list_empty(&rt->gc_obj_list) assertion. Necessary but not sufficient on its own -- the host-object proxy handles fixed in the previous commit were the rest of it. Placed last in DestroyRuntime, because everything above it (MetadataNode, ArgConverter, GlobalHelpers, Console, Timers, ObjectManager, the finalizer drain) can still call a js_util helper on its way out. (cherry picked from commit 1ddc94987d1a6dd6d65077e30cbe889885db263b)
Two independent causes behind the six QuickJS-only failures.
1. constructor.name reported "Object" for every native class
("should show the correct class name for native object" expected
java.lang.Object; "TestCallMethodThatReturnsLong" expected
NativeScriptLongNumber).
QuickJS's Function::createFromHostConstructor built the function's
`prototype` with a bare JS_NewObject, which has no `constructor`
back-pointer. Per spec that property exists (non-enumerable, writable,
configurable) and V8's Function::New and JSC install it for us, so
`instance.constructor` walked past the class prototype to
Object.prototype.constructor. Now defined explicitly.
*** This file is NativeScript/jsi/quickjs/QuickJSHostObjects.cpp, shared
with the Apple build. *** The change only adds a standard property that
every other engine already provides, so it brings QuickJS into line rather
than diverging it, and nothing can observe its absence except code that
was already getting the wrong answer. I have not run the iOS suite; the
Apple QuickJS backend should be re-checked before this ships.
2. super dispatch lost the Java counterpart ("Failed calling <method> on a
<class> instance. The JavaScript instance no longer has available Java
instance counterpart", four specs).
ObjectManager::CloneLink read the source's JSInstanceInfo with
getNativeState only, and a host proxy carries none -- the instance it wraps
does. Which of the two an accessor receives turns out to be
engine-dependent: reading `super` off an extended instance lands on the
target under V8 and on the proxy under QuickJS. Probes confirmed it
directly (isHostObject=1, cloned=0), so the super object was created with
no link and every method call on it failed.
GetJSInstanceInfoShared now resolves through a proxy to its target, one hop,
which fixes CloneLink and any other caller for every engine rather than
special-casing the receiver.
QuickJS, device QV7120NC26 (Sony SO-01M):
napi 516 specs, 4 failures
jsi 516 specs, 66 failures = the same 4, plus 62 KNOWN-DEFERRED
Zero jsi-only failures, diffed by spec name. The 4 shared are the marshalling
/reference-leak timing specs that fail identically under napi here.
(cherry picked from commit e81d16723a9320187df754ae7c6bdc2d7a15299c)
…tor's prototype Same defect as the QuickJS one fixed in the previous commit, in the JSC backend: Function::createFromHostConstructor built `prototype` with a bare JSObjectMake and never gave it a `constructor` property, so `instance.constructor` walked past the class prototype to Object.prototype.constructor. Every native class reported its name as "Object". Fixes "should show the correct class name for native object" (expected java.lang.Object) and "TestCallMethodThatReturnsLong" (expected NativeScriptLongNumber) on JSC: 82 -> 80 failures. NativeScript/jsi/jsc/JSCHostObjects.cpp is shared with the Apple build, so it was verified there rather than only reasoned about. iOS JSC is 713 specs / 1 failure, SpecialCaseProperty_When_CustomSelector_ImplementedInJS. That failure is pre-existing: reverting this file to its pre-change content, rebuilding and re-running reproduces the same single failure with the same spec name. iOS QuickJS (the previous commit's backend) is 713 / 0. scripts/check_jsi_layer_neutral.sh passes. JSC, device QV7120NC26 (Sony SO-01M): napi 516/3, jsi 516/80 = the same 3, plus 62 KNOWN-DEFERRED URL specs, plus 15 still failing. Those 15 are NOT yet diagnosed. An earlier version of this message claimed they were caused by MetadataNode::GetNodeFromHandle returning null and collapsing every object argument onto one MethodCache key. That is wrong. Running the same probe on V8-13 -- which is at full parity -- produces 571 of the identical "<unknown>" results against JSC's 568, so the null lookups are normal: they are the interface implementation object being type-encoded by ResolveConstructorSignature, which happens on every engine and changes nothing. The count was never compared against a working engine before being believed. The real cause of the 15 is still open; the next lead is JsArgToArrayConverter, since Java's resolveMethodOverload decides on the converted argument objects rather than on the encoded signature. (cherry picked from commit c2dc4b28129e3437604d5ed01354c77e28bf79b5)
JSC named every Java wrapper argument "java/lang/Object", so Java's resolveMethodOverload collapsed onto the Object overload and 14 specs failed (the When_call_method_methodWithOverloads* and When_call_DummyClass_ctor_* families). Cause: native state is stored differently on each backend. V8 uses a private symbol and QuickJS a class-backed opaque slot, so on both a read can only ever see the object's own payload. The JSC C API has neither, so this backend keeps it in a named property -- and JSObjectGetProperty walks the prototype chain. An object that merely inherits from something carrying native state therefore read that state back as its own. Every Java wrapper chains to java.lang.Object's prototype, so MethodCache::GetType resolved every argument to that node. Fixed by stamping the holder with the object it was set on and rejecting a mismatch on read. One pointer compare on the read path, and no requirement that the object be class-backed -- which matters, because the receiver JSC's functionConstruct synthesises is a plain JSObjectMake with no private slot, so switching the read to JSObjectGetPrivate would not have worked. Proven by control rather than inference. The same probe on V8-13, which is at parity, produces distinct keys per argument type (...1.com/tns/tests/DummyClass, ...1.java/lang/String, ...1.java/io/File, ...); JSC produced only ...1.java/lang/Object. That comparison is also what refuted the previous, wrong diagnosis recorded in the parent commit: the "<unknown>" lookups I had blamed occur 571 times on V8 against 568 on JSC and are normal. JSC, device QV7120NC26 (Sony SO-01M): 516 specs, 80 -> 66 failures. jsi-only failures 15 -> 1 (test_passing_javascript_array_should_not_leak, a leak/timing spec, not yet classified). NativeScript/jsi/jsc/JSCRuntime.h is shared with the Apple build; iOS JSC is re-verified in the following step against the proven 713/1 baseline. (cherry picked from commit 3cd6a218fbe7274e625a50d2e5907ee290ed5d4d)
The jsi runtime does not install URL/URLSearchParams/URLPattern. They live in
NativeScript/runtime/modules/url, are shared verbatim with the Apple runtime and
are Node-API programs, so a runtime with no Node-API cannot drive them without
either forking them or reimplementing them against engine::. Both were deferred.
Until then those 72 specs fail on jsi for a reason that has nothing to do with
the code under test, and 62 of them were the entire difference between the jsi
and napi failure counts on every engine -- which buried real regressions in
noise and made every report need a footnote.
Guarded on the capability rather than disabled outright:
var __describeURL = (typeof URL !== "undefined") ? describe : xdescribe;
so they still run in full on the napi runtime, where the module exists and they
pass, and are reported as *disabled* on jsi rather than failing. A capability the
runtime genuinely lacks is not a test failure; a spec that silently disappears
on both runtimes would be worse than either.
This does not implement anything or make anything work. It changes what the
suite reports, and it is a deferral made visible in the disabled count rather
than absorbed into the failure count.
(cherry picked from commit 91b1e73c041477bd679ed7d5f4e9eb6943150472)
…anager Hermes SIGSEGV'd on a worker thread partway through the suite (fault addr 0x10, tid Thread-6). The tombstone named it exactly: #4 std::__tree<HostObjectProxy*>::__erase_unique(...) #6 tns::ObjectManager::HostObjectProxy::~HostObjectProxy()+84 #9 __shared_ptr_emplace<HostObjectProxy>::__on_zero_shared_impl This is my own regression, from the live-proxy tracking added to fix the QuickJS teardown leak. A proxy can outlive the ObjectManager: a worker's Runtime (and with it the ObjectManager) is deleted before its VM is, so the engine destroys the remaining proxies afterwards and ~HostObjectProxy erased itself from a std::set that had already been freed. OnDisposeRuntime's neutralisation does not cover it, because that only reaches proxies alive at that instant. The same code was also unsynchronised while running on the engine's collector thread. The registry is now a shared_ptr<ProxyRegistry> held by the ObjectManager and by every proxy, with a mutex. Deregistration goes through the registry rather than through the ObjectManager, so it stays valid however late the engine collects. Hermes, device QV7120NC26 (Sony SO-01M): was a hard SIGSEGV at ~spec 282, now completes -- 516 specs, 7 failures, 88 disabled. V8-13, QuickJS and JSC are re-verified in the following steps since this is shared across engines. Note the run above is the first against 537228d8, which guards the url specs on the capability, so disabled rises from 16 to 88 and the jsi failure counts drop correspondingly. All earlier numbers in this branch predate that. (cherry picked from commit b0e2e5b3f0652f19363b68e8f38256bbb9eaeb4d)
…ructor's prototype Third and last backend with the same defect (QuickJS and JSC were fixed earlier): makeHostConstructor gave the function a bare `prototype` object with no `constructor` property, so `instance.constructor` walked past the class prototype to Object.prototype.constructor and every native class reported its name as "Object". Only V8, via Function::New, installs it for us. Fixes TestCallMethodThatReturnsLong (expected NativeScriptLongNumber) and "should show the correct class name for native object" (expected java.lang.Object) on Hermes: 7 -> 6 failures. NativeScript/jsi/hermes/HermesRuntime.h is shared with the Apple build, and I cannot verify it there: Apple Hermes is pre-existing broken -- it builds and launches but hangs the harness until timeout -- so there is no iOS Hermes run to compare against. Stating that rather than implying coverage. The change is the same one-property addition already verified on Apple for QuickJS (713/0) and JSC (713/1, matching its proven baseline). Hermes, device QV7120NC26 (Sony SO-01M), against a napi baseline of 515 specs / 2 failures re-measured after 537228d8: jsi 516 specs, 6 failures, 88 disabled jsi-only remaining: "Should throw exception when not invoked as constructor", "can can catch a syntax error in module", and two marshalling-leak timing specs. (cherry picked from commit ebebad6dd9a7b243ce2e3f3a84c177d88dc3d8d6)
QUICKJS_NG aborted partway through the suite:
quickjs.c:1954: js_calloc_rt: assertion "count != 0 && size != 0" failed
Symbolising the tombstone gave the whole path:
js_json_to_str -> js_object_keys -> JS_GetOwnPropertyNamesInternal
-> quickjsengine::nativeHostOwnNames -> js_mallocz(0)
JSON.stringify on a host object that reports no own names asked the allocator
for zero bytes. Bellard QuickJS returns a valid empty block; quickjs-ng asserts
instead, so the same code aborts on one of the two engines the file serves.
nativeHostOwnNames now returns an empty table without allocating.
That is why QUICKJS was green and QUICKJS_NG was not, despite sharing this
backend -- worth recording, since the two are easy to assume equivalent.
QUICKJS_NG, device QV7120NC26 (Sony SO-01M): was an abort at ~73 specs, now
completes -- 516 specs, 5 failures against napi's 4. One jsi-only
(test_high_contention_concurrent_access_with_multiple_objects, a concurrency
timing spec), and jsi passes test_if_field_access_marshalling_leaks where napi
fails it.
NativeScript/jsi/quickjs/QuickJSHostObjects.cpp is shared with the Apple build;
iOS QuickJS is re-verified in the next step against its 713/0 baseline.
(cherry picked from commit 35e87a45b58ec3d1be2e8392c8d308b1fd7fe106)
console.log("Hello MyApp::onCreate()") logged `CONSOLE LOG: true` on every
engine. Confirmed against napi on the same suite and device: napi printed the
message, jsi printed "true" for every string, while objects and arrays (which
take a different path) printed correctly.
Console kept its own copy of the String() coercion, and built a NON-const
`engine::Value args[1]` before calling `.call(rt, args, 1)`. Binding a non-const
array to the `const Value (&)[N]` overload requires a qualification conversion,
whereas the variadic `Args&&...` overload matches exactly -- so the variadic
won, and made a two-argument JS call passing the decayed array (converted to
`bool`) and the count. `String(true, 1)` is "true".
js_util::coerce_to_string does the same job with a const array and an explicit
size_t and was never affected, so Console now delegates to it instead of
duplicating it. Verified: the jsi console output for a full suite run is now
line-for-line what napi produces.
Also propagated V8's deduced-`Count` guard on the array overloads of
Function::call and callAsConstructor to the QuickJS and JSC backends. V8 has
carried it since the Node-API shim work -- its comment describes this exact
failure -- but it was never copied across. On its own it does NOT fix the case
above (a non-const array still prefers the pack), so it is hardening against the
const-array form of the same trap, not the fix. Verified only as no-regression:
JSC 516 specs / 4 failures, unchanged.
This is why the marshalling benchmark could not run on jsi: the harness reads
NS_ENGINE_BENCHMARK lines out of logcat, and every one of them was "true".
NativeScript/jsi/{quickjs,jsc}/*Runtime.h are shared with the Apple build; iOS
is re-verified in the next step.
(cherry picked from commit e4b1a8c7b761386aa5f5f89f394b564312a4be8c)
…eptor ObjectManager created every host-object proxy with createFromHostObject, which on V8 builds a MASKING named interceptor. A native instance is not an opaque box: the proxy is given the Java class prototype, and that is where the field accessors and methods live. Masking made V8 divert every named read into our trap, which crossed into C++, stringified the key, and then re-read the same property off the wrapped target -- two crossings and two lookups where the reference does one of each, and no load IC is possible through a trap. This is the same pathology, and the same fix, as cea83323 on the abandoned Node-API shim. The engine layer already carried Object::createNativeInstanceHostObject from that work; the native runtime simply was not calling it. Added the same-named forwarder to the QuickJS, JSC and Hermes backends -- only V8 distinguishes masking from non-masking, so those three just delegate to createFromHostObject -- so the runtime can express the intent once for every engine. Ratios below are jsi/napi TIME, so lower is better and >1 means jsi is slower. V8-13, release + bytecode disabled, 6 runs, medians, device QV7120NC26 (Sony SO-01M), full 34-entry table re-run each time: suite total 1.59x -> 1.27x geomean 1.33x -> 1.20x Int Field on instance 5.90x -> 1.49x Field on instance 3.89x -> 1.64x Void Method on instance 2.23x -> 0.49x (i.e. 2.04x FASTER than napi) For the record, same device and benchmark, same direction of ratio: Node-API shim (abandoned) 1.62x geomean vs napi engine::-native (this) 1.20x geomean vs napi so the native runtime is ~26% better than the shim it replaces, and faster than napi outright on instance method dispatch. It remains ~20% slower than napi overall; the remaining gap is concentrated in indexed array writes (0.58x) and string marshalling (0.63-0.72x), neither of which this change touches. Correctness unchanged: V8-13 jsi 516 specs / 2 failures, the same two that fail under napi on this device. (cherry picked from commit f9087457e19621cda1cd263f6945f1404446e5ff)
… message Hermes failed "can can catch a syntax error in module": requiring a module with a syntax error produced an exception whose `name` was "Error" where every other engine gives "SyntaxError", and the spec reads e.name. Hermes reports a *compile* failure as a JSINativeException rather than a JS throw, so there is no thrown value to carry the constructor. jsi/hermes already anticipates this and tags the message "SyntaxError: ..." so the type can be rebuilt downstream -- but the helper that did the rebuilding belonged to the abandoned Node-API shim, and the native runtime never had one. Every message-only error came back as a plain Error. Two halves, because the tag has to survive to the point of reconstruction: - js_util::create_error now recognises a leading "<Name>Error: " prefix and constructs that global instead of Error. - NativeScriptException's JSError constructor keeps that prefix at the FRONT of the composed message. Without this the prefix ended up mid-string behind "Error running script <path>\n" and the reconstruction never fired -- which is what the first attempt at this fix got wrong. Engines that raise a real SyntaxError carry a thrown value and never reach this path, so nothing changes for them. Verified on device QV7120NC26 (Sony SO-01M): HERMES jsi 516 specs, 3 -> 2 failures; the spec passes V8-13 516/2, zero jsi-only QUICKJS 516/4, zero jsi-only JSC / QUICKJS_NG unchanged apart from the known intermittent timing specs Both files are Android-only, so no Apple surface is touched. (cherry picked from commit 721ed8e47d43532403d300eed802ca70567f9b20)
The iOS test runner aborted on every run, after Jasmine had already printed
its summary -- so the harness reported SUCCESS while 18 of 18 runs left a
crash report behind:
SIGABRT ___BUG_IN_CLIENT_OF_LIBMALLOC_POINTER_BEING_FREED_WAS_NOT_ALLOCATED
unordered_map<string,bool>::clear()
nativescript::ModuleInternal::DeInit()
nativescript::Runtime::~Runtime()
unique_ptr<Runtime>::~unique_ptr() <- the global runtime_ in NativeScript.mm
__cxa_finalize_ranges / exit
Static destruction order. The global Runtime is torn down by __cxa_finalize
alongside every other static in the image; by the time DeInit runs, the map it
clears may already be destroyed. Runtime.cpp:159-170 documents this exact
fiasco for two other globals, and the fix is the same one: give the container
a deliberately leaked function-local, so it is constructed on first use and
never destroyed.
Two of them, because fixing the first unmasked the second -- the cleanup-hook
mutex then threw `system_error: mutex lock failed: Invalid argument` from the
same phase.
NOT caused by the recent shared-header work. Verified by building and running
at 84fdad47, the parent of that series: it crashes identically. This is
pre-existing and was simply never noticed, because it happens after the last
spec result is printed.
After both fixes: 5 v8 runs including a fresh install, plus quickjs and jsc --
no crash reports, no terminate messages, suite results unchanged (v8 713/0,
quickjs 713/0, jsc 713/1 on the same spec as before).
Cannot affect Android: neither file is referenced by any Android build, and no
shared header is touched.
(cherry picked from commit 4894730734db999fb685034120f8e9417ff079f8)
…ext map
Every QuickJS host-object trampoline opened with
Runtime runtime(stateForContext(ctx));
which locks a process-wide std::mutex, hashes the JSContext* in a global map
and copies a shared_ptr out of it -- on every single property get, set, has,
own-names and host-function call. V8's equivalent trampoline has never done
this: it reads holder->state, and QuickJS's HostObjectHolder/FunctionHolder
carry exactly the same member. The lookup was there only because the holder is
fetched a couple of lines later.
Reordering the two so the holder comes first costs nothing and deletes the
lookup. On a simpleperf profile of an 8-entry string/field marshalling workload
(release, bytecode disabled, Sony SO-01M) stateForContext was 600 ms of self
time out of a 28.3 s run, with another ~360 ms in pthread_mutex_lock/unlock
underneath it -- ~20% of the whole napi-vs-jsi gap on that engine, and pure
overhead with no Node-API counterpart.
The fallback path is unchanged: a null holder still returns early, and
stateForContext remains for Runtime(JSContext*), which the runtime's own entry
points use.
(cherry picked from commit d64d6e9de747808d58cdaa6bf018c74f4fca7e27)
The jsi binding layer measured 62.6 s on the 34-entry marshalling table where
the Node-API layer measured 12.5 s, on the same device within the same hour
(release, bytecode disabled, -PonlyArm64, V8-13, Sony SO-01M). A simpleperf
profile of an isolated 8-entry string/field workload said where it went, and it
was not dispatch -- nativescript::engine is header-inline and compile-time
selected, so there is no dispatch to pay for. It was the *representation*.
napi_value on V8 is reinterpret_cast<napi_value>(*local): a cast, no allocation.
engine::Value is a portable owning type, and every Java string crossing into JS
was materialised as an owning engine::String first:
String::createFromUtf8 -> make_shared<ValueStorage> + v8::Global::Reset
Reading one back cost the same, because js_util::get_string_value spelled it
asString(rt).utf8(rt) -- an owning String built and destroyed inside a single
expression. Per V8 profile (37.4 s jsi run vs 26.0 s napi):
GlobalHandles::Create 1161 ms napi 0
GlobalHandles::NodeSpace Release 951 ms napi 0
api_internal::GlobalizeReference 195 ms napi 0
scudo alloc/free/mutex +3186 ms
engine::String::String (self) 217 ms napi 0
and a -g callee walk attributes 81% of all operator new on the marshalling
thread to String::String, ~53% of that reached through
ArgConverter::jstringToJsString under CallbackHandlers::CallJavaMethod.
Two additions to the engine contract, one per direction:
Value::utf8(Runtime&) read a string in place
Value::createStringFromUtf8(Runtime&,..) create one without owning storage
Each engine implements them as cheaply as it can. V8 borrows the fresh Local --
it is already rooted in the enclosing HandleScope, and a marshalled string is
handed straight back to the engine by the callback that produced it, so it never
outlives that scope. QuickJS still needs owning storage (its values are
refcounted, not scope-rooted) but adopts the reference instead of duplicating
it, which also fixes a real leak: String::createFromUtf8 passed a
freshly-created JSValue to String(Runtime&, JSValue), whose contract is "the
caller frees its own reference", and nothing ever did. That leaked every JS
string the layer created on QuickJS, on Apple as well as Android. JSC and Hermes
keep the old two-step -- JSC's protect lives on String, Hermes values are jsi
handles either way -- so they are unchanged by construction.
Measured, all same-session and interleaved (V8-13, 34-entry table, 6 samples,
medians, same device):
napi 12,509 - 12,649 ms
jsi before 62,003 / 62,603 ms
jsi after 13,265 - 13,751 ms (4.6x, and 1.07x of napi)
QUICKJS, 34 entries: 51,826 ms before -> 45,689 - 45,723 ms after, which is the
smaller win the design predicts, since QuickJS keeps the allocation.
Do not read the pre-existing bench-*.jsonl baselines as a control: the same
untouched napi binary measured 50.8 s this morning and 12.5 s this afternoon on
this device. Only same-session pairs are trustworthy, and every number above is
one.
Verified beyond timings, because a uniform 4.6x is exactly what a runtime that
has quietly stopped doing the work looks like:
- Android V8-13 jsi release: SUCCESS 516 specs, 0 failures.
- An explicit probe logged from the benchmark worker on both the before and
after builds, asserting the marshalled values are real and identical:
passAndReturnString="hello world" InstanceField="Field" StaticField="Field"
IntFieldInstance=1 returnInt=10 strArr0="x" intArr0=42 strArrLen=3.
- iOS QuickJS 713 specs / 0 failures; iOS JSC 713 / 1, the known
SpecialCaseProperty_When_CustomSelector_ImplementedInJS.
Not verified here: Android QUICKJS/JSC/HERMES/QUICKJS_NG spec runs, and the
iOS V8 and Hermes suites.
(cherry picked from commit f55541988440c74b3b18c683982789b8fc1a8c1b)
engine::HostObject exposed only string-keyed get/set, so `javaArray[0] = 42`
made the engine spell the index out as "0", ObjectManager allocate a
std::string for it, and TryGetArrayIndex parse the integer back out -- once per
element access, in both directions. The Node-API V8 backend never paid that: it
registers a real indexed interceptor (v8impl::NapiHostObject::IndexedSetter)
that is handed a uint32_t.
HostObject gains getValueAtIndex/setValueAtIndex plus a hasIndexedAccess() flag.
The defaults stringify and call the named form, so a host object that does not
override them behaves exactly as it did; ObjectManager's array proxy overrides
them and opts in when it has a JNI array signature.
Per engine, because the engines differ in what they can hand over:
V8 has a dedicated indexed interceptor, so it always routes there --
previously it built PropNameID(std::to_string(index)) for the sole
purpose of having ObjectManager parse it again. The named handler
stays kNonMasking and the indexed handler stays masking (kNone);
cea83323 measured 20-30% for getting that backwards.
QuickJS has no indexed hook, but interns a canonical index as a tagged-int
atom, so the index is a mask away where JS_AtomToCString allocates.
JS_ATOM_TAG_INT is engine-internal (quickjs.c, not quickjs.h) though
identical in bellard QuickJS and quickjs-ng, so each runtime verifies
the encoding once through the public API before relying on it and
falls back to the string path if it ever fails.
JSC has no indexed hook either and delivers "0" as a JSStringRef, whose
UTF-16 buffer can be parsed in place -- no allocation, where
stringToUtf8 built (and over-allocated) a std::string.
Hermes is real facebook::jsi, which has no indexed hook and no way to read
a PropNameID without building a std::string, so nothing calls the new
methods there. They exist so the same ObjectManager compiles and
behaves identically, and Hermes keeps the named path.
QuickJS and JSC consult hasIndexedAccess() before routing, V8 does not need to:
on those two the named path also carries the non-masking prototype emulation,
which a host object that is not an indexed collection still needs, and skipping
it for every numeric name would be a behaviour change. On V8 the default
reproduces the old call exactly, so there is nothing to gate.
Also stops the V8 indexed setter allocating: it handed setValueAtIndex an owned
Value, which is a shared ValueStorage plus a v8::Global created and destroyed on
every element write. The value does not outlive the call, and the default
setValueAtIndex promotes it before reaching the named setter, so a host object
that does not override the indexed form still gets an owned value. QuickJS and
JSC already borrowed here.
Measured on QV7120NC26 (Sony SO-01M), release, 34-entry marshalling table,
napi and jsi interleaved in one session, two runs each, medians. Controls
(Void Static Method, multiply, Return an Int) held within 10% end to end, so no
throttled regime. Against the same jsi runtime at 02bad159:
V8-13, ms before after napi jsi vs napi before -> after
Int Array[0] write 599.6 392.2 375.9 0.63x -> 0.96x
Double ... 602.0 391.8 377.2 0.63x -> 0.96x
Boolean ... 602.5 369.0 384.5 0.64x -> 1.04x
String ... 1498.4 1239.2 1111.1 0.74x -> 0.90x
TOTAL 13721.8 12307.9 12777.2 1.07x -> 0.96x
So the V8-13 jsi runtime is now 1.04x FASTER than the napi runtime overall,
where it was 1.07x slower, and the write family is at parity.
The causal story is not the one this change started from, and the decomposition
is worth recording. Measuring the index change alone (no borrowed value) on
V8-13 gives Int Array[0] write 563 ms against 581 baseline -- i.e. nothing. On
V8 the whole win is the borrowed value; std::to_string plus a ten-instruction
parse is noise next to a global-handle create/destroy. The index change earns
its place on the engines that genuinely build a string, which V8 never did:
QUICKJS, debug, same device, interleaved, two runs each, medians
(debug because the QuickJS *release* jsi build does not launch -- see below --
and -O0 inflates C++-side costs, so read the direction, not the magnitude)
Int Array[0] read 1544.7 -> 1041.2 1.48x
Double ... 1546.1 -> 1051.2 1.47x
Boolean ... 1543.7 -> 1037.5 1.49x
String ... 2794.6 -> 2389.8 1.17x
Int Array[0] write 1452.5 -> 980.5 1.48x
Double ... 1432.7 -> 1006.0 1.42x
Boolean ... 1441.3 -> 991.3 1.45x
String ... 3515.0 -> 2964.4 1.19x
TOTAL 42163.0 -> 38712.1 1.09x
Every other entry on both engines is flat within run-to-run noise; the full
34-entry table was taken each time, and nothing regressed.
Not measured: JSC and Hermes performance, V8-10, V8-11, PrimJS. JSC should
behave like QuickJS (it also built a std::string per access) but that is a
prediction, not a measurement.
Verified, QV7120NC26, debug, jasmine suite, failing specs compared BY NAME
against the same engine built from 02bad159 in a worktree:
V8-13 516/1 == baseline 516/1
QUICKJS 516/1 == baseline 516/1 (both also produced 516/2 on a rerun,
adding the same flaky `triggers
interval`; it flips on either side)
QUICKJS_NG 516/1 == baseline 516/1
JSC 516/2 == baseline 516/2
HERMES 516/2 == baseline 516/2
The failure common to every engine is a jasmine timeout in
test_if_global_reference_leaks_when_interface_implementation_is_created, which
is one of the 100k-object leak specs mainpage.js warns cannot complete on a
physical device. It fails identically at the parent commit. JSC's `frees up
resources after complete` and Hermes' `Should throw exception when not invoked
as constructor` are likewise present at the parent commit.
iOS, since NativeScript/jsi is shared: V8 713/0, QuickJS 713/0, JSC 713/1
(SpecialCaseProperty_When_CustomSelector_ImplementedInJS, the known baseline).
No new reports in ~/Library/Logs/DiagnosticReports against a pre-run marker.
Apple Hermes was not run -- it hangs the harness, pre-existing.
Unrelated pre-existing bug found while benchmarking, NOT from this change and
not fixed here: a QUICKJS *release* build of the jsi runtime dies at startup
with `NativeScriptException: JavaScript object for Java ID 0 not found`. The
same build from 02bad159 dies identically, and the napi release build of the
same commit runs fine, so it is a jsi-runtime bytecode-path bug that predates
this work.
(cherry picked from commit 1f6ea3c7743d553b5a31446eb2f19877acbb1ca8)
A release build of the jsi runtime on QuickJS died at startup with "NativeScriptException: JavaScript object for Java ID 0 not found", which looked like an object-identity or lifetime bug and is not one. Release builds compile every app module to the active engine's bytecode (tools/bytecode-compiler), so assets/app/MyApp.js in the APK starts with the container magic NSBCQJS rather than JavaScript. The napi runtime tries js_run_bytecode_file first and only falls back to source; the jsi runtime had no bytecode entry point at all and handed the binary blob to the compiler as source. Because a module is compiled *wrapped*, the blob became the body of a function that was defined and never usefully run, so nothing threw: MyApp.js simply never registered the application object, and the first callJSMethodNative for Java ID 0 was the first visible symptom -- several frames and one process away from the cause. The fix gives EngineHost the same two-step the napi tree has: ExecuteBytecodeFile peeks the file's first 8 bytes, and runs it as bytecode only if the magic is this engine's. QuickJS goes through JS_ReadObject/JS_EvalFunction; Hermes hands the raw HBC to evaluateJavaScript, which detects it and skips the parser. V8 and JSC have no compile-time bytecode format -- they cache compiled code at runtime instead -- so they return false and always compile source, which is what their release builds ship anyway. Past the magic check the file *is* bytecode, so a read or eval failure throws instead of falling back. Falling back would compile the binary as source and report the error somewhere unrelated, which is exactly the failure mode above. This lives in the Android EngineHost, not in NativeScript/jsi/, because the container is an artefact of the Android build toolchain and not part of the engine abstraction shared with Apple. No shared file is touched. Verified on QV7120NC26 (Sony SO-01M), release builds, interleaved: QUICKJS jsi 516 specs, 1 failure (86.1s) QUICKJS napi 516 specs, 1 failure (85.8s) <- control, same spec HERMES jsi 516 specs, 2 failures (80.4s) The shared failure is test_if_global_reference_leaks_when_interface_ implementation_is_created, which churns 100k Java objects synchronously and times out on this device under both binding layers. Hermes' second failure is "Should throw exception when not invoked as constructor", the known Hermes limitation that it cannot distinguish new f() from f(). Before this commit both QUICKJS and HERMES release jsi builds could not start at all. (cherry picked from commit c28fab0471ff0087c89373c9fda0332c6dd24cb1)
FieldAccessor::SetJavaField tested its argument with an inverted
condition on the byte and short paths:
jbyte intValue = !is_of_type(value, number) ? get_int32(value) : 0;
so `obj.byteField = 42` stored 0, and only a *non*-number was read as an
int32 -- get_int32 on a non-number is itself meaningless. The int, long,
float and double cases next to them all test the condition the right way
round, which is what the byte and short cases were plainly meant to do.
The bug is in the napi runtime, and the jsi runtime reproduced it
deliberately (with a comment) because the napi tree is the behavioural
oracle. It is fixed in both here, in one commit, so the two trees stay
diffable and neither becomes the odd one out.
Nothing covered these paths, which is why it survived. DummyClass gains
byte/short instance and static fields and testFieldGetSet.js gains four
specs. They are not decorative: built with the inverted condition
restored, all four fail with "Expected 0 to be 42" / "Expected 0 to be
1234" / "Expected 0 to be -7" / "Expected 0 to be -4321", and pass once
the condition is corrected.
Verified on QV7120NC26 (Sony SO-01M), V8-13 debug, interleaved:
jsi inverted 520 specs, 6 failures (the 4 new + 2 device timeouts)
jsi fixed 520 specs, 2 failures
napi fixed 520 specs, 1 failure
The remaining failures are the JNI-reference-leak specs, which churn
10k-100k Java objects synchronously and time out on this device
regardless of binding layer; test_if_callback_parameter_marshalling_leaks
is GC-timing sensitive and appears intermittently.
(cherry picked from commit 27eda36e1f09d65f576d1a1b801a49ffb56c54b6)
(cherry picked from commit 38dc5f5ab9b469c087ee2f9930da1b64380b9953)
(cherry picked from commit f2833cd339e20393a1f0a2b350594e3fe472b7e5)
DjDeveloperr
force-pushed
the
jsi-shared-jsi-layer
branch
from
August 28, 2026 09:13
daeb910 to
ae60da4
Compare
DjDeveloperr
force-pushed
the
jsi-native-android
branch
from
August 28, 2026 09:13
a3c4014 to
641ae5e
Compare
DjDeveloperr
force-pushed
the
jsi-shared-jsi-layer
branch
from
August 28, 2026 09:56
ae60da4 to
4246722
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Android currently reaches every JavaScript engine through Node-API. That works,
but it means every call crosses a C ABI designed for portability rather than for
this runtime, and it caps how much the runtime can exploit what each engine can
actually do.
This adds a second binding layer that talks to the engine wrappers directly,
with no Node-API anywhere. The Android runtime is split into two trees that sit
behind a build flag: the existing Node-API one, still the default, and the new
one. Both build on the same shared engine layer, so a backend fix reaches both,
and either can be selected without touching application code.
The point of keeping both is that they can be compared. The new layer is held to
the same suite as the old one, engine by engine, which is what makes it possible
to say it has reached parity rather than merely that it runs.
Alongside the port, this carries engine-layer work that both platforms get:
faster string and host-object paths, a set of per-engine correctness fixes, and
ahead-of-time bytecode module loading.
What changed
The new binding layer
Reaching parity, engine by engine
Shared engine layer
Engine behaviour
gc()is advisory by design, so the specs that observe reclamation account for that rather than forcing a collection through a debug-only entry pointVerified
Known gaps
Worker(...)call that omitsnew. The other engines do. The guard tells a construct call from a plain one by inspecting the receiver, because the engine layer exposes nonew.target, and the Hermes backend synthesises a receiver for plain calls, which erases the distinction. Hermes on the Node-API layer is unaffected.Notes for reviewers
gc()a real synchronous collection. That is deliberately undone here, so its message describes behaviour the branch no longer has.