From 010cc02c77c06a91b48cbf11c6e1e2e100cfb728 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 2 Jul 2026 21:21:35 +0300 Subject: [PATCH 1/2] Remove per-attribute read lock from CompressedSchema decode path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CompressedSchema.decodeAttribute()/decodeObjectClasses() acquired a ReentrantReadWriteLock read lock for every attribute of every entry read from a backend — 20+ lock acquisitions per bind, hundreds of thousands of CAS operations per second on one contended cache line under load. The lock only guarded reads of the schema and mappings references: the Mappings internals are already concurrent collections. Make both references volatile and read them lock-free. On schema reload the new mappings reference is published before the schema reference, so a reader observing the current schema also observes the mappings rebuilt for it. Mutations (id registration, schema reload) stay serialized on an exclusive lock, with the mappings reference re-read under the lock since a reload may have replaced it. Interleaved A/B under the PR #660 bind benchmark scenario shows +1..+22% throughput (avg +11%) and consistently lower latency on an 8-core host. --- .../opends/server/api/CompressedSchema.java | 122 ++++++++---------- 1 file changed, 54 insertions(+), 68 deletions(-) diff --git a/opendj-server-legacy/src/main/java/org/opends/server/api/CompressedSchema.java b/opendj-server-legacy/src/main/java/org/opends/server/api/CompressedSchema.java index 6b61b3140b..da7b7bbde6 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/api/CompressedSchema.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/api/CompressedSchema.java @@ -13,6 +13,7 @@ * * Copyright 2009 Sun Microsystems, Inc. * Portions Copyright 2013-2016 ForgeRock AS. + * Portions Copyright 2024-2026 3A Systems, LLC */ package org.opends.server.api; @@ -30,11 +31,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; - -import net.jcip.annotations.GuardedBy; +import java.util.concurrent.locks.ReentrantLock; import org.forgerock.opendj.ldap.AttributeDescription; import org.forgerock.opendj.ldap.ByteSequenceReader; @@ -88,16 +85,19 @@ private Mappings(int adEncodeMapSize, int ocEncodeMapSize) } private final ServerContext serverContext; - /** Lock to update the maps. */ - private final ReadWriteLock lock = new ReentrantReadWriteLock(); - private final Lock exclusiveLock = lock.writeLock(); - private final Lock sharedLock = lock.readLock(); + /** Lock serializing all mutations (id registration and schema reload). */ + private final ReentrantLock exclusiveLock = new ReentrantLock(); - /** Schema used to build the compressed information. */ - @GuardedBy("lock") - private Schema schema; - @GuardedBy("lock") - private Mappings mappings = new Mappings(); + /** + * Readers are lock-free: the Mappings internals are concurrent collections, + * and both references are volatile. decodeAttribute() runs for every + * attribute of every entry read from a backend, so taking even a read lock + * here becomes a cross-core hotspot under load. On schema reload the new + * mappings reference is written before the schema reference, so a reader + * observing the current schema also observes the mappings rebuilt for it. + */ + private volatile Schema schema; + private volatile Mappings mappings = new Mappings(); /** * Creates a new empty instance of this compressed schema. @@ -112,47 +112,37 @@ public CompressedSchema(ServerContext serverContext) private Mappings getMappings() { - sharedLock.lock(); - try - { - return mappings; - } - finally - { - sharedLock.unlock(); - } + return mappings; } private Mappings reloadMappingsIfSchemaChanged() { - sharedLock.lock(); - boolean shared = true; + // Lock-free fast path: the schema reference must be read before the + // mappings reference, mirroring the publication order in the slow path. + if (schema == serverContext.getSchema()) + { + return mappings; + } + + exclusiveLock.lock(); try { Schema currentSchema = serverContext.getSchema(); if (schema != currentSchema) { - sharedLock.unlock(); - exclusiveLock.lock(); - shared = false; - - currentSchema = serverContext.getSchema(); - if (schema != currentSchema) - { - // build new maps from existing ones - Mappings newMappings = new Mappings(mappings.adEncodeMap.size(), mappings.ocEncodeMap.size()); - reloadAttributeTypeMaps(mappings, newMappings); - reloadObjectClassesMap(mappings, newMappings); + // build new maps from existing ones + Mappings newMappings = new Mappings(mappings.adEncodeMap.size(), mappings.ocEncodeMap.size()); + reloadAttributeTypeMaps(mappings, newMappings); + reloadObjectClassesMap(mappings, newMappings); - mappings = newMappings; - schema = currentSchema; - } + mappings = newMappings; + schema = currentSchema; } return mappings; } finally { - (shared ? sharedLock : exclusiveLock).unlock(); + exclusiveLock.unlock(); } } @@ -290,21 +280,19 @@ public final void encodeAttribute(final ByteStringBuilder builder, private int getAttributeId(final AttributeDescription ad) throws DirectoryException { - // avoid lazy registration races - boolean shared = true; - sharedLock.lock(); - try + // Lock-free fast path for already-registered attribute descriptions. + Integer id = mappings.adEncodeMap.get(ad); + if (id != null) { - Integer id = mappings.adEncodeMap.get(ad); - if (id != null) - { - return id; - } - - sharedLock.unlock(); - exclusiveLock.lock(); - shared = false; + return id; + } + // Take the exclusive lock to avoid lazy registration races, and re-read + // the mappings reference: a schema reload may have replaced it. + exclusiveLock.lock(); + try + { + final Mappings mappings = this.mappings; id = mappings.adEncodeMap.get(ad); if (id == null) { @@ -317,7 +305,7 @@ private int getAttributeId(final AttributeDescription ad) throws DirectoryExcept } finally { - (shared ? sharedLock : exclusiveLock).unlock(); + exclusiveLock.unlock(); } } @@ -349,21 +337,19 @@ public final void encodeObjectClasses(final ByteStringBuilder builder, private int getObjectClassId(final Map objectClasses) throws DirectoryException { - // avoid lazy registration races - boolean shared = true; - sharedLock.lock(); - try + // Lock-free fast path for already-registered object class sets. + Integer id = mappings.ocEncodeMap.get(objectClasses); + if (id != null) { - Integer id = mappings.ocEncodeMap.get(objectClasses); - if (id != null) - { - return id; - } - - sharedLock.unlock(); - exclusiveLock.lock(); - shared = false; + return id; + } + // Take the exclusive lock to avoid lazy registration races, and re-read + // the mappings reference: a schema reload may have replaced it. + exclusiveLock.lock(); + try + { + final Mappings mappings = this.mappings; id = mappings.ocEncodeMap.get(objectClasses); if (id == null) { @@ -376,7 +362,7 @@ private int getObjectClassId(final Map objectClasses) throw } finally { - (shared ? sharedLock : exclusiveLock).unlock(); + exclusiveLock.unlock(); } } From 7a290de418356b2f82eec353dd8b7e11c35e9784 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Sat, 4 Jul 2026 14:17:19 +0300 Subject: [PATCH 2/2] Reload compressed schema mappings from one stable local snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Capture the mappings reference once under the exclusive lock and reuse the local for the size calculation and both reload calls. Not required for correctness — mutations are serialized by the same lock — but it makes the reload-from-one-snapshot intent explicit, drops the repeated volatile reads and matches the pattern already used in getAttributeId() and getObjectClassId(). --- .../java/org/opends/server/api/CompressedSchema.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/opendj-server-legacy/src/main/java/org/opends/server/api/CompressedSchema.java b/opendj-server-legacy/src/main/java/org/opends/server/api/CompressedSchema.java index da7b7bbde6..74088abf4a 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/api/CompressedSchema.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/api/CompressedSchema.java @@ -130,10 +130,11 @@ private Mappings reloadMappingsIfSchemaChanged() Schema currentSchema = serverContext.getSchema(); if (schema != currentSchema) { - // build new maps from existing ones - Mappings newMappings = new Mappings(mappings.adEncodeMap.size(), mappings.ocEncodeMap.size()); - reloadAttributeTypeMaps(mappings, newMappings); - reloadObjectClassesMap(mappings, newMappings); + // build new maps from one stable snapshot of the existing ones + final Mappings oldMappings = mappings; + Mappings newMappings = new Mappings(oldMappings.adEncodeMap.size(), oldMappings.ocEncodeMap.size()); + reloadAttributeTypeMaps(oldMappings, newMappings); + reloadObjectClassesMap(oldMappings, newMappings); mappings = newMappings; schema = currentSchema;