Skip to content

Remove per-attribute read lock from CompressedSchema decode path - #670

Merged
vharseko merged 5 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/bind-6
Aug 6, 2026
Merged

Remove per-attribute read lock from CompressedSchema decode path#670
vharseko merged 5 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/bind-6

Conversation

@vharseko

@vharsekovharseko commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

Another BIND/read hot-path serialization point found while profiling after
#660: CompressedSchema.decodeAttribute() / decodeObjectClasses() acquire
a ReentrantReadWriteLockread lock for every attribute of every entry
read from a backend. Decoding one user entry takes K+1 lock acquisitions
(~20+ per bind), i.e. hundreds of thousands of CAS operations per second on
one contended cache line under load. This PR makes the read path lock-free.

Problem

Every decode calls reloadMappingsIfSchemaChanged(), which took the shared
lock just to compare the schema reference and read the mappings reference:

sharedLock.lock(); // K+1 times per entry decodetry {
if (schema != serverContext.getSchema()) { ... }
returnmappings;
} finally { sharedLock.unlock(); }

Read locks do not block each other, but every acquire/release is a CAS on
the RRWL sync word — a cross-core cache-coherency hotspot on the per-backend
singleton (PersistentCompressedSchema). The Mappings internals are
already concurrent collections (CopyOnWriteArrayList for the id→element
decode maps, ConcurrentHashMap for the encode maps), so the lock only
guarded reads of two references.

Change

  • schema and mappings become volatile and are read lock-free on the
    decode path. On schema reload the new mappings reference is published
    before the schema reference, so a reader that observes the current
    schema is guaranteed to observe the mappings rebuilt for it (ids are
    stable across rebuilds).
  • getAttributeId() / getObjectClassId() (encode path) get the same
    lock-free fast path via the encode ConcurrentHashMap; registration of a
    new id stays serialized on the exclusive lock, re-reading the mappings
    reference under the lock since a concurrent reload may have replaced it.
  • The ReentrantReadWriteLock is replaced by a single ReentrantLock used
    only by mutators (id registration, schema reload). Behaviour is otherwise
    unchanged.

Benchmark

Same harness as #660/#667/#669: packaged server, je backend, 5,000 users
({SSHA}), authrate with 200 persistent connections re-binding as random
users, JDK 11, 8-core host. Interleaved A/B (old → new → old → new after a
cool-down, warm-up run before each measurement) to control for thermal
drift:

roundold (RRWL per attribute)lock-free (this PR)
127,922 ops/s / 7.22 ms34,008 ops/s / 5.91 ms (+22%)
230,994 ops/s / 6.61 ms31,385 ops/s / 6.42 ms (+1.3%)

Both rounds favor the lock-free version (avg +11% throughput, lower
latency); variance on the shared 8-core host is high, and as with #667/#669
the structural win is removing a serialization ceiling that grows with core
count. authrate errors: 0.

Review follow-up (7a290de)

reloadMappingsIfSchemaChanged() now captures the mappings reference
once under the exclusive lock (final Mappings oldMappings = mappings;)
and reuses the local for the size calculation and both reload calls.
Not required for correctness — all mutations are serialized by the same
lock, so the repeated volatile reads saw one value — but it makes the
reload-from-one-snapshot intent explicit and matches the pattern already
used in getAttributeId()/getObjectClassId().

Testing

mvn -P precommit -pl opendj-server-legacy verify for
ModifyOperationTestCase (932), AddOperationTestCase (137),
TestDnKeyFormat (30) — entry encode/decode and schema paths:
1099 tests, 0 failures — re-run with the same result after the
review follow-up.

Files

  • opendj-server-legacy/src/main/java/org/opends/server/api/CompressedSchema.java

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 OpenIdentityPlatform#660 bind benchmark scenario shows +1..+22%
throughput (avg +11%) and consistently lower latency on an 8-core host.
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().
@vharsekovharseko added the performance Performance / concurrency / lock-contention work label Jul 6, 2026
@vharseko
vharseko merged commit 9254b28 into OpenIdentityPlatform:masterAug 6, 2026
14 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmarkenhancementperformancePerformance / concurrency / lock-contention work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@vharseko@maximthomas