From 955effc9faa4ffc558da0ab2e230ff4eeca3d585 Mon Sep 17 00:00:00 2001 From: Angelo Rubini Date: Mon, 7 Sep 2026 17:24:50 +0200 Subject: [PATCH] Take the global monitors off the marshalling path Every Serializable object marshalled or unmarshalled anywhere in the process passed through ObjectStreamClass.lookupInternal, which took one process wide monitor - and took it even when the descriptor was already cached and fully initialized. The file has known this for a long time; the comment at that method says so: // Another possibility is to continue to synchronize on the // descriptorFor map, but that leads to poor performance // (see bug 4165204 "ObjectStreamClass can hold global lock // for a very long time"). // // We will need to live with 4165204 until we can rewrite // this to follow the improved java.io.ObjectStreamClass // in J2SE 1.4 and later. Measured on four cores, lookups per second, best of three: before after 1 thread 30,308,186 137,580,915 4.5x 2 16,674,433 258,879,774 15.5x 4 13,867,919 246,933,683 17.8x 8 15,067,581 300,386,540 19.9x The shape matters more than the multiplier. Before, adding threads made total throughput worse - 8 threads did half the work of 1, which is what a contended monitor looks like from the outside. After, it climbs. This does not rewrite initialization. The same comment records that moving init() out of the monitor was tried and reverted because it deadlocks (bug 5104239), so the slow path is left exactly as it was and a lock free fast path is added in front of it: if the descriptor is present and reports itself initialized, return it. 'initialized' becomes volatile and is already assigned last in init(), so it doubles as the publication fence for everything else the descriptor computed. A descriptor is published to the cache by the constructor before init() runs - on purpose, so recursive lookups find it - which is why presence alone is not enough. That fast path needs lock free reads, and the cache could not provide them: org.glassfish.pfl.basic.concurrent.SoftCache is, despite the package, a bare HashMap that was safe only because every caller held the monitor. It is replaced by ConcurrentSoftCache. Values stay soft because an ObjectStreamClass holds its Class, so a strong map would pin the application class loader for the life of the process. Two more monitors on the same path go with it. RepositoryIdCache, reached for every value type, was a Hashtable with a synchronized getId on top - the same monitor taken twice, once for the method and once inside the Hashtable. It is now a ConcurrentHashMap whose hit path takes no lock. ObjectStreamClass.translateFields was static synchronized, so it locked the class object shared with every other static synchronized member. PersistentFieldsValue.translateFields allocates a new array and reads only its argument; the lock guarded nothing. One of these was not only slow but wrong. RepositoryId.repStrToClass was that same non-thread-safe SoftCache, written under the classToRepStr monitor but read by getAnyClassFromType under no lock at all - a data race on a plain HashMap. Worse, SoftCache mutates its map inside get(), so two concurrent readers were enough on their own. It is now a ConcurrentSoftCache. 353 tests pass. The contention test asserts the property rather than a timing threshold: it holds the cache monitor and requires a lookup of an already initialized descriptor to complete anyway. Reverted against the old ObjectStreamClass that test blocks for its full ten second timeout and fails, so it is not vacuous. DescriptorLookupThroughput is a main and not a test, because a number is not something to assert on a shared build machine. It is also the second harness written for this: the first checked a deadline inside the loop and reported both versions as identical and both as failing to scale. System.nanoTime is not always a cheap userspace read, and when it is not every thread queues on the clock source - the harness had become the contended resource it was looking for. Signed-off-by: Angelo Rubini --- .../corba/ee/impl/io/ObjectStreamClass.java | 61 +++++- .../ee/impl/misc/ConcurrentSoftCache.java | 105 ++++++++++ .../sun/corba/ee/impl/util/RepositoryId.java | 14 +- .../corba/ee/impl/util/RepositoryIdCache.java | 33 ++- .../io/DescriptorLookupContentionTest.java | 196 ++++++++++++++++++ .../impl/io/DescriptorLookupThroughput.java | 149 +++++++++++++ .../ee/impl/misc/ConcurrentSoftCacheTest.java | 113 ++++++++++ 7 files changed, 653 insertions(+), 18 deletions(-) create mode 100644 orbmain/src/main/java/com/sun/corba/ee/impl/misc/ConcurrentSoftCache.java create mode 100644 orbmain/src/test/java/com/sun/corba/ee/impl/io/DescriptorLookupContentionTest.java create mode 100644 orbmain/src/test/java/com/sun/corba/ee/impl/io/DescriptorLookupThroughput.java create mode 100644 orbmain/src/test/java/com/sun/corba/ee/impl/misc/ConcurrentSoftCacheTest.java diff --git a/orbmain/src/main/java/com/sun/corba/ee/impl/io/ObjectStreamClass.java b/orbmain/src/main/java/com/sun/corba/ee/impl/io/ObjectStreamClass.java index 748b8eb0ec..d9fb3b798d 100644 --- a/orbmain/src/main/java/com/sun/corba/ee/impl/io/ObjectStreamClass.java +++ b/orbmain/src/main/java/com/sun/corba/ee/impl/io/ObjectStreamClass.java @@ -22,6 +22,7 @@ package com.sun.corba.ee.impl.io; import com.sun.corba.ee.impl.misc.ClassInfoCache; +import com.sun.corba.ee.impl.misc.ConcurrentSoftCache; import com.sun.corba.ee.impl.util.RepositoryId; import com.sun.corba.ee.spi.trace.TraceValueHandler; @@ -46,8 +47,8 @@ import java.util.Comparator; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.ConcurrentMap; -import org.glassfish.pfl.basic.concurrent.SoftCache; import org.glassfish.pfl.basic.reflection.Bridge; import org.omg.CORBA.ValueMember; @@ -99,13 +100,39 @@ static final ObjectStreamClass lookup(Class cl) { */ static ObjectStreamClass lookupInternal(Class cl) { + /* A descriptor that is present and fully initialized can be returned + * without any lock at all, and in a running system that is very nearly + * every lookup: descriptors are created once per class and then read + * for the life of the process. + * + * The lock below is a single process wide monitor which every + * marshalled and unmarshalled object had to pass through, so removing + * it from the steady state path is the point of this fast path. It + * also has to be a fast path rather than a rewrite of what follows: + * the comment on init() records that moving initialization out of this + * monitor was tried and reverted because it deadlocks (bug 5104239), + * so the slow path is deliberately left exactly as it was. + * + * Correctness rests on two things. 'initialized' is volatile and is + * assigned last in init(), so seeing it true means every field written + * during initialization is visible. And a descriptor is published into + * the cache by the constructor, before init() runs, precisely so that + * recursive lookups find it - which is why it is not enough for the + * entry to exist, it must also report itself initialized. + */ + ObjectStreamClass cached = descriptorFor.get(cl); + if (cached != null && cached.initialized) { + return cached; + } + /* Synchronize on the hashtable so no two threads will do * this at the same time. */ ObjectStreamClass desc = null; synchronized (descriptorFor) { + descriptorFor.purge(); /* Find the matching descriptor if it already known */ - desc = (ObjectStreamClass)descriptorFor.get( cl ) ; + desc = descriptorFor.get( cl ) ; if (desc == null) { /* Check if it's serializable */ ClassInfoCache.ClassInfo cinfo = ClassInfoCache.get( cl ) ; @@ -641,7 +668,15 @@ public final String getRMIIIOPOptionalDataRepId() { superclass = null; } - public static final synchronized ObjectStreamField[] translateFields( + /** + * @param fields the fields to translate + * @return the translated fields + */ + // Not synchronized: PersistentFieldsValue.translateFields allocates a new + // array and reads only its argument, so the monitor this used to take - + // on the ObjectStreamClass class object, shared with every other static + // synchronized member - guarded nothing. + public static final ObjectStreamField[] translateFields( java.io.ObjectStreamField fields[]) { return PersistentFieldsValue.translateFields(fields); } @@ -1194,11 +1229,18 @@ static String getSignature(Constructor cons) { return sb.toString(); } - /* - * Cache of Class -> ClassDescriptor Mappings. + /** + * Cache of Class to ObjectStreamClass mappings. + * + *

