Skip to content

Avoid per-poll ConcurrentHashMap KeyIterator allocation in operation managers - #3271

Merged
nicolaslopezbravo merged 1 commit into
linkedin:masterfrom
nicolaslopezbravo:nlb/router-poll-keyiterator-garbage
Jun 22, 2026
Merged

Avoid per-poll ConcurrentHashMap KeyIterator allocation in operation managers#3271
nicolaslopezbravo merged 1 commit into
linkedin:masterfrom
nicolaslopezbravo:nlb/router-poll-keyiterator-garbage

Conversation

@nicolaslopezbravo

Copy link
Copy Markdown
Contributor

What

Guard the per-poll() iteration of each operation manager's in-flight operation set with isEmpty(), so the enhanced-for loop does not allocate a ConcurrentHashMap$KeyIterator when the set is empty.

Why

Each operation manager (GetManager, PutManager, DeleteManager, TtlUpdateManager, UndeleteManager, ReplicateBlobManager) stores its in-flight operations in a ConcurrentHashMap.newKeySet() and iterates it once per poll() with an enhanced-for loop. The for-each calls .iterator(), and ConcurrentHashMap.KeySetView.iterator() allocates a KeyIterator unconditionally — even for an empty set — before hasNext() returns false:

public Iterator<K> iterator() {
    Node<K,V>[] t;
    ConcurrentHashMap<K,V> m = map;
    int f = (t = m.table) == null ? 0 : t.length;
    return new KeyIterator<K,V>(t, f, 0, f, m);  // allocated even when empty
}

The RequestResponseHandler poll loop runs continuously across all six managers, so an idle or lightly-loaded router allocates a throwaway iterator per manager per poll. Heap analysis of a busy frontend showed tens of millions of these short-lived ConcurrentHashMap$KeyIterator objects as the dominant young-gen garbage, inflating GC frequency.

Guarding with !operations.isEmpty() skips the .iterator() call in the empty case; isEmpty() checks the map count and allocates nothing.

Behavior

No behavior change — an empty set's for-each is already a no-op, so the guard only removes the wasted allocation. When operations are present the loop runs exactly as before.

Testing Done

  • ./gradlew :ambry-router:compileJava — BUILD SUCCESSFUL.
  • ./gradlew :ambry-router:test --tests GetManagerTest --tests PutManagerTest --tests DeleteManagerTest --tests TtlUpdateManagerTest --tests UndeleteManagerTest201 tests, 201 passed, 0 failed.

Each operation manager (Get/Put/Delete/TtlUpdate/Undelete/ReplicateBlob)
stores its in-flight operations in a ConcurrentHashMap.newKeySet() and
iterates it once per poll() with an enhanced-for loop. The for-each calls
.iterator(), which allocates a ConcurrentHashMap$KeyIterator unconditionally
-- even when the set is empty -- before hasNext() returns false. With the
RequestResponseHandler poll loop spinning every few ms across six managers,
that churns a throwaway iterator per manager per poll even when nothing is
in flight (the common case on a frontend).

Guarding the loop with !set.isEmpty() skips the .iterator() allocation in
the empty case. isEmpty() checks the map's count and allocates nothing.

Behavior is unchanged: an empty set's for-each is already a no-op, so the
guard only removes the wasted allocation; when operations are present the
loop runs exactly as before.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@nicolaslopezbravo
nicolaslopezbravo force-pushed the nlb/router-poll-keyiterator-garbage branch from 6379772 to c7b5b95 Compare June 13, 2026 04:58
@codecov-commenter

codecov-commenter commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.37288% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.85%. Comparing base (52ba813) to head (c7b5b95).
⚠️ Report is 397 commits behind head on master.

Files with missing lines Patch % Lines
.../com/github/ambry/router/ReplicateBlobManager.java 0.00% 10 Missing and 1 partial ⚠️
...in/java/com/github/ambry/router/DeleteManager.java 45.45% 5 Missing and 1 partial ⚠️
...java/com/github/ambry/router/TtlUpdateManager.java 45.45% 5 Missing and 1 partial ⚠️
.../java/com/github/ambry/router/UndeleteManager.java 45.45% 5 Missing and 1 partial ⚠️
.../main/java/com/github/ambry/router/PutManager.java 62.50% 2 Missing and 1 partial ⚠️
.../main/java/com/github/ambry/router/GetManager.java 71.42% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #3271       +/-   ##
=============================================
- Coverage     64.24%   50.85%   -13.39%     
+ Complexity    10398     8680     -1718     
=============================================
  Files           840      937       +97     
  Lines         71755    80235     +8480     
  Branches       8611     9645     +1034     
=============================================
- Hits          46099    40804     -5295     
- Misses        23004    36038    +13034     
- Partials       2652     3393      +741     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nicolaslopezbravo
nicolaslopezbravo merged commit f92d007 into linkedin:master Jun 22, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants