From 7f7ad3a1b717c84fc2d2fd2d72c4e42bc89eadf6 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 11 Nov 2026 08:07:58 -0300 Subject: [PATCH 1/2] perf: guard ObjectTable with a monitor instead of StampedLock StampedLock lives in the boot classpath but outside the boot image, so on device it runs interpreted, and an app's baseline profile can't compile it. A monitor's uncontended enter/exit is compiled into every caller. Reads now take the lock too, which also removes the torn-read handling the optimistic path needed. --- .../src/main/java/com/tns/ObjectTable.java | 217 ++++-------------- .../src/main/java/com/tns/Runtime.java | 6 +- .../test/java/com/tns/ObjectTableTest.java | 8 +- 3 files changed, 54 insertions(+), 177 deletions(-) diff --git a/test-app/runtime/src/main/java/com/tns/ObjectTable.java b/test-app/runtime/src/main/java/com/tns/ObjectTable.java index 4962d3488..7710d657a 100644 --- a/test-app/runtime/src/main/java/com/tns/ObjectTable.java +++ b/test-app/runtime/src/main/java/com/tns/ObjectTable.java @@ -7,10 +7,13 @@ * holds its object strongly while the JS wrapper is reachable and through a WeakReference * after the wrapper was collected (until the object is collected too). * - * Where StampedLock exists (API 24+) any thread may read and findOrRegister: writers are - * serialized by its write lock and exact reads validate against it. Without it, every - * mutation and every exact read must come from the thread that holds the runtime's - * isolate lock. Neither table allocates per entry. + * When concurrent, any thread may read and findOrRegister: every access synchronizes on the + * table. Otherwise every mutation and every read must come from the thread that holds the + * runtime's isolate lock. Neither table allocates per entry. + * + * A monitor rather than java.util.concurrent locks: those live in the boot classpath but + * outside the boot image, so on device they run interpreted, and an app's profile can't + * compile them. Uncontended monitor enter/exit is compiled into every caller. */ final class ObjectTable { static final int NO_ID = -1; @@ -22,114 +25,28 @@ interface IdSource { int newId(); } - // Null where StampedLock is unavailable (API < 24): findIdOptimistic then always misses. - private final Stamps stamps; + private final boolean concurrent; ObjectTable() { this(false); } - ObjectTable(boolean optimisticReads) { - stamps = optimisticReads ? new Stamps() : null; + ObjectTable(boolean concurrent) { + this.concurrent = concurrent; } /** - * Writers are already serialized by the isolate lock; the write stamp only lets an - * optimistic reader detect that a write overlapped it. Kept in its own class so that - * runtimes where StampedLock is missing never load it. - */ - private static final class Stamps { - private final java.util.concurrent.locks.StampedLock lock = new java.util.concurrent.locks.StampedLock(); - - long beginWrite() { - return lock.writeLock(); - } - - void endWrite(long stamp) { - lock.unlockWrite(stamp); - } - - long beginRead() { - return lock.tryOptimisticRead(); - } - - long readLock() { - return lock.readLock(); - } - - void unlockRead(long stamp) { - lock.unlockRead(stamp); - } - - boolean validate(long stamp) { - return lock.validate(stamp); - } - } - - /** - * The id obj is registered under, from any thread, without the isolate lock. A miss - * (NO_ID) is not authoritative: it is also returned when a write overlapped the read. - * A hit was correct at the time of the read; the object may have been released since, - * which the caller must check under the isolate lock. + * The id obj is registered under, from any thread, without the isolate lock; NO_ID when + * the table isn't concurrent. A hit was correct at the time of the read; the object may + * have been released since, which the caller must check under the isolate lock. */ int findIdOptimistic(Object obj) { - if (stamps == null) { - return NO_ID; - } - long stamp = stamps.beginRead(); - if (stamp == 0) { + if (!concurrent) { return NO_ID; } - int id; - try { - id = findIdUnlocked(obj); - } catch (RuntimeException e) { - // A torn view of a resize can index past an array; the stamp check below fails then. - id = NO_ID; - } - return stamps.validate(stamp) ? id : NO_ID; - } - - private int findIdUnlocked(Object obj) { - int[] index = identityIndex; - int[] hashes = identityIndexHashes; - int mask = index.length - 1; - int hash = System.identityHashCode(obj); - int i = mix(hash) & mask; - for (int probes = 0; probes <= mask; probes++) { - int entry = index[i]; - if (entry == 0) { - return NO_ID; - } - if (hashes[i] == hash && getUnlocked(entry - 1) == obj) { - return entry - 1; - } - i = (i + 1) & mask; - } - return NO_ID; - } - - // get() without the lock. Safe on a torn view: every array it reads is indexed within - // its own bounds, and a stale one only makes the caller's stamp check fail. - private Object getUnlocked(int id) { - if (id == NO_ID) { - return null; - } - Chunk[] cs = chunks; - int c = id >>> CHUNK_SHIFT; - if (c >= cs.length) { - return null; - } - Chunk chunk = cs[c]; - if (chunk == null) { - return null; - } - Leaf leaf = chunk.leaves[(id >>> LEAF_BITS) & CHUNK_MASK]; - if (leaf == null) { - return null; + synchronized (this) { + return findIdLocked(obj); } - Object value = leaf.values[id & LEAF_MASK]; - return value == null ? null : deref(value); } private static Object deref(Object value) { @@ -177,15 +94,12 @@ private static final class Chunk { /** Registers obj under id, held strongly, replacing whatever id held before. */ void putStrong(int id, Object obj) { - if (stamps == null) { + if (!concurrent) { putStrongLocked(id, obj); return; } - long stamp = stamps.beginWrite(); - try { + synchronized (this) { putStrongLocked(id, obj); - } finally { - stamps.endWrite(stamp); } } @@ -217,18 +131,17 @@ private void putStrongLocked(int id, Object obj, int hash) { } boolean allowsConcurrentRegistration() { - return stamps != null; + return concurrent; } /** * The id obj is registered under, registering it strongly under a new id from ids when * it has none; FRESH is set in the result when this call registered it. The lookup and - * the insert happen under one write lock, so concurrent calls agree on one id. Only - * when allowsConcurrentRegistration(). + * the insert happen under one lock, so concurrent calls agree on one id. Only when + * allowsConcurrentRegistration(). */ long findOrRegister(Object obj, IdSource ids) { - long stamp = stamps.beginWrite(); - try { + synchronized (this) { int hash = System.identityHashCode(obj); int id = findIdLocked(obj, hash); if (id != NO_ID) { @@ -237,32 +150,16 @@ long findOrRegister(Object obj, IdSource ids) { id = ids.newId(); putStrongLocked(id, obj, hash); return (id & 0xFFFFFFFFL) | FRESH; - } finally { - stamps.endWrite(stamp); } } /** The id obj is registered under, or NO_ID. */ int findId(Object obj) { - if (stamps == null) { + if (!concurrent) { return findIdLocked(obj); } - long stamp = stamps.beginRead(); - if (stamp != 0) { - try { - int id = findIdUnlocked(obj); - if (stamps.validate(stamp)) { - return id; - } - } catch (RuntimeException e) { - // A torn view of a resize; retried under the read lock. - } - } - stamp = stamps.readLock(); - try { + synchronized (this) { return findIdLocked(obj); - } finally { - stamps.unlockRead(stamp); } } @@ -288,25 +185,11 @@ private int findIdLocked(Object obj, int hash) { /** The object registered under id; null if there is none or it was collected. */ Object get(int id) { - if (stamps == null) { + if (!concurrent) { return getLocked(id); } - long stamp = stamps.beginRead(); - if (stamp != 0) { - try { - Object value = getUnlocked(id); - if (stamps.validate(stamp)) { - return value; - } - } catch (RuntimeException e) { - // A torn view of a resize; retried under the read lock. - } - } - stamp = stamps.readLock(); - try { + synchronized (this) { return getLocked(id); - } finally { - stamps.unlockRead(stamp); } } @@ -316,14 +199,11 @@ private Object getLocked(int id) { } boolean contains(int id) { - if (stamps == null) { + if (!concurrent) { return rawLocked(id) != null; } - long stamp = stamps.readLock(); - try { + synchronized (this) { return rawLocked(id) != null; - } finally { - stamps.unlockRead(stamp); } } @@ -332,35 +212,35 @@ boolean contains(int id) { * the object is still alive; an entry whose object was already collected is removed. */ boolean weakenAndCheckAlive(int id) { - if (stamps == null) { + if (!concurrent) { return weakenAndCheckAliveLocked(id); } - long stamp = stamps.beginWrite(); - try { + synchronized (this) { return weakenAndCheckAliveLocked(id); - } finally { - stamps.endWrite(stamp); } } /** - * weakenAndCheckAlive for ids[0..count), under one write lock; overwrites the id of - * every object no longer alive with NO_ID. + * weakenAndCheckAlive for ids[0..count), under one lock; overwrites the id of every + * object no longer alive with NO_ID. */ void weakenAndCheckAlive(int[] ids, int count) { if (count == 0) { return; } - long stamp = stamps == null ? 0 : stamps.beginWrite(); - try { - for (int i = 0; i < count; i++) { - if (!weakenAndCheckAliveLocked(ids[i])) { - ids[i] = NO_ID; - } - } - } finally { - if (stamps != null) { - stamps.endWrite(stamp); + if (!concurrent) { + weakenAndCheckAliveAllLocked(ids, count); + return; + } + synchronized (this) { + weakenAndCheckAliveAllLocked(ids, count); + } + } + + private void weakenAndCheckAliveAllLocked(int[] ids, int count) { + for (int i = 0; i < count; i++) { + if (!weakenAndCheckAliveLocked(ids[i])) { + ids[i] = NO_ID; } } } @@ -383,15 +263,12 @@ private boolean weakenAndCheckAliveLocked(int id) { } void remove(int id) { - if (stamps == null) { + if (!concurrent) { removeLocked(id); return; } - long stamp = stamps.beginWrite(); - try { + synchronized (this) { removeLocked(id); - } finally { - stamps.endWrite(stamp); } } diff --git a/test-app/runtime/src/main/java/com/tns/Runtime.java b/test-app/runtime/src/main/java/com/tns/Runtime.java index 940a1997f..fd2e43ea5 100644 --- a/test-app/runtime/src/main/java/com/tns/Runtime.java +++ b/test-app/runtime/src/main/java/com/tns/Runtime.java @@ -181,9 +181,9 @@ public void run() { "Primitive types need to be manually wrapped in their respective Object wrappers.\n" + "If you are creating an instance of an inner class, make sure to always provide reference to the outer `this` as the first argument."; - // Any thread may read it optimistically and register new objects through findOrRegister - // (see ObjectTable); every other mutation and every exact read comes from the thread - // holding this runtime's isolate lock. + // On API 24+ any thread may read it and register new objects through findOrRegister + // (see ObjectTable); every other mutation comes from the thread holding this runtime's + // isolate lock. private final ObjectTable objectTable; private final ObjectTable.IdSource objectIds = new ObjectTable.IdSource() { diff --git a/test-app/runtime/src/test/java/com/tns/ObjectTableTest.java b/test-app/runtime/src/test/java/com/tns/ObjectTableTest.java index f2d88ae41..71d080b25 100644 --- a/test-app/runtime/src/test/java/com/tns/ObjectTableTest.java +++ b/test-app/runtime/src/test/java/com/tns/ObjectTableTest.java @@ -78,7 +78,7 @@ public void aRegisteredWeakReferenceIsNotMistakenForAWeakEntry() { } @Test - public void optimisticReadsNeverReturnAWrongIdWhileAnotherThreadWrites() throws Exception { + public void concurrentReadsFindStableEntriesWhileAnotherThreadWrites() throws Exception { final ObjectTable table = new ObjectTable(true); final int stableCount = 64; final Object[] stable = new Object[stableCount]; @@ -127,8 +127,8 @@ public void run() { int id = table.findIdOptimistic(stable[expected]); if (id == expected) { hits.incrementAndGet(); - } else if (id != ObjectTable.NO_ID) { - failure.compareAndSet(null, "expected " + expected + " or NO_ID, got " + id); + } else { + failure.compareAndSet(null, "expected " + expected + ", got " + id); } } } @@ -147,7 +147,7 @@ public void run() { } assertNull(failure.get(), failure.get()); - assertTrue("optimistic reads should mostly hit", hits.get() > 1000); + assertTrue("readers should have run", hits.get() > 1000); } @Test From fee28fb3e598cafe93351f56544ffb599b654b9d Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 11 Nov 2026 11:22:09 -0300 Subject: [PATCH 2/2] perf: register objects concurrently on every API level ObjectTable was only concurrent where StampedLock exists (API 24+). With a monitor there is no reason to keep API 21-23 on the slower paths: argument objects resolved natively under the isolate lock, and new ids registered through a separate path. findIdOptimistic is now findId, and getOrCreateObjectId and packageArgs register through findOrRegister alone. --- .../src/main/java/com/tns/ObjectTable.java | 65 ++----------------- .../src/main/java/com/tns/Runtime.java | 29 ++------- .../test/java/com/tns/ObjectTableTest.java | 18 ++--- 3 files changed, 19 insertions(+), 93 deletions(-) diff --git a/test-app/runtime/src/main/java/com/tns/ObjectTable.java b/test-app/runtime/src/main/java/com/tns/ObjectTable.java index 7710d657a..eec847e5b 100644 --- a/test-app/runtime/src/main/java/com/tns/ObjectTable.java +++ b/test-app/runtime/src/main/java/com/tns/ObjectTable.java @@ -7,9 +7,8 @@ * holds its object strongly while the JS wrapper is reachable and through a WeakReference * after the wrapper was collected (until the object is collected too). * - * When concurrent, any thread may read and findOrRegister: every access synchronizes on the - * table. Otherwise every mutation and every read must come from the thread that holds the - * runtime's isolate lock. Neither table allocates per entry. + * Any thread may read and findOrRegister: every access synchronizes on the table. Neither + * table allocates per entry. * * A monitor rather than java.util.concurrent locks: those live in the boot classpath but * outside the boot image, so on device they run interpreted, and an app's profile can't @@ -25,30 +24,6 @@ interface IdSource { int newId(); } - private final boolean concurrent; - - ObjectTable() { - this(false); - } - - ObjectTable(boolean concurrent) { - this.concurrent = concurrent; - } - - /** - * The id obj is registered under, from any thread, without the isolate lock; NO_ID when - * the table isn't concurrent. A hit was correct at the time of the read; the object may - * have been released since, which the caller must check under the isolate lock. - */ - int findIdOptimistic(Object obj) { - if (!concurrent) { - return NO_ID; - } - synchronized (this) { - return findIdLocked(obj); - } - } - private static Object deref(Object value) { return value instanceof WeakRef ? ((WeakRef) value).get() : value; } @@ -94,10 +69,6 @@ private static final class Chunk { /** Registers obj under id, held strongly, replacing whatever id held before. */ void putStrong(int id, Object obj) { - if (!concurrent) { - putStrongLocked(id, obj); - return; - } synchronized (this) { putStrongLocked(id, obj); } @@ -130,15 +101,10 @@ private void putStrongLocked(int id, Object obj, int hash) { identityCount++; } - boolean allowsConcurrentRegistration() { - return concurrent; - } - /** * The id obj is registered under, registering it strongly under a new id from ids when * it has none; FRESH is set in the result when this call registered it. The lookup and - * the insert happen under one lock, so concurrent calls agree on one id. Only when - * allowsConcurrentRegistration(). + * the insert happen under one lock, so concurrent calls agree on one id. */ long findOrRegister(Object obj, IdSource ids) { synchronized (this) { @@ -153,11 +119,11 @@ long findOrRegister(Object obj, IdSource ids) { } } - /** The id obj is registered under, or NO_ID. */ + /** + * The id obj is registered under, or NO_ID. From a thread without the isolate lock, the + * object may have been released since, which the caller must check under the lock. + */ int findId(Object obj) { - if (!concurrent) { - return findIdLocked(obj); - } synchronized (this) { return findIdLocked(obj); } @@ -185,9 +151,6 @@ private int findIdLocked(Object obj, int hash) { /** The object registered under id; null if there is none or it was collected. */ Object get(int id) { - if (!concurrent) { - return getLocked(id); - } synchronized (this) { return getLocked(id); } @@ -199,9 +162,6 @@ private Object getLocked(int id) { } boolean contains(int id) { - if (!concurrent) { - return rawLocked(id) != null; - } synchronized (this) { return rawLocked(id) != null; } @@ -212,9 +172,6 @@ boolean contains(int id) { * the object is still alive; an entry whose object was already collected is removed. */ boolean weakenAndCheckAlive(int id) { - if (!concurrent) { - return weakenAndCheckAliveLocked(id); - } synchronized (this) { return weakenAndCheckAliveLocked(id); } @@ -228,10 +185,6 @@ void weakenAndCheckAlive(int[] ids, int count) { if (count == 0) { return; } - if (!concurrent) { - weakenAndCheckAliveAllLocked(ids, count); - return; - } synchronized (this) { weakenAndCheckAliveAllLocked(ids, count); } @@ -263,10 +216,6 @@ private boolean weakenAndCheckAliveLocked(int id) { } void remove(int id) { - if (!concurrent) { - removeLocked(id); - return; - } synchronized (this) { removeLocked(id); } diff --git a/test-app/runtime/src/main/java/com/tns/Runtime.java b/test-app/runtime/src/main/java/com/tns/Runtime.java index fd2e43ea5..75ec2758c 100644 --- a/test-app/runtime/src/main/java/com/tns/Runtime.java +++ b/test-app/runtime/src/main/java/com/tns/Runtime.java @@ -181,8 +181,8 @@ public void run() { "Primitive types need to be manually wrapped in their respective Object wrappers.\n" + "If you are creating an instance of an inner class, make sure to always provide reference to the outer `this` as the first argument."; - // On API 24+ any thread may read it and register new objects through findOrRegister - // (see ObjectTable); every other mutation comes from the thread holding this runtime's + // Any thread may read it and register new objects through findOrRegister (see + // ObjectTable); every other mutation comes from the thread holding this runtime's // isolate lock. private final ObjectTable objectTable; @@ -334,7 +334,7 @@ public Runtime(ClassResolver classResolver, GcListener gcListener, StaticConfigu } public Runtime(StaticConfiguration config, DynamicConfiguration dynamicConfiguration) { - this.objectTable = new ObjectTable(android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.N); + this.objectTable = new ObjectTable(); synchronized (Runtime.currentRuntime) { ManualInstrumentation.Frame frame = ManualInstrumentation.start("new Runtime"); try { @@ -1186,19 +1186,9 @@ private int getOrCreateJavaObjectID(Object obj) { // obj's id (low 32 bits), registering it strongly under a new id if it has none, with // ObjectTable.FRESH set then. Threads registering call arguments (packageArgs) do not - // hold the isolate lock, so where they may, the lookup and insert are one atomic step. + // hold the isolate lock, so the lookup and insert are one atomic step. private long getOrCreateObjectId(Object obj) { - // Exact: only the isolate-lock holder (this thread) removes entries. - int id = objectTable.findId(obj); - if (id != ObjectTable.NO_ID) { - return id & 0xFFFFFFFFL; - } - if (objectTable.allowsConcurrentRegistration()) { - return objectTable.findOrRegister(obj, objectIds); - } - id = generateNewObjectId(getRuntimeId()); - makeInstanceStrong(obj, id); - return (id & 0xFFFFFFFFL) | ObjectTable.FRESH; + return objectTable.findOrRegister(obj, objectIds); } public static Object callJSMethodFromPossibleNonMainThread(Object javaObject, String methodName, Class retType, Object... args) throws NativeScriptException { @@ -1383,11 +1373,8 @@ private PackagedArgs packageArgs(Object... args) { id = bound.getObjectId__ns(); } } - if (id == ObjectTable.NO_ID) { - id = objectTable.findIdOptimistic(value); - } long fresh = 0; - if (id == ObjectTable.NO_ID && objectTable.allowsConcurrentRegistration()) { + if (id == ObjectTable.NO_ID) { // Registered here rather than under the isolate lock, so threads calling // in with new objects only serialize on creating their wrappers. long found = objectTable.findOrRegister(value, objectIds); @@ -1396,8 +1383,6 @@ private PackagedArgs packageArgs(Object... args) { fresh = FRESH_ARG; } } - // Otherwise the id is a hint, resolved or registered natively under the - // isolate lock. values[i] = packIdAndClass(id, value) | fresh; } @@ -1513,7 +1498,7 @@ private Object dispatchCallJSMethodNative(final Object javaObject, int objectIdH // A hint: other threads may be writing the table, so native re-checks it under the // isolate lock and resolves the id itself on a miss. ret = callJSMethodNative(getRuntimeId(), javaObject, - objectIdHint != ObjectTable.NO_ID ? objectIdHint : objectTable.findIdOptimistic(javaObject), methodName, MethodNameIds.idOf(methodName), returnType, isConstructor, packagedArgs.count, packagedArgs.values, packagedArgs.refs, primitiveResult); + objectIdHint != ObjectTable.NO_ID ? objectIdHint : objectTable.findId(javaObject), methodName, MethodNameIds.idOf(methodName), returnType, isConstructor, packagedArgs.count, packagedArgs.values, packagedArgs.refs, primitiveResult); ret = takePrimitiveResult(ret, returnType, primitiveResult); } catch (NativeScriptException e) { // With multithreaded JS the owner can be torn down between the diff --git a/test-app/runtime/src/test/java/com/tns/ObjectTableTest.java b/test-app/runtime/src/test/java/com/tns/ObjectTableTest.java index 71d080b25..e39fa3740 100644 --- a/test-app/runtime/src/test/java/com/tns/ObjectTableTest.java +++ b/test-app/runtime/src/test/java/com/tns/ObjectTableTest.java @@ -79,7 +79,7 @@ public void aRegisteredWeakReferenceIsNotMistakenForAWeakEntry() { @Test public void concurrentReadsFindStableEntriesWhileAnotherThreadWrites() throws Exception { - final ObjectTable table = new ObjectTable(true); + final ObjectTable table = new ObjectTable(); final int stableCount = 64; final Object[] stable = new Object[stableCount]; for (int i = 0; i < stableCount; i++) { @@ -124,7 +124,7 @@ public void run() { Random random = new Random(seed); while (!done.get()) { int expected = random.nextInt(stableCount); - int id = table.findIdOptimistic(stable[expected]); + int id = table.findId(stable[expected]); if (id == expected) { hits.incrementAndGet(); } else { @@ -152,7 +152,7 @@ public void run() { @Test public void concurrentRegistrationsAgreeOnOneIdPerObject() throws Exception { - final ObjectTable table = new ObjectTable(true); + final ObjectTable table = new ObjectTable(); final java.util.concurrent.atomic.AtomicInteger nextId = new java.util.concurrent.atomic.AtomicInteger(); final ObjectTable.IdSource ids = new ObjectTable.IdSource() { @Override @@ -215,7 +215,7 @@ public void run() { @Test public void exactReadsStayExactWhileAnotherThreadRegisters() throws Exception { - final ObjectTable table = new ObjectTable(true); + final ObjectTable table = new ObjectTable(); final java.util.concurrent.atomic.AtomicInteger nextId = new java.util.concurrent.atomic.AtomicInteger(1000); final ObjectTable.IdSource ids = new ObjectTable.IdSource() { @Override @@ -309,7 +309,7 @@ public void keepsSurvivorsOfReleasedRangesAndOldIdsRegisteredLate() { @Test public void idsPastIntMaxAreUnsigned() { - ObjectTable table = new ObjectTable(true); + ObjectTable table = new ObjectTable(); Object a = new Object(); Object b = new Object(); table.putStrong(Integer.MIN_VALUE + 3, a); @@ -335,14 +335,6 @@ public int newId() { assertEquals(1, table.size()); } - @Test - public void optimisticReadsMissWhenDisabled() { - ObjectTable table = new ObjectTable(false); - Object a = new Object(); - table.putStrong(1, a); - assertEquals(ObjectTable.NO_ID, table.findIdOptimistic(a)); - } - @Test public void matchesReferenceMapsUnderRandomOperations() { Random random = new Random(42);