[BUG] Hot Path Performance Regression

[asimmahmood1]

What is the bug?

The current state of bufferpool implemention has 1.5 to 2x regression, especially for search requests that look up many docIds, e.g.

1. cardinality-agg-high: 316 → 914 (1.8x)
2. multi-terms: 9.7s → 24s (2.5x)

For full report see below.

Using async profile, the culprit pretty clear:

Profiling shows BlockSlotTinyCache.acquireRefCountedValue taking majority of the time. This was added in this PR to solve memory leak issue. Also confirmed by not unpinning the memory segment (that leads to memory leak).

How can one reproduce the bug?
Run any osb workload with storage type cryptofs

What is the expected behavior?
Same performance mmap or better.

What is your host/environment?

• OS: Linux
• Version 3.3.1
• Plugins opensearch-storage-encryption

INSTANCE_TYPE="r5.2xlarge"
VOLUME_SIZE=500
VOLUME_TYPE="gp3"
IOPS=16000
THROUGHPUT=1000

Do you have any screenshots?
See above

Do you have any additional context?
Full big5 run, p50

Operation	Metric	Baseline (mmap)	Challenger (bufferpool)	Delta %
index-append	Mean Throughput (docs/s)	62,717	56,328	-10.2% 🔴
index-append	p50 Latency (ms)	15.40	15.75	+2.3%
match-all	p50 Service Time (ms)	4.28	3.27	-23.6% 🟢
desc_sort_timestamp	p50 Service Time (ms)	5.35	6.56	+22.6% 🔴
asc_sort_timestamp	p50 Service Time (ms)	5.26	6.89	+31.2% 🔴
desc_sort_with_after_timestamp	p50 Service Time (ms)	3.72	5.00	+34.2% 🔴
asc_sort_with_after_timestamp	p50 Service Time (ms)	4.33	4.59	+6.1%
desc_sort_timestamp_can_match_shortcut	p50 Service Time (ms)	8.96	5.97	-33.4% 🟢
desc_sort_timestamp_no_can_match_shortcut	p50 Service Time (ms)	8.98	5.65	-37.1% 🟢
asc_sort_timestamp_can_match_shortcut	p50 Service Time (ms)	6.54	5.85	-10.5% 🟢
asc_sort_timestamp_no_can_match_shortcut	p50 Service Time (ms)	6.48	5.83	-10.0% 🟢
term	p50 Service Time (ms)	3.26	3.57	+9.6%
multi_terms-keyword	p50 Service Time (ms)	141.51	263.74	+86.3% 🔴
keyword-terms	p50 Service Time (ms)	28.59	34.09	+19.3% 🟡
keyword-terms-low-cardinality	p50 Service Time (ms)	23.28	27.95	+20.1% 🔴
composite-terms	p50 Service Time (ms)	38.84	99.05	+155.1% 🔴
composite_terms-keyword	p50 Service Time (ms)	63.99	169.39	+164.7% 🔴
composite-date_histogram-daily	p50 Service Time (ms)	3.47	3.95	+13.8% 🟡
range	p50 Service Time (ms)	4.14	4.57	+10.4% 🟡
range-numeric	p50 Service Time (ms)	1.47	1.39	-5.3%
keyword-in-range	p50 Service Time (ms)	10.97	31.37	+186.1% 🔴
date_histogram_hourly_agg	p50 Service Time (ms)	5.69	7.65	+34.3% 🔴
date_histogram_hourly_with_filter_agg	p50 Service Time (ms)	76.51	186.90	+144.3% 🔴
date_histogram_minute_agg	p50 Service Time (ms)	32.65	51.66	+58.2% 🔴
scroll	p50 Service Time (ms)	376.41	372.70	-1.0%
query-string-on-message	p50 Service Time (ms)	4.42	4.75	+7.5%
query-string-on-message-filtered	p50 Service Time (ms)	12.03	29.69	+146.8% 🔴
query-string-on-message-filtered-sorted-num	p50 Service Time (ms)	13.17	26.93	+104.5% 🔴
sort_keyword_can_match_shortcut	p50 Service Time (ms)	3.42	3.69	+7.8%
sort_keyword_no_can_match_shortcut	p50 Service Time (ms)	3.23	3.66	+13.4% 🟡
sort_numeric_desc	p50 Service Time (ms)	3.63	4.51	+24.3% 🔴
sort_numeric_asc	p50 Service Time (ms)	3.35	3.56	+6.2%
sort_numeric_desc_with_match	p50 Service Time (ms)	1.63	1.60	-2.0%
sort_numeric_asc_with_match	p50 Service Time (ms)	1.67	1.60	-3.9%
range_field_conjunction_big_range_big_term	p50 Service Time (ms)	1.36	1.31	-3.9%
range_field_disjunction_big_range_small_term	p50 Service Time (ms)	1.58	1.54	-2.6%
range_field_conjunction_small_range_small_term	p50 Service Time (ms)	1.45	1.60	+9.9%
range_field_conjunction_small_range_big_term	p50 Service Time (ms)	1.24	1.22	-1.2%
range-auto-date-histo	p50 Service Time (ms)	482.53	1,385.98	+187.2% 🔴
range-with-metrics	p50 Service Time (ms)	1,755.76	4,898.02	+178.9% 🔴
range-auto-date-histo-with-metrics	p50 Service Time (ms)	2,011.74	4,911.37	+144.1% 🔴
range-agg-1	p50 Service Time (ms)	1.75	1.77	+1.1%
range-agg-2	p50 Service Time (ms)	1.78	1.86	+4.8%
cardinality-agg-low	p50 Service Time (ms)	4.13	3.52	-14.9% 🟢
cardinality-agg-high	p50 Service Time (ms)	316.37	914.28	+189.0% 🔴
+------------------------------------------------+--------------------------+-----------------+-------------------------+------------+

