Repository navigation
perf: faster object lifecycle and JNI calls - #2099
Draft
edusperoni wants to merge 27 commits into
Draft
edusperoni wants to merge 27 commits into
edusperoni wants to merge 27 commits into
Conversation
One, two and four Java threads call a JS implementation at once with no arguments, three existing objects, or three new objects per call. The benchmark runs asynchronously so the JS thread can release the isolate.
…e lock com.tns.Runtime kept four maps per runtime (id -> object and object -> id, each strong and weak), wrapped in synchronizedMap/ConcurrentHashMap when multithreaded JS is enabled. Every JS wrapper creation, object return and finalization paid for those locks, a second WeakReference per object, and NativeScriptHashMap entry allocations. They are replaced by one ObjectTable: open addressing over the object id and over System.identityHashCode, no per-entry allocation, no locks. Its rule is that only the thread holding the runtime's isolate lock touches it, so the accesses that used to happen in Java before the lock now happen natively after taking it: the receiver of a multithreaded Java -> JS call and every object argument are resolved (and new objects registered) in callJSMethodNative, and createJSInstance registers inside createJSInstanceNative. With multithreaded JS off the receiver is still resolved on the runtime's own thread, as before. Object ids stay monotonic, so native caches keyed by them never see a reused id. NativeScriptHashMap, NativeScriptWeakHashMap and NativeScriptAbstractMap have no other users and are removed.
Resolving every object argument natively cost a JNI upcall per argument inside the lock. ObjectTable gains an optimistic read (StampedLock, API 24+) that any thread may make: writers, already serialized by the isolate lock, bracket each mutation with the write stamp, and a reader that overlaps one gets a miss instead of a torn result. Java passes the ids it reads this way as hints for the receiver and object arguments; the native side uses a hint only when it names a still-linked wrapper (ids are never reused) and resolves exactly under the lock otherwise.
… each A JS wrapper's finalizer called Java once per collected wrapper, on its first collection to weaken the Java object and on later ones to ask whether it was still alive. The finalizer now revives the wrapper and queues its id; one processFinalizedBatch call weakens or checks the whole batch through two int arrays and reports the dead ids, whose wrappers are then released. A batch is flushed inside the finalizer once it reaches 1024 ids, so a long synchronous JS loop holds at most that many Java objects strongly past their wrappers' collection, and otherwise from the event loop (V8 runs the GC epilogue before the finalizers). Every native upcall into the object table now asserts, in debug builds, that the calling thread holds the isolate lock.
Finalizers only queue ids now; the batch is flushed when JS next calls into Java or Java next calls into JS, with the posted event-loop task as the fallback for a thread that goes idle. JS cannot create or receive a Java object without crossing, so Java objects kept strong for collected wrappers cannot pile up during a long synchronous stretch, and nothing calls into Java or frees other handles inside V8's finalizer pass. A wrapper handed back to JS while its id is queued is skipped by the flush, so its Java object is not weakened or released while JS holds it. A failed flush (Java OOM) is retried after a pause instead of throwing out of a finalizer or into an unrelated JS call. releaseNativeCounterpart is no longer public, since it may only run under the isolate lock.
Java hands every class a dense index (once per class per process) and sends it with each object: packed next to the id hint for Java->JS arguments, and returned with the id by the single upcall native already makes for returns and field reads. Native caches the class's JNI name and a global ref of the object's own class by that index, so wrapping a new object no longer calls Class.getName() and FindClass, and links the wrapper to the exact runtime class even when another loader defines the same name. The first sight of a class also stores it by name, which replaces the last-stored-class shortcut. An object that already has an id comes back without its class index; native asks for the class only when it has to create a wrapper.
A Java->JS call with an object JS has never seen used to register it from native code under the isolate lock, so threads calling in with new objects serialized on that upcall. packageArgs now registers the object itself (ObjectTable.findOrRegister: lookup and insert under one write lock, so concurrent registrations agree on one id) and marks the argument fresh; native wraps a fresh argument without asking Java again. The exact reads made under the isolate lock now validate against the same StampedLock, falling back to its read lock, since writers may be on other threads. If a call fails before its arguments are converted, the objects it registered are released again rather than staying strong. Where StampedLock is missing (API < 24) nothing changes: registration stays under the isolate lock.
processFinalizedBatch took the table's write lock once per id while other threads may be registering objects before the isolate lock; it now takes it once per batch. Wrappers collected during a Java->JS call are settled before the call releases the isolate, and the idle-thread fallback task is delayed a few milliseconds so a busy runtime no longer wakes its event loop per GC.
With two Java threads calling into JS, every isolate handoff put the waiter to sleep: V8's mutex spins only briefly, a call holds the isolate for tens of microseconds, and waking a sleeper took longer than the call itself, so the isolate sat idle for a wake-up per call. Java->JS calls now pass a per-runtime gate before v8::Locker whose waiters spin for up to 50us before sleeping. Nested calls from a thread that already holds the isolate skip it. Runtime.setAdmissionSpinMicros changes the budget (negative disables the gate) for benchmarks.
The phone used for these numbers flips between two clock states about 1.6x apart, which swamps an A/B across launches. Contention rows can now run several admission spin budgets round by round inside one launch, and the number of rounds is configurable. The settings live on JniBenchConfig rather than JniBench: every static added to JniBench is another property on the JS constructor its static benchmarks are called through, and two more were enough to slow those rows by ~10 ns.
ConvertJavaArgsToJsArgs returned early for zero arguments before marking the call converted, so every Java->JS call without arguments ran ReleaseUnwrappedArgs' slow path: a JEnv lookup, saving and restoring the pending exception and three array reads, about a third of the native time of callback.noArgs(). Calls without arguments now pass null arrays, so they make no array JNI calls at all.
Arguments crossed as an int[] of types and a long[] of values, so native made three JNI calls per callback (the length and two region copies) and Java allocated two arrays. The type ids now ride at the end of the long[], one per byte, and Java passes the count, so native reads everything with a single GetLongArrayRegion.
…pcall Every object constructed from JS made two upcalls after the constructor: makeInstanceStrong, then getUsedMemory for Isolate::AdjustAmountOfExternalAllocatedMemory. registerConstructedInstance does both. TryCallGC also skips its compare-and-swap when no GC was requested.
Package objects exposed each child (java.lang, android.view...) through a native data property, so every read of `java.lang.Object` ran the C++ getter once per segment: a std::string conversion, a private-symbol registry lookup, and a HasPrivate/GetPrivate pair, even after the child had been resolved. Children are now lazy data properties: V8 calls the getter once and replaces the property with the value, so later reads are plain inline-cached loads. The property keeps its attributes (writable, enumerable, configurable).
Every `new` and every call to an overloaded method built its cache key as a fresh std::string, copied each argument's type name into another, and returned the cached entry by value, copying its signature and return-type strings again. The key is now built in a reused per-thread buffer (copied out only to insert, since resolving calls into Java and may re-enter), the cache is a node map whose entries never move, and lookups return a reference to the entry. MetadataNode::GetName returns a reference instead of a copy.
The per-isolate constants (prototype, toString, valueOf, ...) were created as ordinary strings, so each property access keyed by one first had to look it up in the string table. They are now created internalized.
… a fresh key string Wrapping a Java object created the empty wrapper by calling an API function as a constructor (a full JS -> C++ transition into an empty callback), copied the class's method-callback vector just to confirm its constructor template existed, and set `constructor` with a newly allocated string. The wrapper is now instantiated from the function template's instance template (the same object, without entering the callback), GetConstructorFunction returns the cached function directly, and `constructor` is a cached internalized string.
Without ELF TLS (minSdk < 29) every thread_local access goes through __emutls_get_address. JEnv's cached env and its pinned flag, and the runtime's current runtime and id, were separate thread_locals read together on every JNI entry and runtime lookup; each pair is now one struct.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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. |
…er function each The first touch of a class installs every static field on its constructor, and each field got two JS functions (a getter and a setter) built from their own FunctionTemplates. For android.R.attr that took ~9 ms on the test phone, android.R.style ~6 ms and android.view.View ~5 ms, per runtime. A class with more than 64 static fields is now moved to dictionary mode first (adding properties one by one to a fast-mode object copies its descriptors each time, which made the native accessors below quadratic) and gets: - for a final field, a lazy data property: the first read goes through JNI and the value replaces the accessor, read-only, so later reads are plain property loads (android.R.id.content: 74 ns -> 9 ns); - for any other field, a native accessor pair instead of two functions. First access: android.R.attr 8.9 ms -> 1.6 ms, android.R.style 6.2 -> 0.9 ms, android.view.View 4.9 -> 2.7 ms. Smaller classes keep their accessor functions. Assigning to a final field of a large class now fails like any read-only property (a TypeError in strict code) instead of with the runtime's own error.
A package object gets one lazy property per child, added one by one. On a fast-mode object each addition copies the descriptors, so android.view (~800 µs) and android.widget (~950 µs) cost quadratic time on first access. Packages with more than 64 children now start in dictionary mode, the same way a class with many static fields does.
V8 internalizes every property name, so a name built from a std::string copy with kNormal was copied twice more. Names are now created internalized, and static field names come from a view into the metadata's name table, so installing a static field no longer reads its name into the entry. The field accessors read the name on first use instead.
Each method of a class scanned every method of its base class, so a large class over a large base (TextView over View) did hundreds of thousands of string comparisons. The base class's methods are now indexed by name once.
When it instantiates a constructible API function, V8 also builds that function's prototype object, a constructor property on it and an instance map. A Java class's methods never use them, and a large class has hundreds of methods (View's first access: 2.0 -> 1.5 ms). Instance methods, field and property accessors and the runtime's helper functions are now created with ConstructorBehavior::kThrow. `new view.getAlpha()` already threw (no Java object behind `this`); it now throws a TypeError. Class constructors stay constructible, and so do static methods: code like `new SomeClass.staticFactory()` works today because the factory returns an object, and published plugins rely on it.
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.
Description
Follow-up to #2092/#2093/#2096/#2097. This PR has three parts:
android.R.*,android.view,TextViewand similar no longer pay quadratic or per-member costs when first touched.Multithreaded JS is the model optimized for: the test app runs with it on.
Object lifecycle
com.tns.Runtimekept four id maps (strong/weak × both directions), synchronized when multithreaded JS is on. OneObjectTablereplaces them: open addressing over the id and overSystem.identityHashCode, with no per-entry allocation.NativeScriptHashMap/WeakHashMap/AbstractMapare removed.StampedLockread (API 24+). Java passes the ids it reads as hints, and native re-checks them under the isolate lock.processFinalizedBatchcall per batch replaces one JNI call per wrapper. The batch flushes at the next JS↔Java crossing, with a delayed event-loop task as the idle fallback. A wrapper handed back to JS while queued is skipped.Class.getName()andFindClass. This makes the class-name cache in #2075 unnecessary.ObjectTable.findOrRegisterdoes the find and insert under one write lock. A call that fails before converting its arguments releases the objects it registered. API < 24 keeps registration under the isolate lock.v8::Lockerthat spins for up to 50 µs before sleeping. Nested calls skip it.Runtime.setAdmissionSpinMicros(public, static) tunes the budget; a negative value disables the gate.Single-thread call paths
callback.noArgs().long[], so native makes one region read instead of three JNI calls.new X()made two upcalls after the constructor; now it makes one.java.lang.Objectran a C++ getter for every package segment on every access. Children are now lazy data properties.std::strings on every call."constructor"string.JEnvandRuntimestate are each read as one struct.First access of large classes
android.R.attr. Classes with more than 64 static fields now go to dictionary mode first: adding native accessors one by one to a fast-mode object copies its descriptors each time. Final fields become lazy read-only data properties, so after the first read they are plain property loads (R.id.content: 74 → 9 ns). Small classes keep the getter/setter functions, which call faster on a fast-mode object.android.viewandandroid.widgetwere quadratic for the same reason.kNormalfrom astd::stringcopy were copied twice more. Static field names come from a view into the metadata's name table.TextViewoverView.constructorproperty and an instance map, which a method never uses. Instance methods, field/property accessors and helper functions now useConstructorBehavior::kThrow. Class constructors and static methods stay constructible (see Notes for review).Numbers
These are medians in ns on a Samsung SM-G780G (Snapdragon 865, not rooted): release APK,
speed-compiled, 7 rounds. The phone flips between two clock states about 1.6× apart. The table below is the one launch pair where both builds ran in the same (slow) state, checked by the reference rows (field.get int287 vs 286). In the fast state every number is about 1.6× lower, and the relative changes match other matched runs.Java threads calling one runtime at once, in ns per call (from the same launch pair). "Gate off" and "gate on" alternate round by round inside one launch:
Single-threaded JS (
enableMultithreadedJavascript: false), main vs this PR, one matched launch pair:First access of large classes, in µs. The first table is the static-field commit against the commit before it. The second is the other three commits against the static-field commit, from one launch pair per build in the same clock state with a full GC before each row. First access is measured once per launch, so each value is a median over 8 launches.
The non-constructor commit, against the commit before it (same method):
Steady state is unchanged:
View.getAlpha()261 → 259 ns, readingandroid.view.Viewthrough the package 15.6 ns in both, small-class static field reads and writes within ±1%.Known costs
Notes for review
Tried and dropped for first access:
View's first access by 40%, but each method's first call got more expensive. With V8'sSetLazyDataProperty, 20 first calls took 443 µs instead of 207, and later calls were 5% slower. With an accessor that replaces itself, they took 3.5 ms: the delete makes V8 rebuild the whole prototype's layout.View's remaining ~1.5 ms is turning its ~665 methods into JS functions.Behavior change, non-constructors:
new view.getAlpha()already threw "No java object found…", becausethisis not a Java wrapper; it now throws aTypeError(not a constructor). Method functions no longer have a.prototype. Static methods were left constructible on purpose:new SomeClass.staticFactory()returns the factory's object today, and published plugins do this (e.g.new …TransferUtility.builder()). A spec pins that.Gate teardown window: the gate lives in
Runtimeand inherits the existing teardown window. A Java thread that passesTryGetRuntimewhile the runtime is being destroyed was already a hazard before this PR.API < 24: there's no
StampedLock, so there are no optimistic reads, and every table access stays under the isolate lock as before.Benchmark harness: adding static methods to
JniBenchslowed its static rows by ~10 ns, through V8's property lookup on the JS constructor. The harness settings therefore live onJniBenchConfig. Any Java class with many statics probably has the same constructor-lookup cost in real apps, and is worth a follow-up.Does your pull request have unit tests?
Yes.
runtestsAndVerifyResults-equivalent run on an API 36 emulator, 1499 specs, 0 failures. New specs intestGC.js("collected wrappers waiting for a flush") cover:ObjectTableTest(9 tests, including concurrent registration and exact reads during writes). The runtime unit tests pass, 25/25.