feat!: upgrade V8 to 14.9.207.39#1987
Draft
edusperoni wants to merge 8 commits into
Draft
Conversation
Reproduces the prebuilt libv8_monolith.a from source: - fetch_v8.sh pins 14.9.207.39 and applies the patches - build_v8_source.sh builds an ABI and vendors the matching headers - build_v8_linux_docker.sh covers the two 32-bit ABIs, which cannot be built on macOS -- mksnapshot has to run V8's simulator for a 32-bit target and v8config.h only permits that from an ia32 host - vendor_inspector_sources.py refreshes the V8 internals that the inspector glue compiles against, by transitive closure from what it actually includes v8_resurrecting_finalizers.patch restores WeakCallbackType::kFinalizer, removed upstream right after 10.3.22; ObjectManager depends on it. android_build_on_macos.patch relaxes chromium's linux-host assert and makes android_ndk_root selectable.
Refreshed wholesale from the pinned checkout by build_v8_source.sh. The deletions are 10.3-era headers that no longer exist upstream; nothing includes them. zip.h/zipconf.h belong to libzip and inspector/ holds the generated protocol headers, so both are left alone by the vendoring step.
JsV8InspectorClient and ns-v8-tracing-agent-impl compile against src/inspector, which is not public API, so this copy has to move with libv8_monolith.a or it is an ABI mismatch. Regenerated by transitive closure from the six headers the runtime actually includes. src/common/globals.h now reaches abseil through base/numbers/double.h, so the needed absl headers come along and land at the top of the tree, where the "absl/..." spelling already resolves.
Mechanical, except where noted: - Context/Object/Function/Promise/Message::GetIsolate() removed -> Isolate::GetCurrent() - External::New/Value() and the aligned internal-field accessors take a type tag - Object::CreationContext() -> GetCreationContext(isolate) - ScriptOrigin no longer takes an isolate; AccessControl is gone - GetInternalField returns Local<Data> - V8Inspector::connect takes a trust level; createForConsoleAPI takes a span - FunctionCallbackInfo is no longer copyable, so ArgsWrapper holds a reference -- every instance is a local in the callback it wraps Accessors are the part that is not mechanical. PropertyCallbackInfo no longer exposes the receiver at all, and SetNativeDataProperty is not a drop-in for SetAccessor on anything that is inherited from. Six MetadataNode accessors live on an object other than the one they are read through -- class, nullObject and static fields on the constructor, super on the implementation object, instance fields and properties on the prototype template -- and are now SetAccessorProperty with FunctionTemplate-backed callbacks, whose This() is still the receiver. Static fields matter most: they have a setter, and a data-like property would let Derived.baseField = x shadow the base and silently never reach it. The field accessors used thiz->StrictEquals(info.Holder()) to detect an instance field read straight off the prototype. Function-backed accessors have no holder, so that is now !IsJsRuntimeObject(thiz) -- being a runtime-managed object is what actually separates an instance from the prototype. The array wrapper's indexed handler returns v8::Intercepted; both paths handle the access completely and neither ever fell through. V8::Initialize() freezes the flag list and changing a flag afterwards aborts, so the app's v8Flags are applied once in InitializeV8() rather than per isolate. unistd.h is included explicitly where usleep/read used to arrive transitively.
V8 14.9's src/base/atomicops.h uses std::atomic_ref unconditionally, and r27d ships libc++ 18, which does not implement it. The runtime compiles that header because v8_inspector vendors V8 internals, so the bump is forced rather than opportunistic. r29 is the first released NDK new enough. minSdk is unchanged at 21. V8 has to be built against the same NDK: libc++ is only ABI-compatible with itself across a static link, and V8's bundled NDK is newer than any released one -- mixing them fails the link on std::__ndk1::__hash_memory. -Pabis=arm64-v8a,x86_64 restricts abiFilters, which is needed while the 32-bit monoliths can only be produced on a linux/amd64 host.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository 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 |
The 32-bit build runs for hours. With stdin attached the container died along with whatever shell started it, losing the work. It now starts detached and the logs are followed separately, so interrupting the script leaves the build running; --attach picks the stream back up.
Only the host assert in that patch is macOS-specific, and widening it is a no-op on Linux. The API 21 floor and the android_ndk_root gn arg are needed everywhere -- gating the whole patch on Darwin broke the linux container at gn gen with "default_min_sdk_version (21) must be >= min_supported_sdk_version (23)". Renamed to match what it actually does.
depot_tools and the ~3GB NDK were fetched into the container filesystem, so every retry re-downloaded them; they now live in the same volume as the V8 checkout. The mounted build script also moves off mktemp and loses its cleanup trap. bash reads a script incrementally, so deleting it when the launching shell exits could break a container that is still running -- the whole point of detaching.
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
1 participant
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.
Upgrades the runtime from V8 10.3.22 → 14.9.207.39, on top of
feat/error-handling.Draft until NativeScript/v8-buildscripts cuts the release this branch pins — see Prerequisite. Everything else is complete and validated.
Status
The five skips are the pre-existing
xit()s already in the suite.The libraries are no longer committed
./download_v8.shinstalls them from the release pinned inV8_RELEASE, verifying every archive against theSHA256SUMSpublished with it. It is a no-op once they are in place,V8_SKIP_DOWNLOAD=1skips it entirely, andbuild.shcalls it — deliberately not a Gradle task, matching how the iOS runtime treatsdownload_llvm.sh. Keeping it out of the build means a stale or unreachable release cannot wedge an otherwise working build.Two reasons this had to change:
arm64-v8aandx86_64monoliths are 100.7 MiB and 106.1 MiB, over GitHub's hard 100 MiB per-file push limit. Stripping recovers ~2%, leaving x86_64 still over.v8config.hrefuses anything else — and the Apple variants need macOS. Hand-assembled artifacts are therefore neither reproducible nor verifiable.The vendored headers are ignored along with the libraries, not just the binaries. Keeping a copy in git is how it drifts out of step with the libraries it describes — the iOS runtime was carrying 10.3 crdtp headers against a 14.9
libcrdtp.afor exactly that reason. Sourcing both from one verified artifact makes the mismatch impossible.libzip.aandzip.hstay tracked; they are not V8.tools/v8/vendor_inspector_sources.pystays too: it recomputes the inspector's internals by closure from the release'ssrc-headersartifact. What the glue includes is this repo's business, not the build repo's.Prerequisite
V8_RELEASEpinsv8-14.9.207.39-1, which does not exist yet — buildscripts' matrix has to go green first. Until then a fresh checkout will not link. Contributors with a local V8 build can useV8_SKIP_DOWNLOAD=1.What changed
Build config. JIT and WebAssembly stay on, i18n stays off — same shape as the 10.3 build. The gn args live in buildscripts now; the reasoning is in
docs/knowledge/v8-14-migration.md. The one worth repeating here:v8_array_buffer_internal_field_count/..._view_...defaulted to 2 in 10.3 and default to 0 in 14.9.JSToJavaObjectsConvertermarshals an ArrayBuffer/SharedArrayBuffer/typed array to a Java NIO buffer by callingObjectManager::Linkon the buffer object itself, andLinkstores itsJSInstanceInfoin internal field 0. With zero fields every such conversion throws "Trying to link invalid 'this' to a Java object" — this was the only test failure in the first full run.v8-array-buffer.hstill falls back to2when the macro is undefined, so the gn default also put V8 and the embedder silently out of agreement.NDK r27d → r29. Forced: V8 14.9's
src/base/atomicops.husesstd::atomic_refunconditionally and r27d ships libc++ 18, which does not implement it. The runtime compiles that header becausev8_inspectorvendors V8 internals.minSdkis unchanged at 21. V8 must be built against the same NDK — libc++ is only ABI-compatible with itself across a static link, and V8's bundled NDK is newer than any released one (mixing fails onstd::__ndk1::__hash_memory).API migration. Mostly mechanical — full table in the doc. The one part that is not:
PropertyCallbackInfono longer exposes the receiver at all, andSetNativeDataPropertyis not a drop-in forSetAccessoron anything inherited from. SixMetadataNodeaccessors live on an object other than the one they are read through —class,nullObjectand static fields on the constructor,superon the implementation object, instance fields and properties on the prototype template — and are nowSetAccessorPropertywithFunctionTemplate-backed callbacks, whoseThis()is still the receiver. Static fields are the sharp edge: they have a setter, and a data-like property would letDerived.baseField = xshadow the base with an own property and silently never reach the native setter.One guard could not be translated directly. The field accessors used
thiz->StrictEquals(info.Holder())to catch an instance field read straight off the prototype; function-backed accessors have no holder, so that is now!objectManager->IsJsRuntimeObject(thiz)— being a runtime-managed object is what actually separates an instance from the prototype. The two agree for every receiver the old check could see.Flags.
V8::Initialize()freezes the flag list and changing a flag afterwards aborts the process, sov8Flagsis applied once inInitializeV8()instead of per isolate.Size impact
Optimized, stripped, arm64-v8a
libNativeScript.so, against the publishednativescript-optimized.aar:Roughly half of that is V8's two new compiler tiers, which did not exist in 10.3: Turboshaft 6.1 MiB + Maglev 3.4 MiB. Levers, if size matters more than warm-up:
v8_enable_maglev=false(no feature loss — pure new cost since 10.3), orv8_enable_webassembly=false, measured at −9.9 MiB raw / −2.5 MB gzip, which would halve the upgrade's download cost. WASM is left on here to preserve current behaviour; turning it off would also needMessageLoopTimer::Initguarded, since its proxy script references the bareWebAssemblyidentifier at every isolate startup.JIT is confirmed working on 14.9: a hot loop runs at 0.82 ns/iteration against 356 ns/iteration for the same body wrapped in
with(o){}to block optimisation.Notes for review
docs/knowledge/v8-14-migration.mdcarries the full API table and the reasoning behind every build-config pin.V8_STATIC_ROOTSis deliberately not defined by the runtime. V8 is built with it and it would letValue::IsUndefined()/IsNull()use the inline static-root comparison, but the root addresses are hardcoded constants inv8-internal.hthat must match the library exactly. 10.3 had no such fast path, so this is parity; enabling it is a separate, measurable change.SomeClass.nullpermanently breaks that class's staticvalueOf(). Confirmed pre-existing on 10.3, filed as ReadingSomeClass.nullpermanently breaks that class's staticvalueOf()#1986.