[asimmahmood1]

One option is switch to using DirectByteBuffer, which is a big change in terms of architecture of bufferpool. I asked Kiro-cli for analysis:

Current Design: RefCountedMemorySegment + MemorySegmentPool

The hot path in acquireRefCountedValue does:

1. VarHandle acquire-load of stamp
2. CAS-based tryPin() (VarHandle CAS loop on packed state)
3. Generation validation
4. On miss: getOrLoad → retry loop with LockSupport.parkNanos backoff (up to 10 attempts)

The contention comes from the CAS loop in tryPin()/unpin() — every read operation pins, reads, unpins. Slices make it worse since they unpin after every operation (
releasePinnedBlockIfSlice), meaning 10k+ slices = 10k+ CAS pairs per read sweep.

Pros of switching to DirectByteBuffer (GC-managed)

1. Eliminates all CAS contention — no tryPin/unpin, no generation checks, no retry loops. The entire acquireRefCountedValue hot path simplifies dramatically.

2. No retry loop — the current 10-attempt loop with exponential backoff (parkNanos(50_000L << attempts)) goes away entirely. A DirectByteBuffer is always safe to read from
   as long as you hold a reference.

3. Slice fan-out becomes free — the releasePinnedBlockIfSlice() workaround for 10k+ slices pinning blocks disappears. GC handles it.

4. Simpler code — no BlockReleaser, no reset(), no packed state with generation+refcount, no MemorySegmentPool lock contention on release.

5. No pool exhaustion stalls — current design blocks on notEmpty.await() when pool is exhausted. DirectByteBuffer allocation never blocks (until you hit OOM).

Cons of switching to DirectByteBuffer

1. No deterministic memory reclamation — this is the big one. Your pool currently has a hard cap (maxSegments) and reclaims memory immediately on decRef→0. With
   DirectByteBuffer, memory is freed by Cleaner during GC, which is unpredictable. Under heavy allocation churn you can hit OutOfMemoryError: Direct buffer memory even though
   buffers are logically unreachable but not yet GC'd.

2. No memory zeroing guarantee — your pool does segment.fill((byte) 0) on release for security (encryption key residue). DirectByteBuffer doesn't zero on deallocation. For
   an encryption plugin, this is a real concern — decrypted plaintext could linger in native memory until the OS reclaims the page.

