(pulsar) 01/01: [improve][common] Optimize TopicName.get() to reduce lock contention on cache lookup

[xi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

xiangying pushed a commit to branch optimize/TopicName-get
in repository https://gitbox.apache.org/repos/asf/pulsar.git

commit c52b6f52ac7cf715aebf447acc868f69e1c76185
Author: xiangying <me...@xiaohongshu.com>
AuthorDate: Fri Mar 20 11:52:25 2026 +0800

    [improve][common] Optimize TopicName.get() to reduce lock contention on cache lookup
    
    ### Motivation
    
    `TopicName.get()` previously used `ConcurrentHashMap.computeIfAbsent()` to populate
    the topic-name cache. Although `computeIfAbsent` is atomic, it holds the internal
    bin-lock for the entire duration of the mapping function, which includes the
    non-trivial `TopicName` construction (string splitting, validation, etc.).
    
    Under high-concurrency workloads where many threads simultaneously encounter the same
    uncached topic name, this causes unnecessary lock contention and can degrade throughput.
    
    ### Modifications
    
    Replace `computeIfAbsent` with an explicit two-step pattern:
    
    1. **Fast path**: call `cache.get(topic)` first — a single volatile read with no
       locking — and return immediately on a cache hit (steady-state case).
    2. **Slow path** (cache miss): construct `TopicName` *outside* the lock, then use
       `cache.putIfAbsent()` to insert. If two threads race on the same key, one wins
       the `putIfAbsent` and the other's instance is discarded; this is safe because
       `TopicName` is immutable.
    
    Add a Javadoc comment on `get()` explaining the rationale.
---
 .../org/apache/pulsar/common/naming/TopicName.java | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
index a1c7055a897..4728e00d294 100644
--- a/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
+++ b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
@@ -71,12 +71,32 @@ public class TopicName implements ServiceUnitId {
         return TopicName.get(name);
     }
 
+    /**
+     * Get or create a TopicName from the cache.
+     *
+     * <p>Optimization over {@code computeIfAbsent}: avoids holding the ConcurrentHashMap bin-lock
+     * while constructing a new TopicName object. The construction (string splitting / parsing) is
+     * pure CPU work and can be done outside the lock. In the typical steady-state (cache hit) this
+     * method does a single volatile read via {@code get()} and returns immediately with no
+     * synchronization overhead.
+     *
+     * <p>In the cache-miss case, two threads racing on the same key may both construct a
+     * {@code TopicName} instance, but only one wins the {@code putIfAbsent} and the loser's
+     * instance is simply discarded. This is safe because {@code TopicName} is immutable and
+     * construction is cheap compared to the lock-contention / context-switch cost of
+     * {@code computeIfAbsent}.
+     */
     public static TopicName get(String topic) {
+        // Fast path: already cached — single volatile read, no lock.
         TopicName tp = cache.get(topic);
         if (tp != null) {
             return tp;
         }
-        return cache.computeIfAbsent(topic, k -> new TopicName(k));
+        // Slow path: construct outside the bin-lock to avoid blocking other threads.
+        TopicName newTp = new TopicName(topic);
+        TopicName existing = cache.putIfAbsent(topic, newTp);
+        // If another thread raced us and already inserted, use its instance (keeps identity stable).
+        return existing != null ? existing : newTp;
     }
 
     public static TopicName getPartitionedTopicName(String topic) {