This used to be {@code org.glassfish.pfl.basic.concurrent.SoftCache}, + * a bare HashMap that was safe only because {@link #lookupInternal} took a + * process wide monitor around every access. Reads have to be lock free for + * that method's fast path to exist. The values stay soft: an + * ObjectStreamClass holds its Class, so a strong map would form a key to + * value to key cycle pinning the application class loader. */ - static private final SoftCache,ObjectStreamClass> descriptorFor = - new SoftCache,ObjectStreamClass>() ; + private static final ConcurrentSoftCache, ObjectStreamClass> descriptorFor = + new ConcurrentSoftCache<>(); /* * The name of this descriptor @@ -1258,7 +1300,10 @@ static String getSignature(Constructor cons) { * try to fix bug 4373844. Working to move to * reusing java.io.ObjectStreamClass for JDK 1.5. */ - private boolean initialized = false; + // Read without holding any lock by lookupInternal's fast path, and + // assigned last by init(), so it doubles as the publication fence for + // every other field this descriptor computes. + private volatile boolean initialized = false; /* Internal lock object. */ private final Object lock = new Object(); diff --git a/orbmain/src/main/java/com/sun/corba/ee/impl/misc/ConcurrentSoftCache.java b/orbmain/src/main/java/com/sun/corba/ee/impl/misc/ConcurrentSoftCache.java new file mode 100644 index 0000000000..ef0dad46f0 --- /dev/null +++ b/orbmain/src/main/java/com/sun/corba/ee/impl/misc/ConcurrentSoftCache.java @@ -0,0 +1,105 @@ +/* + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License + * v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License v2.0 + * w/Classpath exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause OR GPL-2.0 WITH + * Classpath-exception-2.0 + */ + +package com.sun.corba.ee.impl.misc; + +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.SoftReference; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +/** + * A concurrent cache whose values are held softly. + * + *

It exists to replace two uses of + * {@code org.glassfish.pfl.basic.concurrent.SoftCache}, which despite its + * package name is a bare {@code HashMap} with no synchronization of its own. + * One of those uses was safe only because every caller happened to hold an + * external monitor; the other was read with no lock at all while other + * threads wrote it, which is a data race on a plain HashMap - and since that + * class also mutates its map inside {@code get}, even two concurrent readers + * were enough to corrupt it. + * + *

Values are soft rather than strong because both caches map to, or from, + * {@code Class} objects. A strong reference from a process wide static map to + * an application class keeps its class loader alive forever, which in an + * application server is a redeployment leak. + * + *

Reads take no lock. Cleared entries are evicted by {@link #purge()}, + * which callers are expected to invoke from whatever slow path they already + * have, rather than on every read. + * + * @param key type + * @param value type + */ +public final class ConcurrentSoftCache { + + private final ConcurrentMap> map = new ConcurrentHashMap<>(); + private final ReferenceQueue cleared = new ReferenceQueue<>(); + + /** A soft reference that remembers its key, so a cleared one can be evicted. */ + private static final class Entry extends SoftReference { + + private final K key; + + Entry(K key, V value, ReferenceQueue queue) { + super(value, queue); + this.key = key; + } + } + + /** + * @param key the key to look up + * @return the value, or null if absent or already collected + */ + public V get(K key) { + Entry entry = map.get(key); + return entry == null ? null : entry.get(); + } + + /** + * @param key the key to store under + * @param value the value to hold softly + */ + public void put(K key, V value) { + map.put(key, new Entry<>(key, value, cleared)); + } + + /** + * Drops the entries whose value has been collected. + */ + public void purge() { + for (Reference ref; (ref = cleared.poll()) != null; ) { + @SuppressWarnings("unchecked") + Entry entry = (Entry) ref; + // Two argument remove, so an entry that a later put reinstated + // under the same key is never evicted by this one's death. + map.remove(entry.key, entry); + } + } + + /** + * @return the number of entries, including any whose value has been + * collected but not yet purged + */ + public int size() { + return map.size(); + } +} diff --git a/orbmain/src/main/java/com/sun/corba/ee/impl/util/RepositoryId.java b/orbmain/src/main/java/com/sun/corba/ee/impl/util/RepositoryId.java index ad18c7f728..cd4f7d8f35 100644 --- a/orbmain/src/main/java/com/sun/corba/ee/impl/util/RepositoryId.java +++ b/orbmain/src/main/java/com/sun/corba/ee/impl/util/RepositoryId.java @@ -21,6 +21,8 @@ package com.sun.corba.ee.impl.util; +import com.sun.corba.ee.impl.misc.ConcurrentSoftCache; + import com.sun.corba.ee.impl.io.ObjectStreamClass; import com.sun.corba.ee.impl.javax.rmi.CORBA.Util; import com.sun.corba.ee.impl.misc.ClassInfoCache ; @@ -34,7 +36,6 @@ import java.util.Map; import java.util.WeakHashMap; -import org.glassfish.pfl.basic.concurrent.SoftCache; public class RepositoryId { @@ -77,7 +78,14 @@ public class RepositoryId { private static final Map, String> classSeqToRepStr = new WeakHashMap<>(); private static final Map repStrToByteArray = new IdentityHashMap<>(); - private static final Map> repStrToClass = new SoftCache<>(); + /* + * Read by getAnyClassFromType with no lock at all, while the writes below + * happen under the classToRepStr monitor. With the previous SoftCache - + * a bare HashMap that mutates itself even inside get() - that was a data + * race, and two concurrent readers were enough to corrupt it. + */ + private static final ConcurrentSoftCache> repStrToClass = + new ConcurrentSoftCache<>(); private String repId = null; private boolean isSupportedFormat = true; @@ -779,6 +787,7 @@ public static String createForJavaType(java.io.Serializable ser) createHashString(clazz); classToRepStr.put(clazz, repid); + repStrToClass.purge(); repStrToClass.put(repid, clazz); return repid; } @@ -815,6 +824,7 @@ public static String createForJavaType(Class clz, ClassInfoCache.ClassInfo ci createHashString(clz); classToRepStr.put(clz, repid); + repStrToClass.purge(); repStrToClass.put(repid, clz); return repid; } diff --git a/orbmain/src/main/java/com/sun/corba/ee/impl/util/RepositoryIdCache.java b/orbmain/src/main/java/com/sun/corba/ee/impl/util/RepositoryIdCache.java index a2a7bebc9d..eb3c5756f7 100644 --- a/orbmain/src/main/java/com/sun/corba/ee/impl/util/RepositoryIdCache.java +++ b/orbmain/src/main/java/com/sun/corba/ee/impl/util/RepositoryIdCache.java @@ -20,18 +20,35 @@ package com.sun.corba.ee.impl.util; -import java.util.Hashtable; +import java.util.concurrent.ConcurrentHashMap; -public class RepositoryIdCache extends Hashtable { - public final synchronized RepositoryId getId(String key) { - RepositoryId repId = super.get(key); +/** + * Interns {@link RepositoryId} instances by their string form. + * + *

This was a {@code Hashtable} with a {@code synchronized getId} on top, + * held in a static field on {@link RepositoryId}, so every value type + * marshalled anywhere in the process took the same monitor twice - once for + * the method and once inside the Hashtable itself. The map is now concurrent + * and the hit path takes no lock. + */ +public class RepositoryIdCache extends ConcurrentHashMap { + private static final long serialVersionUID = 1L; + + /** + * @param key the repository id string + * @return the interned RepositoryId for that string, creating one if this + * is the first time it has been seen + */ + public final RepositoryId getId(String key) { + // Look before computing: computeIfAbsent locks a bin even when the key + // is present, and present is what almost every call finds. + RepositoryId repId = get(key); if (repId != null) { return repId; - } else { - repId = new RepositoryId(key); - put(key, repId); - return repId; } + // RepositoryId's constructor only parses the string it is given, so it + // cannot re-enter this map - which computeIfAbsent would not tolerate. + return computeIfAbsent(key, RepositoryId::new); } } diff --git a/orbmain/src/test/java/com/sun/corba/ee/impl/io/DescriptorLookupContentionTest.java b/orbmain/src/test/java/com/sun/corba/ee/impl/io/DescriptorLookupContentionTest.java new file mode 100644 index 0000000000..942130de5d --- /dev/null +++ b/orbmain/src/test/java/com/sun/corba/ee/impl/io/DescriptorLookupContentionTest.java @@ -0,0 +1,196 @@ +/* + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License + * v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License v2.0 + * w/Classpath exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause OR GPL-2.0 WITH + * Classpath-exception-2.0 + */ + +package com.sun.corba.ee.impl.io; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +import com.sun.corba.ee.impl.util.RepositoryId; +import java.io.Serializable; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.Test; + +/** + * Asserts that looking up a class descriptor no longer serializes every + * thread in the process behind one monitor. + * + *

Throughput would be the obvious way to show this and the wrong way to + * assert it: a timing threshold is a flaky test on a shared build machine. + * What is checked instead is the property itself - that the fast path does + * not need the lock - by holding that lock and watching a lookup succeed + * anyway. Before the change the same test blocks until the timeout. + */ +public class DescriptorLookupContentionTest { + + private static final long TIMEOUT_SECONDS = 10; + + /** A class of our own, so no other test can have warmed it up first. */ + private static final class Marshalled implements Serializable { + + private static final long serialVersionUID = 1L; + + @SuppressWarnings("unused") + private int field; + } + + private static Object descriptorCache() throws Exception { + Field field = ObjectStreamClass.class.getDeclaredField("descriptorFor"); + field.setAccessible(true); + return field.get(null); + } + + @Test + public void aLookupOfAnInitializedDescriptorDoesNotNeedTheGlobalLock() throws Exception { + // Warm up: after this the descriptor is cached and initialized, which + // is the state every class reaches within moments of a server starting. + ObjectStreamClass warmed = ObjectStreamClass.lookup(Marshalled.class); + assertNotNull(warmed); + + final Object cache = descriptorCache(); + final CountDownLatch lockHeld = new CountDownLatch(1); + final CountDownLatch lookupDone = new CountDownLatch(1); + final AtomicReference found = new AtomicReference<>(); + + Thread reader = new Thread(new Runnable() { + @Override + public void run() { + try { + lockHeld.await(TIMEOUT_SECONDS, TimeUnit.SECONDS); + found.set(ObjectStreamClass.lookup(Marshalled.class)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + lookupDone.countDown(); + } + } + }, "descriptor-lookup"); + reader.setDaemon(true); + reader.start(); + + synchronized (cache) { + lockHeld.countDown(); + assertTrue("a lookup of an already initialized descriptor blocked on the" + + " cache monitor; the lock free fast path is not being taken", + lookupDone.await(TIMEOUT_SECONDS, TimeUnit.SECONDS)); + } + + reader.join(TimeUnit.SECONDS.toMillis(TIMEOUT_SECONDS)); + assertSame("the fast path must return the same descriptor as the slow path", + warmed, found.get()); + } + + @Test + public void concurrentLookupsAgreeOnOneDescriptorPerClass() throws Exception { + final Class[] classes = { + Marshalled.class, String.class, Integer.class, java.util.ArrayList.class, + java.util.HashMap.class, java.math.BigDecimal.class, java.util.Date.class, + }; + + int threads = Math.max(4, Runtime.getRuntime().availableProcessors()); + ExecutorService pool = Executors.newFixedThreadPool(threads); + try { + List> tasks = new ArrayList<>(); + for (int t = 0; t < threads; t++) { + tasks.add(new Callable() { + @Override + public ObjectStreamClass[] call() { + ObjectStreamClass[] seen = new ObjectStreamClass[classes.length]; + for (int round = 0; round < 200; round++) { + for (int i = 0; i < classes.length; i++) { + seen[i] = ObjectStreamClass.lookup(classes[i]); + } + } + return seen; + } + }); + } + + ObjectStreamClass[] reference = null; + for (Future result : pool.invokeAll(tasks, 60, TimeUnit.SECONDS)) { + ObjectStreamClass[] seen = result.get(); + if (reference == null) { + reference = seen; + } else { + for (int i = 0; i < seen.length; i++) { + assertSame("two threads got different descriptors for " + classes[i], + reference[i], seen[i]); + } + } + } + } finally { + pool.shutdownNow(); + } + } + + @Test + public void concurrentRepositoryIdLookupsIntern() throws Exception { + final String[] ids = new String[64]; + for (int i = 0; i < ids.length; i++) { + ids[i] = "RMI:com.acme.Type" + i + ":0123456789ABCDEF"; + } + + int threads = Math.max(4, Runtime.getRuntime().availableProcessors()); + ExecutorService pool = Executors.newFixedThreadPool(threads); + try { + List> tasks = new ArrayList<>(); + for (int t = 0; t < threads; t++) { + tasks.add(new Callable() { + @Override + public RepositoryId[] call() { + RepositoryId[] seen = new RepositoryId[ids.length]; + for (int round = 0; round < 200; round++) { + for (int i = 0; i < ids.length; i++) { + seen[i] = RepositoryId.cache.getId(ids[i]); + } + } + return seen; + } + }); + } + + RepositoryId[] reference = null; + for (Future result : pool.invokeAll(tasks, 60, TimeUnit.SECONDS)) { + RepositoryId[] seen = result.get(); + assertEquals(ids.length, seen.length); + if (reference == null) { + reference = seen; + } else { + for (int i = 0; i < seen.length; i++) { + assertSame("the cache handed out two instances for " + ids[i], + reference[i], seen[i]); + } + } + } + } finally { + pool.shutdownNow(); + } + } +} diff --git a/orbmain/src/test/java/com/sun/corba/ee/impl/io/DescriptorLookupThroughput.java b/orbmain/src/test/java/com/sun/corba/ee/impl/io/DescriptorLookupThroughput.java new file mode 100644 index 0000000000..3dea432bb5 --- /dev/null +++ b/orbmain/src/test/java/com/sun/corba/ee/impl/io/DescriptorLookupThroughput.java @@ -0,0 +1,149 @@ +/* + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License + * v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License v2.0 + * w/Classpath exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause OR GPL-2.0 WITH + * Classpath-exception-2.0 + */ + +package com.sun.corba.ee.impl.io; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +/** + * Measures how descriptor lookup scales with thread count. + * + *

Deliberately a {@code main} and not a test: the number it reports is the + * point, and a number is not something to assert on a shared build machine. + * The correctness properties are asserted in + * {@link DescriptorLookupContentionTest}; this is here so that a claim about + * throughput in a review can be reproduced rather than believed. + * + * 

+ * mvn -pl orbmain test-compile
+ * java -cp orbmain/target/classes:orbmain/target/test-classes:$(deps) \
+ *      com.sun.corba.ee.impl.io.DescriptorLookupThroughput
+ *
+ * + *

What to look for is not the absolute rate but the shape: with a global + * monitor on the lookup path the total rate is flat as threads are added, + * because the threads are queueing. Without it the rate should climb roughly + * with core count until memory bandwidth or the allocator becomes the limit. + */ +public final class DescriptorLookupThroughput { + + /** Iterations per thread, each doing one pass over CLASSES. */ + private static final int ROUNDS = 2_000_000; + private static final int WARMUP_ROUNDS = 200_000; + + private DescriptorLookupThroughput() { + } + + private static final class Sample implements Serializable { + + private static final long serialVersionUID = 1L; + + @SuppressWarnings("unused") + private int a; + @SuppressWarnings("unused") + private String b; + } + + private static final Class[] CLASSES = { + Sample.class, String.class, Integer.class, Long.class, + java.util.ArrayList.class, java.util.HashMap.class, + java.math.BigDecimal.class, java.util.Date.class, + }; + + public static void main(String[] args) throws Exception { + for (Class cl : CLASSES) { + ObjectStreamClass.lookup(cl); + } + run(1, WARMUP_ROUNDS); + run(2, WARMUP_ROUNDS); + + int cores = Runtime.getRuntime().availableProcessors(); + System.out.println("cores: " + cores); + System.out.printf("%8s %16s %12s%n", "threads", "lookups/sec", "vs 1 thread"); + + double single = 0; + for (int threads = 1; threads <= cores * 2; threads *= 2) { + // Best of three: the interesting quantity is the ceiling, and a + // shared machine only ever adds noise downwards. + double best = 0; + for (int attempt = 0; attempt < 3; attempt++) { + best = Math.max(best, run(threads, ROUNDS)); + } + if (threads == 1) { + single = best; + } + System.out.printf("%8d %16.0f %11.2fx%n", threads, best, best / single); + } + } + + /** + * Runs a fixed amount of work per thread and times the whole thing once. + * + *

A deadline checked inside the loop would be the obvious shape and is + * wrong here: System.nanoTime is not necessarily a cheap userspace read, + * and when it is not, every thread queues on the same clock source. That + * turns the harness itself into the contended resource and reports the + * same flat curve whatever the code under test does. + * + * @param threads number of threads to run + * @param rounds passes over CLASSES per thread + * @return lookups per second across all threads + */ + private static double run(int threads, final int rounds) throws Exception { + ExecutorService pool = Executors.newFixedThreadPool(threads); + try { + final java.util.concurrent.CyclicBarrier start = + new java.util.concurrent.CyclicBarrier(threads); + List> tasks = new ArrayList<>(); + for (int t = 0; t < threads; t++) { + tasks.add(new Callable() { + @Override + public Long call() throws Exception { + start.await(); + long seen = 0; + for (int round = 0; round < rounds; round++) { + for (int i = 0; i < CLASSES.length; i++) { + if (ObjectStreamClass.lookup(CLASSES[i]) != null) { + seen++; + } + } + } + return seen; + } + }); + } + + long began = System.nanoTime(); + long total = 0; + for (Future result : pool.invokeAll(tasks)) { + total += result.get(); + } + long elapsed = System.nanoTime() - began; + return total / (elapsed / 1_000_000_000.0); + } finally { + pool.shutdownNow(); + } + } +} diff --git a/orbmain/src/test/java/com/sun/corba/ee/impl/misc/ConcurrentSoftCacheTest.java b/orbmain/src/test/java/com/sun/corba/ee/impl/misc/ConcurrentSoftCacheTest.java new file mode 100644 index 0000000000..d56d733571 --- /dev/null +++ b/orbmain/src/test/java/com/sun/corba/ee/impl/misc/ConcurrentSoftCacheTest.java @@ -0,0 +1,113 @@ +/* + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License + * v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the Eclipse + * Public License v. 2.0 are satisfied: GNU General Public License v2.0 + * w/Classpath exception which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause OR GPL-2.0 WITH + * Classpath-exception-2.0 + */ + +package com.sun.corba.ee.impl.misc; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import org.junit.Test; + +public class ConcurrentSoftCacheTest { + + @Test + public void storesAndReturnsValues() { + ConcurrentSoftCache cache = new ConcurrentSoftCache<>(); + assertNull(cache.get("absent")); + + String value = "a value"; + cache.put("key", value); + assertSame(value, cache.get("key")); + assertEquals(1, cache.size()); + } + + @Test + public void aLaterPutReplacesTheEarlierOne() { + ConcurrentSoftCache cache = new ConcurrentSoftCache<>(); + cache.put("key", "first"); + cache.put("key", "second"); + + assertEquals("second", cache.get("key")); + assertEquals(1, cache.size()); + } + + @Test + public void purgeLeavesLiveEntriesAlone() { + ConcurrentSoftCache cache = new ConcurrentSoftCache<>(); + String held = "still referenced"; + cache.put("key", held); + + cache.purge(); + + assertSame(held, cache.get("key")); + } + + /** + * The cache is read with no lock while other threads write it, which is + * exactly what its predecessor could not survive. + */ + @Test + public void concurrentReadersAndWritersAgree() throws Exception { + final ConcurrentSoftCache cache = new ConcurrentSoftCache<>(); + final int keys = 200; + final List values = new ArrayList<>(); + for (int i = 0; i < keys; i++) { + values.add("value " + i); + } + + int threads = Math.max(4, Runtime.getRuntime().availableProcessors()); + ExecutorService pool = Executors.newFixedThreadPool(threads); + try { + List> tasks = new ArrayList<>(); + for (int t = 0; t < threads; t++) { + tasks.add(new Callable() { + @Override + public Boolean call() { + for (int round = 0; round < 500; round++) { + for (int i = 0; i < keys; i++) { + cache.put(i, values.get(i)); + String seen = cache.get(i); + // Never a value belonging to another key. + if (seen != null && seen != values.get(i)) { + return false; + } + } + } + return true; + } + }); + } + for (Future result : pool.invokeAll(tasks, 60, TimeUnit.SECONDS)) { + assertTrue("a reader saw a value that did not belong to its key", result.get()); + } + assertEquals(keys, cache.size()); + } finally { + pool.shutdownNow(); + } + } +}