3. Allocation cost — MemorySegmentPool reuses pre-allocated native memory. DirectByteBuffer.allocateDirect() calls malloc + mmap each time (or hits the JVM's internal
   DirectByteBuffer cache, which is limited and has its own lock contention). Your pool's warmup strategy (warmUp()) gives predictable startup performance that you'd lose.

4. No backpressure — the pool's isUnderPressure() and bounded capacity provide natural flow control. With DirectByteBuffer, there's no way to signal "slow down" to callers —
   you just OOM. The -XX:MaxDirectMemorySize limit is a blunt instrument compared to your current pool sizing.

5. GC pressure — each DirectByteBuffer creates a Java heap object + a Cleaner reference. With high block churn (cache evictions), you're creating GC work. The current pool
   avoids this entirely — the same RefCountedMemorySegment objects are recycled without GC involvement.

6. Loss of MemorySegment API — your encryption code uses MemorySegment for Panama FFI (PanamaNativeAccess.malloc), OpenSSL JNI integration, and io_uring Direct I/O.
   DirectByteBuffer would require converting back to MemorySegment via MemorySegment.ofBuffer() at every crypto boundary, adding overhead and complexity.

7. No metrics/observability — pool stats (utilizationRatio, allocationRatio, isUnderPressure) disappear. You'd be flying blind on memory usage.

Alternative: reduce CAS contention without abandoning ref counting

Rather than switching to DirectByteBuffer, consider addressing the root cause — the CAS contention in tryPin/unpin:

• Epoch-based reclamation — readers enter/exit an epoch (a single volatile write) instead of per-block CAS. Eviction waits for epoch quiescence. This is what crossbeam
  (Rust) and java.util.concurrent hazard pointers do.
• Batch pin/unpin for sequential reads — pin a block once, read multiple bytes, unpin once. The slice path already does this for master inputs; extend it.
• Striped refcounts — use LongAdder-style striping to reduce CAS contention on hot blocks, at the cost of more memory per block.
• Read-copy-update (RCU) — readers never block; writers (eviction) defer cleanup until all readers have exited. Good fit since reads vastly outnumber evictions.

The TL;DR: DirectByteBuffer would fix the contention but at the cost of deterministic memory control, security zeroing, and Panama FFI compatibility — all of which are core
to an encryption plugin. I'd lean toward fixing the contention within the current ref-counting model.

[asimmahmood1]

Estimate for DirectByteBuffer

1. Switch from MemorySegment to DirectByteBuffer - 2 day

• many of the API are similar, RefCountedMemorySegment class would simplified/removed

2. Add admission control when allocating memory - 2 days

• this may be the best effect based on live cache data, we don't know when JVM will actually release, but we can start with this one

3. Create jmh bechmarks to compare the result, should be similar/better than MMAP - 1 day
4. PR review and feedback - 2 days

Estimate for educe CAS contention

This is more open-ended, need to experiment:

1. Epoch-based reclamation — need to understand this
2. Batch pin/unpin for sequential reads - this was state before pin/unpin was added per read. Alternative to clean up the slices is using Cleaner - this should work for sequencial reads - 1 day?
3. Striped refcounts - need to understand
4. Read-copy-update - there no writes happening on host, not sure how it helps.

[abiesps]

Basically, the way I understand it is that if we don't do reference counting at byte granularity (for slices/clones), then we have to rely on the garbage collector, because Lucene doesn't call close() on slices and clones.

If we choose ByteBuffer, the parent direct buffer creates a cleaner, and all slices of the ByteBuffer refer to the same parent buffer. As a result, the garbage collector can free the underlying native memory once the parent and all slices become unreachable, without requiring explicit reference counting.

With MemorySegments, we could theoretically follow a similar approach and avoid reference counting for slices as well.

However, an important distinction that @jaredlundell pointed out is that the JVM can track how much memory is used by direct byte buffers and can trigger GC when there is memory pressure. In contrast, with MemorySegments, the JVM has no built-in awareness of how much native memory has been allocated and therefore won't necessarily trigger GC unless heap occupancy makes it worthwhile.

