Skip to content

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

Open
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/bind-6
Open

Remove per-attribute read lock from CompressedSchema decode path#670
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:features/bind-6

Conversation

@vharseko

@vharseko vharseko 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 ReentrantReadWriteLock read 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 decode
try {
  if (schema != serverContext.getSchema()) { ... }
  return mappings;
} 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:

round old (RRWL per attribute) lock-free (this PR)
1 27,922 ops/s / 7.22 ms 34,008 ops/s / 5.91 ms (+22%)
2 30,994 ops/s / 6.61 ms 31,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.
@vharseko vharseko requested a review from maximthomas July 2, 2026 18:23
@vharseko vharseko added this to the 5.2.0 milestone Jul 2, 2026
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().
@vharseko vharseko added the performance Performance / concurrency / lock-contention work label Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark enhancement performance Performance / concurrency / lock-contention work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants