From 0b021e8c786eda5158ff8e5de142ae1fe32b52ef Mon Sep 17 00:00:00 2001 From: David Jacot Date: Sat, 31 Oct 2026 07:37:30 +0100 Subject: [PATCH] MINOR: Avoid double map lookups in OverlayMap (#24040) `OverlayMap` calls `containsKey` followed by `get` with the same key in `get`, `put`, `remove` and the entry iterator, which looks the key up twice. This patch replaces each pair with a single `get` followed by a null check. This is equivalent because neither the base map nor the updates may hold null values. The class Javadoc now states this explicitly. Reviewers: Sean Quah --- .../coordinator/group/util/OverlayMap.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/util/OverlayMap.java b/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/util/OverlayMap.java index 23bdf5ca896..0f612a1a63c 100644 --- a/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/util/OverlayMap.java +++ b/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/util/OverlayMap.java @@ -31,7 +31,7 @@ import java.util.Set; * that overlay on top of it. This class expects the underlying base map to * be immutable. * - *

Null values are not supported. + *

Null values are not supported, neither in the base map nor in updates. * * @param The key type. * @param The value type. @@ -74,8 +74,10 @@ public class OverlayMap extends AbstractMap { @Override public V get(Object key) { - if (additions.containsKey(key)) return additions.get(key); - if (replacements.containsKey(key)) return replacements.get(key); + V value = additions.get(key); + if (value != null) return value; + value = replacements.get(key); + if (value != null) return value; if (removals.contains(key)) return null; return base.get(key); } @@ -94,9 +96,10 @@ public class OverlayMap extends AbstractMap { replacements.put(key, value); return null; } - if (base.containsKey(key)) { + V baseValue = base.get(key); + if (baseValue != null) { replacements.put(key, value); - return base.get(key); + return baseValue; } additions.put(key, value); return null; @@ -115,9 +118,10 @@ public class OverlayMap extends AbstractMap { if (removals.contains(key)) { return null; } - if (base.containsKey(key)) { + V baseValue = base.get(key); + if (baseValue != null) { removals.add(key); - return base.get(key); + return baseValue; } return null; } @@ -147,8 +151,9 @@ public class OverlayMap extends AbstractMap { if (next != null) return true; while (baseIterator.hasNext()) { Entry entry = baseIterator.next(); - if (replacements.containsKey(entry.getKey())) { - next = Map.entry(entry.getKey(), replacements.get(entry.getKey())); + V replacement = replacements.get(entry.getKey()); + if (replacement != null) { + next = Map.entry(entry.getKey(), replacement); } else { next = entry; }