Since our design already relies on GC for cleanup, this difference becomes important. The JVM can react to large amounts of unreachable direct byte buffers because it tracks direct memory usage and enforces limits such as -XX:MaxDirectMemorySize, whereas MemorySegment allocations are not accounted for in the same way.

Given that we depend on GC for reclamation anyway, using DirectByteBuffer seems safer. With MemorySegments there is a higher risk of native OOM if a large number of segments become unreachable but GC is not triggered promptly.

So my inclination is that we should go with direct byte buffers.

cc @jaredlundell, @itiyamas

[jaredlundell]

No deterministic memory reclamation

While this is true, there is some mitigation via -XX:MaxDirectMemorySize, which will trigger GCs when needed.

No memory zeroing guarantee

The plaintext is already present in memory. Not zeroing on release doesn't meaningfully change the threat model.

Allocation cost — MemorySegmentPool reuses pre-allocated native memory

Yep, that's a downside. Since we're always allocating 8KB buffers, I'd expect our allocation pattern to be as easy as it gets for the memory allocator to deal with.

No backpressure — the pool's isUnderPressure() and bounded capacity provide natural flow control. With DirectByteBuffer, there's no way to signal "slow down" to callers

What are we doing with the "slow down" signal today? Is there actually anything meaningful we can do to reduce the memory needed when we're under pressure?

GC pressure — each DirectByteBuffer creates a Java heap object + a Cleaner reference

Yes, there is some risk here, especially since these are relatively long lived objects. We should benchmark. But I expect the Caffiene cache is also creating some on-heap objects, so the incremental cost here may not be all that high.

Loss of MemorySegment API

We can and should to use MemorySegments for DirectIO and crypto integration. We're using separate buffers for IO vs the block cache, and the data gets copied from the IO buffers into the block cache buffers after it is decrypted. That's the ideal place to transition from MemorySegments to Direct ByteBuffers.

No metrics/observability

We still know the size of the block cache, and all the regular JVM metrics still exist. There are JMX metrics for direct memory consumption.

My perspective on this is that Lucene's behavior of not closing IndexInput Slices and Clones is actively hostile to any approach that involves ref counting. I'd love it if we could fix that problem on the Lucene side, but that's a pretty heavy lift, even if we could get the Lucene community to agree. We either end up with the excessive ref counting we have now that incurs atomic operation costs for every single byte read, or we rely on the GC in some fashion. If we're relying on the GC I see no point in also incurring any ref counting overhead, and doing it with MemorySegments is going to be strictly worse than with Direct ByteBuffers because the VM will have no insight into the off-heap usage and thus won't trigger GCs. That's a major risk for the Cleaner approach.

I recommend we move forward with direct ByteBuffers

[jaredlundell]

We should also benchmark this with other garbage collectors, specifically Generational Shenandoah (with JDK25)

[asimmahmood1]

I'm trying to explore some other options. I need to spend time to think about this validate analysis. Would be great to get others eyes on this option:

Epoch-Based Reclamation for your cache

The core problem: every readByte() on a slice does tryPin() (CAS) → read → unpin() (CAS). That's 2 atomic CAS operations per read, contending on the same cache line when multiple threads read the same hot block.

Epoch-based reclamation flips the model: readers announce "I'm reading" cheaply (thread-local write), and the evictor waits until all readers have left before recycling memory.

How it maps to your code

Current flow (per-block refcount):

Reader thread:                          Evictor (Caffeine removalListener):
  tryPin()    ← CAS on block.state
  read segment data
  unpin()     ← CAS on block.state       close() ← CAS on block.state
                                          if refCount==0 → return to pool

Epoch-based flow:

Reader thread:                          Evictor (Caffeine removalListener):
  enterEpoch()  ← thread-local write
  read segment data                       retire(segment) → add to retired list
  leaveEpoch()  ← thread-local write     reclaimRetired() → check all threads
                                            if all threads past this epoch
                                              → zero memory, return to pool

How the read path changes

In CachedMemorySegmentIndexInput:

