From c52b6f52ac7cf715aebf447acc868f69e1c76185 Mon Sep 17 00:00:00 2001
From: xiangying <mengxiangying@xiaohongshu.com>
Date: Fri, 20 Mar 2026 11:52:25 +0800
Subject: [PATCH] [improve][common] Optimize TopicName.get() to reduce lock
 contention on cache lookup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

### 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.
---
 .../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 a1c7055a8972c..4728e00d29446 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 static TopicName get(String domain, String tenant, String namespace, Stri
         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) {