Repository navigation
feat(jsi): sync JSI to React Native 0.88 and make android-jsi load - #79
Open
NathanWalker wants to merge 10 commits into
Open
NathanWalker wants to merge 10 commits into
NathanWalker wants to merge 10 commits into
Conversation
Brings in the IRuntime split along with runtime data (jsi::UUID), Array::push, ArrayBuffer::tryGetMutableBuffer, Value-keyed property access and the create*Error helpers. Runtime supplies JS-backed defaults for every new method, so V8Runtime only needs invalidate() to become noexcept to match the new PointerValue contract. Nitro Modules v1 (nitro-core) requires these APIs.
Runtime's defaults for the new APIs go through JS (Reflect.get, per-element setProperty) or a process-wide mutex (runtime data), and its tryGetMutableBuffer always copies. Nitro reads runtime data for every struct field and ArrayBuffer it converts, so V8Runtime now implements these directly: - runtime data in a per-runtime map, released before the context - Value-keyed get/has/set/delete and PropNameID/String delete - Array push, ArrayBuffer detached - tryGetMutableBuffer returns the MutableBuffer behind ArrayBuffers created with createArrayBuffer, so native buffers round-trip without a copy - queueMicrotask enqueues on the isolate instead of doing nothing
V8Runtime wraps the isolate NativeScript already owns, but its destructor disposed that isolate, so DisposeJSIRuntimeForCurrentIsolate() tore down the app's JS engine.
Async JSI libraries need a way to get back onto the runtime's thread, the role React Native's CallInvoker plays. tns::GetJSICallInvoker() returns one backed by the JS thread's CFRunLoop on iOS and ALooper on Android. Tasks run through V8Runtime::ExecuteTask: a top-level JS entry under a verbose TryCatch, so errors reach NativeScript's uncaught error handling, followed by a microtask checkpoint so settled promises resume immediately. The per-isolate runtime map is now leaked so exit-time static destructors never release a runtime after its isolate is gone.
Runtime's defaults transcode through UTF-8 in both directions, and createStringFromUtf16 evaluates an escaped literal for any non-ASCII input. V8Runtime now creates two-byte strings directly, copies out with WriteV2, and getStringData/getPropNameIdData expose V8's flat string contents without copying (Latin-1 one-byte strings are widened, since JSI's ascii flag means 7-bit). Nitro's Swift string bridge relies on these; build it with NITRO_JSI_HAS_FAST_UTF16=1 against this runtime.
libnativescript-jsi.so could not be loaded at all: - It had no DT_NEEDED entry for libNativeScript.so, and Android's linker only binds a library's undefined symbols through its own dependency tree, so every V8 symbol failed to resolve. It now links an empty stub whose soname is libNativeScript.so. - Inline V8 header code (Object::GetInternalField) calls v8::internal::Internals::GetCurrentIsolate(), which libNativeScript.so does not export. Define it as Isolate::GetCurrent(), as V8 does. - The V8 headers were compiled without V8_COMPRESS_POINTERS and V8_31BIT_SMIS_ON_64BIT_ARCH, which the runtime's V8 is built with, so inline handle code misread objects and crashed. They are now PUBLIC definitions so every library including V8 through nativescript-jsi matches.
Each V8Runtime method took a v8::Locker and entered the isolate and context, and every released jsi::Value did the same in V8PointerValue::invalidate(). NativeScript never takes a Locker itself, so each one was a real mutex acquire. JSI calls almost always come from host functions that are already inside the isolate and context, so JSIScope now enters only what is missing. Nitro method calls on Android go from ~860 ns to ~415 ns.
…t call getNativeState() entered the isolate and resolved the object three times (isHostObject, the hasNativeState assert, then the lookup), and Nitro calls it on every method call to unwrap `this`. It now reads the internal field type once. The host function container copied its arguments into a heap-allocated std::vector on every call; up to eight now use the stack.
Host functions received undefined as thisVal whenever the receiver had no internal fields, i.e. for every plain JS object, so a host function installed on an ordinary object or prototype could not see the object it was called on.
The NativeScript CLI already builds platforms/android/java into the plugin's AAR, and include.gradle added the same directory to the app's sources, so release builds failed dexing with "Type org.nativescript.jsi.JSIRuntime is defined multiple times".
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
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 |
This branch has not been deployed
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
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.
Stacked on #76. Brings
@nativescript/ios-jsiand@nativescript/android-jsiup to current JSI so libraries built against React Native 0.88's JSI (e.g. Nitro Modules v1nitro-core) drop in, and fixes the Android package so it actually loads.Summary
packages/{ios,android}-jsi jsi/* # vendored facebook::jsi - # ~RN 0.77 revision + # RN 0.88.0, verbatim V8Runtime + runtime data, Value-keyed props, push, detached # native, no JS fallback + tryGetMutableBuffer # native buffers round-trip without a copy + UTF-16 create/read, getStringData # upstream default eval()s non-ASCII + queueMicrotask # was a no-op + ExecuteTask() # top-level JS entry + microtask checkpoint + JSIScope # enter isolate/context only when not already inside - ~V8Runtime disposes the isolate # it is NativeScript's isolate JSIRuntime + tns::GetJSICallInvoker() # CallInvoker: CFRunLoop (iOS) / ALooper (Android) android-jsi/platforms/android + cpp: DT_NEEDED libNativeScript.so via soname stub + cpp: Internals::GetCurrentIsolate shim (not exported by libNativeScript.so) + cpp: PUBLIC V8_COMPRESS_POINTERS / V8_31BIT_SMIS_ON_64BIT_ARCH (match the runtime's V8) - include.gradle # duplicated JSIRuntime.java in release dexA Nitro host on top of this is a ~40 line
Dispatcherforwarding toJSICallInvokerplusmargelo::nitro::install(); see nstudio/ns-nitrofeat/nitro-v1(nitro/host/NativeScriptNitro.cpp).Evidence
Before:
nitro-corev1 fails to compile against the vendoredjsi.h(missingjsi::UUID/runtime data,Array::push,ArrayBuffer::tryGetMutableBuffer,Runtime::createError,getProperty(Value)). On Android, loadinglibnativescript-jsi.sofails withcannot locate symbol "_ZN2v87Isolate10GetCurrentEv", then..._ZN2v88internal9Internals17GetCurrentIsolateEv, then SIGSEGV inJSIV8ValueConverter::ToV8String; release builds fail dexing withType org.nativescript.jsi.JSIRuntime is defined multiple times.After: ns-nitro runs real Nitro v1 Hybrid Objects (C++, Swift, Kotlin; constructors, static props, structs, native Promises resolved via
JSICallInvoker, zero-copy ArrayBuffers) on the iOS simulator and an Android emulator, debug and release.Nitro method call (
add(a, b)), each row measured like-for-like:JSIScope)getNativeState, no per-call alloc)Release, current: Nitro → C++ 188 ns (iOS) / 212 ns (Android) vs classic interop 223 ns / 663 ns.
Merge Danger
Door: two-way
Revert restores the previous packages; no persisted state or published API is removed.
Blast Radius: JSI consumers
jsi.hchanges ABI (IRuntimesplit,PointerValue::invalidate()is nownoexcept), so any native code compiled against these packages must rebuild. Host functions now receive the real receiver asthisinstead ofundefinedfor plain objects.V8Runtimeno longer takes av8::Lockerwhen the caller is already inside the isolate, and no longer disposes the isolate on destruction.