// Before (current code - 2 CAS ops per read):
BlockCacheValue<RefCountedMemorySegment> val = cache.acquireRefCountedValue(blockOff);
// ... read from val.value().segment() ...
val.unpin();  // in releasePinnedBlockIfSlice()

// After (epoch - 2 thread-local volatile writes per read):
epochTracker.enter();
try {
    MemorySegment seg = cache.getSegment(blockOff); // no pin needed
    // ... read from seg ...
} finally {
    epochTracker.leave();
}

The acquireRefCountedValue retry loop (10 attempts with backoff) goes away entirely — the segment can't be recycled while you're inside the epoch.

How eviction changes

In the Caffeine removalListener:

// Before (current):
removalExec.execute(() -> value.close());
// close() does: bump generation + decRef → if 0 → return to pool

// After (epoch):
removalExec.execute(() -> {
    long retiredAt = epochTracker.advanceEpoch();
    retiredList.add(new RetiredBlock(value, retiredAt));
});

// Separate reclaim thread (or piggyback on removalExec):
void reclaimRetired() {
    long safe = epochTracker.safeEpoch();
    Iterator<RetiredBlock> it = retiredList.iterator();
    while (it.hasNext()) {
        RetiredBlock rb = it.next();
        if (rb.retiredAt < safe) {
            rb.segment.fill((byte) 0);  // security zeroing
            pool.release(rb.segment);
            it.remove();
        }
    }
}

What you gain

Aspect	Current (refcount)	Epoch-based
Read hot path cost	2× CAS on shared cache line	2× thread-local volatile write
Slice fan-out (10k slices)	10k CAS pairs per sweep	10k thread-local writes (no contention)
Retry loop	Up to 10 attempts + parkNanos	None — segment always safe in epoch
BlockSlotTinyCache complexity	stamp + generation + tryPin + retry	stamp + simple lookup (no pin)

What you lose / tradeoffs

1. Delayed reclamation — memory isn't freed the instant refcount hits 0. It's freed when the slowest reader leaves its epoch. A stalled reader thread holds up all reclamation. You'd need a timeout/watchdog for this.

2. Retired list overhead — evicted blocks sit in a limbo list until safe to reclaim. Under heavy eviction churn, this list can grow. You'd want to bound it and apply backpressure (pause eviction if retired list is too large).

3. Thread registration — each reader thread registers a ThreadState. The evictor scans all of them in safeEpoch(). With hundreds of threads this scan is O(N) but still just volatile reads — much cheaper than the current CAS contention.

4. Security zeroing delay — decrypted plaintext stays in native memory slightly longer (until epoch is safe). For your encryption plugin, you'd want to minimize the epoch window.

[kumargu]

My perspective on this is that Lucene's behavior of not closing IndexInput Slices and Clones is actively hostile to any approach that involves ref counting. I'd love it if we could fix that problem on the Lucene side, but that's a pretty heavy lift, even if we could get the Lucene community to agree.

Agreed. The fact that slices are never closed brings this kind of complexity.

We either end up with the excessive ref counting we have now that incurs atomic operation costs for every single byte read, or we rely on the GC in some fashion. If we're relying on the GC I see no point in also incurring any ref counting overhead, and doing it with MemorySegments is going to be strictly worse than with Direct ByteBuffers because the VM will have no insight into the off-heap usage and thus won't trigger GCs. That's a major risk for the Cleaner approach.

I had tried GC with MemorySegments and not doing refcounting and relying on -XX:MaxDirectMemorySize and it was more expensive.

[kumargu]

BTW this is not a bug that we created while fixing a bug. Running out of memory is more dangerous over performance. see comment where it was called out that we see perf impact #129 (comment) but i was also out of ideas on how we can avoid the overhead of the pin/unpin.

[abiesps]

Another idea that I was thinking to overcome the allocation cost
With direct byte buffers - if we see any concerns in cpu profiles. We could potentially keep empty byte buffers in the MemorySegment pool and get a byte buffer from memory segment pool but while releasing we don't have to add it back to the free list. We can periodically keep on adding empty new byte buffer to the pool based on pool, cache and memory metric.

However I do feel that we should first measure what's the "allocation cost" actually is. Especially during cold starts