Copilot commented on code in PR #63797: URL: https://github.com/apache/doris/pull/63797#discussion_r3315888983
########## fe/fe-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2ObjectHashMap.java: ########## @@ -0,0 +1,487 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import it.unimi.dsi.fastutil.longs.AbstractLong2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectFunction; +import it.unimi.dsi.fastutil.longs.Long2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import it.unimi.dsi.fastutil.longs.LongSet; +import it.unimi.dsi.fastutil.objects.ObjectArrayList; +import it.unimi.dsi.fastutil.objects.ObjectCollection; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import it.unimi.dsi.fastutil.objects.ObjectSet; + +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongFunction; + +/** + * A concurrent map with primitive long keys and object values, backed by segmented + * {@link Long2ObjectOpenHashMap} instances with {@link ReentrantReadWriteLock} per segment. + * + * <p>This class provides similar concurrency guarantees to {@link java.util.concurrent.ConcurrentHashMap} + * while avoiding the memory overhead of boxing long keys. For a cluster with millions of tablet entries, + * this saves ~32 bytes per entry compared to {@code ConcurrentHashMap<Long, V>}. + * + * <p>Like {@link java.util.concurrent.ConcurrentHashMap}, null values are not permitted. + * + * <p>Iteration methods ({@link #long2ObjectEntrySet()}, {@link #keySet()}, {@link #values()}) + * return snapshot copies and are weakly consistent. + * Review Comment: `keySet()`, `values()`, and `long2ObjectEntrySet()` return snapshot copies. This breaks the `java.util.Map` contract that these collections are backed by the map (changes should be reflected both ways) and also undermines the stated goal of being a drop-in replacement for `ConcurrentHashMap<Long,V>`. Consider returning live view collections (with iterators that are weakly consistent / segment-locked), and if snapshots are needed, expose them as separate `snapshotKeySet()/snapshotValues()/snapshotEntrySet()` helpers instead of overriding the standard view methods. ########## fe/fe-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2ObjectHashMap.java: ########## @@ -0,0 +1,487 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import it.unimi.dsi.fastutil.longs.AbstractLong2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectFunction; +import it.unimi.dsi.fastutil.longs.Long2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import it.unimi.dsi.fastutil.longs.LongSet; +import it.unimi.dsi.fastutil.objects.ObjectArrayList; +import it.unimi.dsi.fastutil.objects.ObjectCollection; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import it.unimi.dsi.fastutil.objects.ObjectSet; + +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongFunction; + +/** + * A concurrent map with primitive long keys and object values, backed by segmented + * {@link Long2ObjectOpenHashMap} instances with {@link ReentrantReadWriteLock} per segment. + * + * <p>This class provides similar concurrency guarantees to {@link java.util.concurrent.ConcurrentHashMap} + * while avoiding the memory overhead of boxing long keys. For a cluster with millions of tablet entries, + * this saves ~32 bytes per entry compared to {@code ConcurrentHashMap<Long, V>}. + * + * <p>Like {@link java.util.concurrent.ConcurrentHashMap}, null values are not permitted. + * + * <p>Iteration methods ({@link #long2ObjectEntrySet()}, {@link #keySet()}, {@link #values()}) + * return snapshot copies and are weakly consistent. + * + * <p><b>Important:</b> All compound operations from both {@link Long2ObjectMap} and {@link Map} + * interfaces (computeIfAbsent, computeIfPresent, compute, merge, putIfAbsent, replace, remove) + * are overridden to ensure atomicity within a segment. + * + * @param <V> the type of mapped values + */ +public class ConcurrentLong2ObjectHashMap<V> extends AbstractLong2ObjectMap<V> { + + private static final int DEFAULT_SEGMENT_COUNT = 16; + private static final int DEFAULT_INITIAL_CAPACITY_PER_SEGMENT = 16; + + private final Segment<V>[] segments; + private final int segmentMask; + private final int segmentBits; + + public ConcurrentLong2ObjectHashMap() { + this(DEFAULT_SEGMENT_COUNT); + } + + @SuppressWarnings("unchecked") + public ConcurrentLong2ObjectHashMap(int segmentCount) { + if (segmentCount <= 0 || (segmentCount & (segmentCount - 1)) != 0) { + throw new IllegalArgumentException("segmentCount must be a positive power of 2: " + segmentCount); + } + this.segmentBits = Integer.numberOfTrailingZeros(segmentCount); + this.segmentMask = segmentCount - 1; + this.segments = new Segment[segmentCount]; + for (int i = 0; i < segmentCount; i++) { + segments[i] = new Segment<>(DEFAULT_INITIAL_CAPACITY_PER_SEGMENT); + } + } + + /** Murmur3 64-bit finalizer for segment selection. */ + private static long mix(long x) { + x ^= x >>> 33; + x *= 0xff51afd7ed558ccdL; + x ^= x >>> 33; + x *= 0xc4ceb9fe1a85ec53L; + x ^= x >>> 33; + return x; + } + + private Segment<V> segmentFor(long key) { + return segments[(int) (mix(key) >>> (64 - segmentBits)) & segmentMask]; + } + + // ---- Read operations (read-lock) ---- + + @Override + public V get(long key) { + Segment<V> seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.get(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + public V getOrDefault(long key, V defaultValue) { + Segment<V> seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + V val = seg.map.get(key); + return (val != null || seg.map.containsKey(key)) ? val : defaultValue; + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsKey(long key) { + Segment<V> seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.containsKey(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsValue(Object value) { + for (Segment<V> seg : segments) { + seg.lock.readLock().lock(); + try { + if (seg.map.containsValue(value)) { + return true; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return false; + } + + @Override + public int size() { + long total = 0; + for (Segment<V> seg : segments) { + seg.lock.readLock().lock(); + try { + total += seg.map.size(); + } finally { + seg.lock.readLock().unlock(); + } + } + return (int) Math.min(total, Integer.MAX_VALUE); + } + + @Override + public boolean isEmpty() { + for (Segment<V> seg : segments) { + seg.lock.readLock().lock(); + try { + if (!seg.map.isEmpty()) { + return false; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return true; + } + + // ---- Write operations (write-lock) ---- + + @Override + public V put(long key, V value) { + Objects.requireNonNull(value, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.put(key, value); + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public V remove(long key) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.remove(key); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V putIfAbsent(long key, V value) { + Objects.requireNonNull(value, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V existing = seg.map.get(key); + if (existing != null || seg.map.containsKey(key)) { + return existing; + } + seg.map.put(key, value); + return null; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public boolean replace(long key, V oldValue, V newValue) { + Objects.requireNonNull(newValue, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V curValue = seg.map.get(key); + if (!Objects.equals(curValue, oldValue) || (curValue == null && !seg.map.containsKey(key))) { + return false; + } + seg.map.put(key, newValue); + return true; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V replace(long key, V value) { + Objects.requireNonNull(value, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.put(key, value); + } + return null; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public boolean remove(Object key, Object value) { + if (!(key instanceof Long)) { + return false; + } + long k = (Long) key; + Segment<V> seg = segmentFor(k); + seg.lock.writeLock().lock(); + try { + V curValue = seg.map.get(k); + if (!Objects.equals(curValue, value) || (curValue == null && !seg.map.containsKey(k))) { + return false; + } + seg.map.remove(k); + return true; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public void clear() { + for (Segment<V> seg : segments) { + seg.lock.writeLock().lock(); + try { + seg.map.clear(); + } finally { + seg.lock.writeLock().unlock(); + } + } + } + + @Override + public void putAll(Map<? extends Long, ? extends V> m) { + for (Map.Entry<? extends Long, ? extends V> entry : m.entrySet()) { + put(entry.getKey().longValue(), entry.getValue()); + } + } + + // ---- Atomic compound operations ---- + // Override ALL compound methods from both Long2ObjectMap and Map interfaces + // to ensure the check-then-act is atomic within a segment's write lock. + + public V computeIfAbsent(long key, LongFunction<? extends V> mappingFunction) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V val = seg.map.get(key); + if (val != null || seg.map.containsKey(key)) { + return val; + } + V newValue = mappingFunction.apply(key); + if (newValue != null) { + seg.map.put(key, newValue); + } + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V computeIfAbsent(long key, Long2ObjectFunction<? extends V> mappingFunction) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V val = seg.map.get(key); + if (val != null || seg.map.containsKey(key)) { + return val; + } + V newValue = mappingFunction.get(key); + if (newValue != null) { + seg.map.put(key, newValue); + } + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public V computeIfAbsent(Long key, Function<? super Long, ? extends V> mappingFunction) { + return computeIfAbsent(key.longValue(), (long k) -> mappingFunction.apply(k)); + } + + public V computeIfPresent(long key, BiFunction<? super Long, ? super V, ? extends V> remappingFunction) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V oldValue = seg.map.get(key); + if (oldValue != null || seg.map.containsKey(key)) { + V newValue = remappingFunction.apply(key, oldValue); + if (newValue != null) { + seg.map.put(key, newValue); + } else { + seg.map.remove(key); + } + return newValue; + } + return null; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V compute(long key, BiFunction<? super Long, ? super V, ? extends V> remappingFunction) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V oldValue = seg.map.containsKey(key) ? seg.map.get(key) : null; + V newValue = remappingFunction.apply(key, oldValue); + if (newValue != null) { + seg.map.put(key, newValue); + } else if (seg.map.containsKey(key)) { + seg.map.remove(key); + } + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V merge(long key, V value, BiFunction<? super V, ? super V, ? extends V> remappingFunction) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { Review Comment: `merge(long key, V value, ...)` does not reject a null `value`, even though this map disallows null values (and `ConcurrentHashMap.merge` throws NPE for a null value). Add `Objects.requireNonNull(value, ...)` (and similarly validate `remappingFunction`) before acquiring the segment lock to keep behavior consistent and prevent silent no-ops/removals when callers accidentally pass null. ########## fe/fe-core/src/test/java/org/apache/doris/common/util/ConcurrentLong2LongHashMapTest.java: ########## @@ -0,0 +1,458 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import com.google.gson.Gson; Review Comment: The file is located under `org/apache/doris/common/util`, but the declared package is `org.apache.doris.foundation.util`. This is inconsistent with the surrounding test layout and may violate build/style checks. Either move the test to `src/test/java/org/apache/doris/foundation/util/` or change the package to `org.apache.doris.common.util` and import the map under test. ########## fe/fe-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2LongHashMap.java: ########## @@ -0,0 +1,526 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import it.unimi.dsi.fastutil.longs.AbstractLong2LongMap; +import it.unimi.dsi.fastutil.longs.Long2LongFunction; +import it.unimi.dsi.fastutil.longs.Long2LongMap; +import it.unimi.dsi.fastutil.longs.Long2LongOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import it.unimi.dsi.fastutil.longs.LongSet; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import it.unimi.dsi.fastutil.objects.ObjectSet; + +import java.util.Map; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongBinaryOperator; +import java.util.function.LongUnaryOperator; + +/** + * A concurrent map with primitive long keys and primitive long values, backed by segmented + * {@link Long2LongOpenHashMap} instances with {@link ReentrantReadWriteLock} per segment. + * + * <p>This class saves ~48 bytes per entry compared to {@code ConcurrentHashMap<Long, Long>} + * by avoiding boxing of both keys and values. For fields like partition update row counts + * with millions of entries, this translates to hundreds of MB of heap savings. + * + * <p>The {@link #addTo(long, long)} method provides atomic increment semantics, useful for + * counter patterns. + * + * <p><b>Note:</b> The {@code defaultReturnValue} is fixed at 0. Calling + * {@link #defaultReturnValue(long)} will throw {@link UnsupportedOperationException} + * because it cannot be propagated to the underlying segment maps consistently. + * + * <p><b>Important:</b> All compound operations from both {@link Long2LongMap} and {@link Map} + * interfaces (computeIfAbsent, computeIfPresent, compute, merge, mergeLong, putIfAbsent, + * replace, remove) are overridden to ensure atomicity within a segment. + */ +public class ConcurrentLong2LongHashMap extends AbstractLong2LongMap { + + private static final int DEFAULT_SEGMENT_COUNT = 16; + private static final int DEFAULT_INITIAL_CAPACITY_PER_SEGMENT = 16; + + private final Segment[] segments; + private final int segmentMask; + private final int segmentBits; + + public ConcurrentLong2LongHashMap() { + this(DEFAULT_SEGMENT_COUNT); + } + + public ConcurrentLong2LongHashMap(int segmentCount) { + if (segmentCount <= 0 || (segmentCount & (segmentCount - 1)) != 0) { + throw new IllegalArgumentException("segmentCount must be a positive power of 2: " + segmentCount); + } + this.segmentBits = Integer.numberOfTrailingZeros(segmentCount); + this.segmentMask = segmentCount - 1; + this.segments = new Segment[segmentCount]; + for (int i = 0; i < segmentCount; i++) { + segments[i] = new Segment(DEFAULT_INITIAL_CAPACITY_PER_SEGMENT); + } + } + + @Override + public void defaultReturnValue(long rv) { + throw new UnsupportedOperationException( + "ConcurrentLong2LongHashMap does not support changing defaultReturnValue. " + + "It is fixed at 0."); + } + + /** Murmur3 64-bit finalizer for segment selection. */ + private static long mix(long x) { + x ^= x >>> 33; + x *= 0xff51afd7ed558ccdL; + x ^= x >>> 33; + x *= 0xc4ceb9fe1a85ec53L; + x ^= x >>> 33; + return x; + } + + private Segment segmentFor(long key) { + return segments[(int) (mix(key) >>> (64 - segmentBits)) & segmentMask]; + } + + // ---- Read operations (read-lock) ---- + + @Override + public long get(long key) { + Segment seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.get(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + public long getOrDefault(long key, long defaultValue) { + Segment seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.containsKey(key) ? seg.map.get(key) : defaultValue; + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsKey(long key) { + Segment seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.containsKey(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsValue(long value) { + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + if (seg.map.containsValue(value)) { + return true; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return false; + } + + @Override + public int size() { + long total = 0; + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + total += seg.map.size(); + } finally { + seg.lock.readLock().unlock(); + } + } + return (int) Math.min(total, Integer.MAX_VALUE); + } + + @Override + public boolean isEmpty() { + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + if (!seg.map.isEmpty()) { + return false; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return true; + } + + // ---- Write operations (write-lock) ---- + + @Override + public long put(long key, long value) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.put(key, value); + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public long remove(long key) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.remove(key); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long putIfAbsent(long key, long value) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.get(key); + } + seg.map.put(key, value); + return defaultReturnValue(); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public boolean replace(long key, long oldValue, long newValue) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key) && seg.map.get(key) == oldValue) { + seg.map.put(key, newValue); + return true; + } + return false; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long replace(long key, long value) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.put(key, value); + } + return defaultReturnValue(); + } finally { + seg.lock.writeLock().unlock(); + } + } Review Comment: Only the primitive overloads of `putIfAbsent`/`replace` are implemented here. Because `Map.putIfAbsent`/`Map.replace` are default methods, callers using a `Map<Long,Long>` reference may invoke non-atomic default implementations. Add boxed overrides (`putIfAbsent(Long,Long)`, `replace(Long,Long)`, `replace(Long,Long,Long)`) delegating to the segment-locked primitive methods. ########## fe/fe-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2ObjectHashMap.java: ########## @@ -0,0 +1,487 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import it.unimi.dsi.fastutil.longs.AbstractLong2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectFunction; +import it.unimi.dsi.fastutil.longs.Long2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import it.unimi.dsi.fastutil.longs.LongSet; +import it.unimi.dsi.fastutil.objects.ObjectArrayList; +import it.unimi.dsi.fastutil.objects.ObjectCollection; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import it.unimi.dsi.fastutil.objects.ObjectSet; + +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongFunction; + +/** + * A concurrent map with primitive long keys and object values, backed by segmented + * {@link Long2ObjectOpenHashMap} instances with {@link ReentrantReadWriteLock} per segment. + * + * <p>This class provides similar concurrency guarantees to {@link java.util.concurrent.ConcurrentHashMap} + * while avoiding the memory overhead of boxing long keys. For a cluster with millions of tablet entries, + * this saves ~32 bytes per entry compared to {@code ConcurrentHashMap<Long, V>}. + * + * <p>Like {@link java.util.concurrent.ConcurrentHashMap}, null values are not permitted. + * + * <p>Iteration methods ({@link #long2ObjectEntrySet()}, {@link #keySet()}, {@link #values()}) + * return snapshot copies and are weakly consistent. + * + * <p><b>Important:</b> All compound operations from both {@link Long2ObjectMap} and {@link Map} + * interfaces (computeIfAbsent, computeIfPresent, compute, merge, putIfAbsent, replace, remove) + * are overridden to ensure atomicity within a segment. + * Review Comment: The Javadoc says all compound operations from both `Long2ObjectMap` and `Map` are overridden to ensure atomicity, but this class only defines primitive-key overloads (e.g. `computeIfPresent(long, ...)`, `merge(long, ...)`, `putIfAbsent(long, ...)`). Calls made through a `Map<Long,V>` / `ConcurrentMap<Long,V>` reference will dispatch to the boxed `Map` default methods unless you override the boxed signatures here, losing atomicity within a segment. Add boxed overrides like `putIfAbsent(Long,V)`, `computeIfPresent(Long,BiFunction)`, `compute(Long,BiFunction)`, `merge(Long,V,BiFunction)`, and `replace(...)` that delegate to the segment-locked implementations. ########## fe/fe-foundation/pom.xml: ########## @@ -29,9 +29,19 @@ under the License. <artifactId>fe-foundation</artifactId> <packaging>jar</packaging> <name>Doris FE Foundation</name> - <description>Zero-dependency foundation utilities for Doris FE modules and SPI plugins</description> + <description>Foundation utilities for Doris FE modules and SPI plugins</description> - <!-- Intentionally NO dependencies. This module depends only on JDK. --> + <dependencies> + <dependency> + <groupId>it.unimi.dsi</groupId> + <artifactId>fastutil-core</artifactId> + </dependency> + <dependency> + <groupId>com.google.code.gson</groupId> + <artifactId>gson</artifactId> + <scope>test</scope> + </dependency> + </dependencies> Review Comment: `fe-foundation` currently has no `src/test` sources, so the added `gson` dependency with `<scope>test</scope>` is unused. Keeping this module’s dependency set minimal helps downstream consumers (SPI plugins) and reduces build noise; remove the unused test dependency or add tests in this module that justify it. ########## fe/fe-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2LongHashMap.java: ########## @@ -0,0 +1,526 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import it.unimi.dsi.fastutil.longs.AbstractLong2LongMap; +import it.unimi.dsi.fastutil.longs.Long2LongFunction; +import it.unimi.dsi.fastutil.longs.Long2LongMap; +import it.unimi.dsi.fastutil.longs.Long2LongOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import it.unimi.dsi.fastutil.longs.LongSet; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import it.unimi.dsi.fastutil.objects.ObjectSet; + +import java.util.Map; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongBinaryOperator; +import java.util.function.LongUnaryOperator; + +/** + * A concurrent map with primitive long keys and primitive long values, backed by segmented + * {@link Long2LongOpenHashMap} instances with {@link ReentrantReadWriteLock} per segment. + * + * <p>This class saves ~48 bytes per entry compared to {@code ConcurrentHashMap<Long, Long>} + * by avoiding boxing of both keys and values. For fields like partition update row counts + * with millions of entries, this translates to hundreds of MB of heap savings. + * + * <p>The {@link #addTo(long, long)} method provides atomic increment semantics, useful for + * counter patterns. + * + * <p><b>Note:</b> The {@code defaultReturnValue} is fixed at 0. Calling + * {@link #defaultReturnValue(long)} will throw {@link UnsupportedOperationException} + * because it cannot be propagated to the underlying segment maps consistently. + * + * <p><b>Important:</b> All compound operations from both {@link Long2LongMap} and {@link Map} + * interfaces (computeIfAbsent, computeIfPresent, compute, merge, mergeLong, putIfAbsent, + * replace, remove) are overridden to ensure atomicity within a segment. + */ Review Comment: The Javadoc claims compound operations from both `Long2LongMap` and `Map` are overridden for atomicity, but only primitive-key overloads are present for several operations (e.g. `computeIfPresent(long, ...)`, `compute(long, ...)`, `merge(long, ...)`, `putIfAbsent(long, ...)`, `replace(long, ...)`). If this map is used via a `Map<Long,Long>` reference, boxed `Map` default methods may be invoked instead and will not be atomic under a single segment lock. Add boxed overrides (`putIfAbsent(Long,Long)`, `computeIfPresent(Long,BiFunction)`, `compute(Long,BiFunction)`, `merge(Long,Long,BiFunction)`, `replace(...)`, etc.) delegating to the locked implementations. ########## fe/fe-core/src/test/java/org/apache/doris/common/util/ConcurrentLong2ObjectHashMapTest.java: ########## @@ -0,0 +1,432 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; +import it.unimi.dsi.fastutil.longs.Long2ObjectMap; Review Comment: The file is located under `org/apache/doris/common/util`, but the declared package is `org.apache.doris.foundation.util`. This is inconsistent with the surrounding test layout and may violate build/style checks. Either move the test to `src/test/java/org/apache/doris/foundation/util/` or change the package to `org.apache.doris.common.util` and import the map under test. ########## fe/fe-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2LongHashMap.java: ########## @@ -0,0 +1,526 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import it.unimi.dsi.fastutil.longs.AbstractLong2LongMap; +import it.unimi.dsi.fastutil.longs.Long2LongFunction; +import it.unimi.dsi.fastutil.longs.Long2LongMap; +import it.unimi.dsi.fastutil.longs.Long2LongOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import it.unimi.dsi.fastutil.longs.LongSet; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import it.unimi.dsi.fastutil.objects.ObjectSet; + +import java.util.Map; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongBinaryOperator; +import java.util.function.LongUnaryOperator; + +/** + * A concurrent map with primitive long keys and primitive long values, backed by segmented + * {@link Long2LongOpenHashMap} instances with {@link ReentrantReadWriteLock} per segment. + * + * <p>This class saves ~48 bytes per entry compared to {@code ConcurrentHashMap<Long, Long>} + * by avoiding boxing of both keys and values. For fields like partition update row counts + * with millions of entries, this translates to hundreds of MB of heap savings. + * + * <p>The {@link #addTo(long, long)} method provides atomic increment semantics, useful for + * counter patterns. + * + * <p><b>Note:</b> The {@code defaultReturnValue} is fixed at 0. Calling + * {@link #defaultReturnValue(long)} will throw {@link UnsupportedOperationException} + * because it cannot be propagated to the underlying segment maps consistently. + * + * <p><b>Important:</b> All compound operations from both {@link Long2LongMap} and {@link Map} + * interfaces (computeIfAbsent, computeIfPresent, compute, merge, mergeLong, putIfAbsent, + * replace, remove) are overridden to ensure atomicity within a segment. + */ +public class ConcurrentLong2LongHashMap extends AbstractLong2LongMap { + + private static final int DEFAULT_SEGMENT_COUNT = 16; + private static final int DEFAULT_INITIAL_CAPACITY_PER_SEGMENT = 16; + + private final Segment[] segments; + private final int segmentMask; + private final int segmentBits; + + public ConcurrentLong2LongHashMap() { + this(DEFAULT_SEGMENT_COUNT); + } + + public ConcurrentLong2LongHashMap(int segmentCount) { + if (segmentCount <= 0 || (segmentCount & (segmentCount - 1)) != 0) { + throw new IllegalArgumentException("segmentCount must be a positive power of 2: " + segmentCount); + } + this.segmentBits = Integer.numberOfTrailingZeros(segmentCount); + this.segmentMask = segmentCount - 1; + this.segments = new Segment[segmentCount]; + for (int i = 0; i < segmentCount; i++) { + segments[i] = new Segment(DEFAULT_INITIAL_CAPACITY_PER_SEGMENT); + } + } + + @Override + public void defaultReturnValue(long rv) { + throw new UnsupportedOperationException( + "ConcurrentLong2LongHashMap does not support changing defaultReturnValue. " + + "It is fixed at 0."); + } + + /** Murmur3 64-bit finalizer for segment selection. */ + private static long mix(long x) { + x ^= x >>> 33; + x *= 0xff51afd7ed558ccdL; + x ^= x >>> 33; + x *= 0xc4ceb9fe1a85ec53L; + x ^= x >>> 33; + return x; + } + + private Segment segmentFor(long key) { + return segments[(int) (mix(key) >>> (64 - segmentBits)) & segmentMask]; + } + + // ---- Read operations (read-lock) ---- + + @Override + public long get(long key) { + Segment seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.get(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + public long getOrDefault(long key, long defaultValue) { + Segment seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.containsKey(key) ? seg.map.get(key) : defaultValue; + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsKey(long key) { + Segment seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.containsKey(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsValue(long value) { + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + if (seg.map.containsValue(value)) { + return true; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return false; + } + + @Override + public int size() { + long total = 0; + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + total += seg.map.size(); + } finally { + seg.lock.readLock().unlock(); + } + } + return (int) Math.min(total, Integer.MAX_VALUE); + } + + @Override + public boolean isEmpty() { + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + if (!seg.map.isEmpty()) { + return false; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return true; + } + + // ---- Write operations (write-lock) ---- + + @Override + public long put(long key, long value) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.put(key, value); + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public long remove(long key) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.remove(key); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long putIfAbsent(long key, long value) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.get(key); + } + seg.map.put(key, value); + return defaultReturnValue(); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public boolean replace(long key, long oldValue, long newValue) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key) && seg.map.get(key) == oldValue) { + seg.map.put(key, newValue); + return true; + } + return false; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long replace(long key, long value) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.put(key, value); + } + return defaultReturnValue(); + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public boolean remove(Object key, Object value) { + if (!(key instanceof Long) || !(value instanceof Long)) { + return false; + } + long k = (Long) key; + long v = (Long) value; + Segment seg = segmentFor(k); + seg.lock.writeLock().lock(); + try { + if (!seg.map.containsKey(k) || seg.map.get(k) != v) { + return false; + } + seg.map.remove(k); + return true; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public void clear() { + for (Segment seg : segments) { + seg.lock.writeLock().lock(); + try { + seg.map.clear(); + } finally { + seg.lock.writeLock().unlock(); + } + } + } + + @Override + public void putAll(Map<? extends Long, ? extends Long> m) { + for (Map.Entry<? extends Long, ? extends Long> entry : m.entrySet()) { + put(entry.getKey().longValue(), entry.getValue().longValue()); + } + } + + // ---- Atomic compound operations ---- + + /** + * Atomically adds the given increment to the value associated with the key. + * If the key is not present, the entry is created with the increment as value + * (starting from defaultReturnValue, which is 0L by default). + * + * @return the new value after the increment + */ + public long addTo(long key, long increment) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + long newValue = seg.map.addTo(key, increment) + increment; + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long computeIfAbsent(long key, LongUnaryOperator mappingFunction) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.get(key); + } + long newValue = mappingFunction.applyAsLong(key); + seg.map.put(key, newValue); + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long computeIfAbsent(long key, Long2LongFunction mappingFunction) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.get(key); + } + long newValue = mappingFunction.get(key); + seg.map.put(key, newValue); + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public Long computeIfAbsent(Long key, Function<? super Long, ? extends Long> mappingFunction) { + long k = key.longValue(); + Segment seg = segmentFor(k); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(k)) { + return seg.map.get(k); + } + Long newValue = mappingFunction.apply(key); + if (newValue != null) { + seg.map.put(k, newValue.longValue()); + } + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long computeIfPresent(long key, + BiFunction<? super Long, ? super Long, ? extends Long> remappingFunction) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (!seg.map.containsKey(key)) { + return defaultReturnValue(); + } + long oldValue = seg.map.get(key); + Long newValue = remappingFunction.apply(key, oldValue); + if (newValue != null) { + seg.map.put(key, newValue.longValue()); + return newValue; + } else { + seg.map.remove(key); + return defaultReturnValue(); + } + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long compute(long key, BiFunction<? super Long, ? super Long, ? extends Long> remappingFunction) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + Long oldValue = seg.map.containsKey(key) ? seg.map.get(key) : null; + Long newValue = remappingFunction.apply(key, oldValue); + if (newValue != null) { + seg.map.put(key, newValue.longValue()); + return newValue; + } else if (oldValue != null) { + seg.map.remove(key); + } + return defaultReturnValue(); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long merge(long key, long value, + BiFunction<? super Long, ? super Long, ? extends Long> remappingFunction) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (!seg.map.containsKey(key)) { + seg.map.put(key, value); + return value; + } + long oldValue = seg.map.get(key); + Long newValue = remappingFunction.apply(oldValue, value); + if (newValue != null) { + seg.map.put(key, newValue.longValue()); + return newValue; + } else { + seg.map.remove(key); + return defaultReturnValue(); + } + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long mergeLong(long key, long value, LongBinaryOperator remappingFunction) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (!seg.map.containsKey(key)) { + seg.map.put(key, value); + return value; + } + long oldValue = seg.map.get(key); + long newValue = remappingFunction.applyAsLong(oldValue, value); + seg.map.put(key, newValue); + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + // ---- Iteration (weakly consistent snapshots) ---- + + @Override + public ObjectSet<Long2LongMap.Entry> long2LongEntrySet() { + ObjectOpenHashSet<Long2LongMap.Entry> snapshot = new ObjectOpenHashSet<>(); + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + for (Long2LongMap.Entry entry : seg.map.long2LongEntrySet()) { + snapshot.add(new AbstractLong2LongMap.BasicEntry(entry.getLongKey(), entry.getLongValue())); + } + } finally { + seg.lock.readLock().unlock(); + } + } + return snapshot; + } Review Comment: This class also returns snapshot copies from `long2LongEntrySet()`, `keySet()`, and `values()`. Like the object-map variant, that violates the `Map` contract that these are live views backed by the map and can break callers that rely on `keySet().remove(...)` / iterator removals. Consider providing live view implementations for the standard methods and separate snapshot helpers if needed. ########## fe/fe-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2ObjectHashMap.java: ########## @@ -0,0 +1,487 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import it.unimi.dsi.fastutil.longs.AbstractLong2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectFunction; +import it.unimi.dsi.fastutil.longs.Long2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import it.unimi.dsi.fastutil.longs.LongSet; +import it.unimi.dsi.fastutil.objects.ObjectArrayList; +import it.unimi.dsi.fastutil.objects.ObjectCollection; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import it.unimi.dsi.fastutil.objects.ObjectSet; + +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongFunction; + +/** + * A concurrent map with primitive long keys and object values, backed by segmented + * {@link Long2ObjectOpenHashMap} instances with {@link ReentrantReadWriteLock} per segment. + * + * <p>This class provides similar concurrency guarantees to {@link java.util.concurrent.ConcurrentHashMap} + * while avoiding the memory overhead of boxing long keys. For a cluster with millions of tablet entries, + * this saves ~32 bytes per entry compared to {@code ConcurrentHashMap<Long, V>}. + * + * <p>Like {@link java.util.concurrent.ConcurrentHashMap}, null values are not permitted. + * + * <p>Iteration methods ({@link #long2ObjectEntrySet()}, {@link #keySet()}, {@link #values()}) + * return snapshot copies and are weakly consistent. + * + * <p><b>Important:</b> All compound operations from both {@link Long2ObjectMap} and {@link Map} + * interfaces (computeIfAbsent, computeIfPresent, compute, merge, putIfAbsent, replace, remove) + * are overridden to ensure atomicity within a segment. + * + * @param <V> the type of mapped values + */ +public class ConcurrentLong2ObjectHashMap<V> extends AbstractLong2ObjectMap<V> { + + private static final int DEFAULT_SEGMENT_COUNT = 16; + private static final int DEFAULT_INITIAL_CAPACITY_PER_SEGMENT = 16; + + private final Segment<V>[] segments; + private final int segmentMask; + private final int segmentBits; + + public ConcurrentLong2ObjectHashMap() { + this(DEFAULT_SEGMENT_COUNT); + } + + @SuppressWarnings("unchecked") + public ConcurrentLong2ObjectHashMap(int segmentCount) { + if (segmentCount <= 0 || (segmentCount & (segmentCount - 1)) != 0) { + throw new IllegalArgumentException("segmentCount must be a positive power of 2: " + segmentCount); + } + this.segmentBits = Integer.numberOfTrailingZeros(segmentCount); + this.segmentMask = segmentCount - 1; + this.segments = new Segment[segmentCount]; + for (int i = 0; i < segmentCount; i++) { + segments[i] = new Segment<>(DEFAULT_INITIAL_CAPACITY_PER_SEGMENT); + } + } + + /** Murmur3 64-bit finalizer for segment selection. */ + private static long mix(long x) { + x ^= x >>> 33; + x *= 0xff51afd7ed558ccdL; + x ^= x >>> 33; + x *= 0xc4ceb9fe1a85ec53L; + x ^= x >>> 33; + return x; + } + + private Segment<V> segmentFor(long key) { + return segments[(int) (mix(key) >>> (64 - segmentBits)) & segmentMask]; + } + + // ---- Read operations (read-lock) ---- + + @Override + public V get(long key) { + Segment<V> seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.get(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + public V getOrDefault(long key, V defaultValue) { + Segment<V> seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + V val = seg.map.get(key); + return (val != null || seg.map.containsKey(key)) ? val : defaultValue; + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsKey(long key) { + Segment<V> seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.containsKey(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsValue(Object value) { + for (Segment<V> seg : segments) { + seg.lock.readLock().lock(); + try { + if (seg.map.containsValue(value)) { + return true; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return false; + } + + @Override + public int size() { + long total = 0; + for (Segment<V> seg : segments) { + seg.lock.readLock().lock(); + try { + total += seg.map.size(); + } finally { + seg.lock.readLock().unlock(); + } + } + return (int) Math.min(total, Integer.MAX_VALUE); + } + + @Override + public boolean isEmpty() { + for (Segment<V> seg : segments) { + seg.lock.readLock().lock(); + try { + if (!seg.map.isEmpty()) { + return false; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return true; + } + + // ---- Write operations (write-lock) ---- + + @Override + public V put(long key, V value) { + Objects.requireNonNull(value, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.put(key, value); + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public V remove(long key) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.remove(key); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V putIfAbsent(long key, V value) { + Objects.requireNonNull(value, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V existing = seg.map.get(key); + if (existing != null || seg.map.containsKey(key)) { + return existing; + } + seg.map.put(key, value); + return null; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public boolean replace(long key, V oldValue, V newValue) { + Objects.requireNonNull(newValue, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V curValue = seg.map.get(key); + if (!Objects.equals(curValue, oldValue) || (curValue == null && !seg.map.containsKey(key))) { + return false; + } + seg.map.put(key, newValue); + return true; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V replace(long key, V value) { + Objects.requireNonNull(value, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.put(key, value); + } + return null; + } finally { + seg.lock.writeLock().unlock(); + } + } Review Comment: Only the primitive overloads of `putIfAbsent`/`replace` are implemented here. Because `Map.putIfAbsent`/`Map.replace` are default methods, callers using a `Map<Long,V>` reference will otherwise get the non-atomic default implementations. Add boxed overrides (`putIfAbsent(Long,V)`, `replace(Long,V)`, `replace(Long,V,V)`) that delegate to these segment-locked primitive methods. ########## fe/fe-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2ObjectHashMap.java: ########## @@ -0,0 +1,487 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import it.unimi.dsi.fastutil.longs.AbstractLong2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectFunction; +import it.unimi.dsi.fastutil.longs.Long2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import it.unimi.dsi.fastutil.longs.LongSet; +import it.unimi.dsi.fastutil.objects.ObjectArrayList; +import it.unimi.dsi.fastutil.objects.ObjectCollection; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import it.unimi.dsi.fastutil.objects.ObjectSet; + +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongFunction; + +/** + * A concurrent map with primitive long keys and object values, backed by segmented + * {@link Long2ObjectOpenHashMap} instances with {@link ReentrantReadWriteLock} per segment. + * + * <p>This class provides similar concurrency guarantees to {@link java.util.concurrent.ConcurrentHashMap} + * while avoiding the memory overhead of boxing long keys. For a cluster with millions of tablet entries, + * this saves ~32 bytes per entry compared to {@code ConcurrentHashMap<Long, V>}. + * + * <p>Like {@link java.util.concurrent.ConcurrentHashMap}, null values are not permitted. + * + * <p>Iteration methods ({@link #long2ObjectEntrySet()}, {@link #keySet()}, {@link #values()}) + * return snapshot copies and are weakly consistent. + * + * <p><b>Important:</b> All compound operations from both {@link Long2ObjectMap} and {@link Map} + * interfaces (computeIfAbsent, computeIfPresent, compute, merge, putIfAbsent, replace, remove) + * are overridden to ensure atomicity within a segment. + * + * @param <V> the type of mapped values + */ +public class ConcurrentLong2ObjectHashMap<V> extends AbstractLong2ObjectMap<V> { + + private static final int DEFAULT_SEGMENT_COUNT = 16; + private static final int DEFAULT_INITIAL_CAPACITY_PER_SEGMENT = 16; + + private final Segment<V>[] segments; + private final int segmentMask; + private final int segmentBits; + + public ConcurrentLong2ObjectHashMap() { + this(DEFAULT_SEGMENT_COUNT); + } + + @SuppressWarnings("unchecked") + public ConcurrentLong2ObjectHashMap(int segmentCount) { + if (segmentCount <= 0 || (segmentCount & (segmentCount - 1)) != 0) { + throw new IllegalArgumentException("segmentCount must be a positive power of 2: " + segmentCount); + } + this.segmentBits = Integer.numberOfTrailingZeros(segmentCount); + this.segmentMask = segmentCount - 1; + this.segments = new Segment[segmentCount]; + for (int i = 0; i < segmentCount; i++) { + segments[i] = new Segment<>(DEFAULT_INITIAL_CAPACITY_PER_SEGMENT); + } + } + + /** Murmur3 64-bit finalizer for segment selection. */ + private static long mix(long x) { + x ^= x >>> 33; + x *= 0xff51afd7ed558ccdL; + x ^= x >>> 33; + x *= 0xc4ceb9fe1a85ec53L; + x ^= x >>> 33; + return x; + } + + private Segment<V> segmentFor(long key) { + return segments[(int) (mix(key) >>> (64 - segmentBits)) & segmentMask]; + } + + // ---- Read operations (read-lock) ---- + + @Override + public V get(long key) { + Segment<V> seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.get(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + public V getOrDefault(long key, V defaultValue) { + Segment<V> seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + V val = seg.map.get(key); + return (val != null || seg.map.containsKey(key)) ? val : defaultValue; + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsKey(long key) { + Segment<V> seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.containsKey(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsValue(Object value) { + for (Segment<V> seg : segments) { + seg.lock.readLock().lock(); + try { + if (seg.map.containsValue(value)) { + return true; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return false; + } + + @Override + public int size() { + long total = 0; + for (Segment<V> seg : segments) { + seg.lock.readLock().lock(); + try { + total += seg.map.size(); + } finally { + seg.lock.readLock().unlock(); + } + } + return (int) Math.min(total, Integer.MAX_VALUE); + } + + @Override + public boolean isEmpty() { + for (Segment<V> seg : segments) { + seg.lock.readLock().lock(); + try { + if (!seg.map.isEmpty()) { + return false; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return true; + } + + // ---- Write operations (write-lock) ---- + + @Override + public V put(long key, V value) { + Objects.requireNonNull(value, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.put(key, value); + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public V remove(long key) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.remove(key); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V putIfAbsent(long key, V value) { + Objects.requireNonNull(value, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V existing = seg.map.get(key); + if (existing != null || seg.map.containsKey(key)) { + return existing; + } + seg.map.put(key, value); + return null; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public boolean replace(long key, V oldValue, V newValue) { + Objects.requireNonNull(newValue, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V curValue = seg.map.get(key); + if (!Objects.equals(curValue, oldValue) || (curValue == null && !seg.map.containsKey(key))) { + return false; + } + seg.map.put(key, newValue); + return true; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V replace(long key, V value) { + Objects.requireNonNull(value, "Null values are not permitted"); + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.put(key, value); + } + return null; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public boolean remove(Object key, Object value) { + if (!(key instanceof Long)) { + return false; + } + long k = (Long) key; + Segment<V> seg = segmentFor(k); + seg.lock.writeLock().lock(); + try { + V curValue = seg.map.get(k); + if (!Objects.equals(curValue, value) || (curValue == null && !seg.map.containsKey(k))) { + return false; + } + seg.map.remove(k); + return true; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public void clear() { + for (Segment<V> seg : segments) { + seg.lock.writeLock().lock(); + try { + seg.map.clear(); + } finally { + seg.lock.writeLock().unlock(); + } + } + } + + @Override + public void putAll(Map<? extends Long, ? extends V> m) { + for (Map.Entry<? extends Long, ? extends V> entry : m.entrySet()) { + put(entry.getKey().longValue(), entry.getValue()); + } + } + + // ---- Atomic compound operations ---- + // Override ALL compound methods from both Long2ObjectMap and Map interfaces + // to ensure the check-then-act is atomic within a segment's write lock. + + public V computeIfAbsent(long key, LongFunction<? extends V> mappingFunction) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V val = seg.map.get(key); + if (val != null || seg.map.containsKey(key)) { + return val; + } + V newValue = mappingFunction.apply(key); + if (newValue != null) { + seg.map.put(key, newValue); + } + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public V computeIfAbsent(long key, Long2ObjectFunction<? extends V> mappingFunction) { + Segment<V> seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + V val = seg.map.get(key); + if (val != null || seg.map.containsKey(key)) { + return val; + } + V newValue = mappingFunction.get(key); + if (newValue != null) { + seg.map.put(key, newValue); + } + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public V computeIfAbsent(Long key, Function<? super Long, ? extends V> mappingFunction) { + return computeIfAbsent(key.longValue(), (long k) -> mappingFunction.apply(k)); + } + Review Comment: `computeIfPresent`, `compute`, and `merge` are only implemented as primitive-key overloads here. If callers use this class via `Map<Long,V>`, the boxed `Map` default implementations can be invoked instead, which won’t be atomic under a single segment write lock. Add boxed overrides (`computeIfPresent(Long,...)`, `compute(Long,...)`, `merge(Long,...)`) that delegate to the segment-locked primitive implementations (and enforce the same null-value rules). ########## fe/fe-foundation/src/main/java/org/apache/doris/foundation/util/ConcurrentLong2LongHashMap.java: ########## @@ -0,0 +1,526 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.foundation.util; + +import it.unimi.dsi.fastutil.longs.AbstractLong2LongMap; +import it.unimi.dsi.fastutil.longs.Long2LongFunction; +import it.unimi.dsi.fastutil.longs.Long2LongMap; +import it.unimi.dsi.fastutil.longs.Long2LongOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongOpenHashSet; +import it.unimi.dsi.fastutil.longs.LongSet; +import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import it.unimi.dsi.fastutil.objects.ObjectSet; + +import java.util.Map; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongBinaryOperator; +import java.util.function.LongUnaryOperator; + +/** + * A concurrent map with primitive long keys and primitive long values, backed by segmented + * {@link Long2LongOpenHashMap} instances with {@link ReentrantReadWriteLock} per segment. + * + * <p>This class saves ~48 bytes per entry compared to {@code ConcurrentHashMap<Long, Long>} + * by avoiding boxing of both keys and values. For fields like partition update row counts + * with millions of entries, this translates to hundreds of MB of heap savings. + * + * <p>The {@link #addTo(long, long)} method provides atomic increment semantics, useful for + * counter patterns. + * + * <p><b>Note:</b> The {@code defaultReturnValue} is fixed at 0. Calling + * {@link #defaultReturnValue(long)} will throw {@link UnsupportedOperationException} + * because it cannot be propagated to the underlying segment maps consistently. + * + * <p><b>Important:</b> All compound operations from both {@link Long2LongMap} and {@link Map} + * interfaces (computeIfAbsent, computeIfPresent, compute, merge, mergeLong, putIfAbsent, + * replace, remove) are overridden to ensure atomicity within a segment. + */ +public class ConcurrentLong2LongHashMap extends AbstractLong2LongMap { + + private static final int DEFAULT_SEGMENT_COUNT = 16; + private static final int DEFAULT_INITIAL_CAPACITY_PER_SEGMENT = 16; + + private final Segment[] segments; + private final int segmentMask; + private final int segmentBits; + + public ConcurrentLong2LongHashMap() { + this(DEFAULT_SEGMENT_COUNT); + } + + public ConcurrentLong2LongHashMap(int segmentCount) { + if (segmentCount <= 0 || (segmentCount & (segmentCount - 1)) != 0) { + throw new IllegalArgumentException("segmentCount must be a positive power of 2: " + segmentCount); + } + this.segmentBits = Integer.numberOfTrailingZeros(segmentCount); + this.segmentMask = segmentCount - 1; + this.segments = new Segment[segmentCount]; + for (int i = 0; i < segmentCount; i++) { + segments[i] = new Segment(DEFAULT_INITIAL_CAPACITY_PER_SEGMENT); + } + } + + @Override + public void defaultReturnValue(long rv) { + throw new UnsupportedOperationException( + "ConcurrentLong2LongHashMap does not support changing defaultReturnValue. " + + "It is fixed at 0."); + } + + /** Murmur3 64-bit finalizer for segment selection. */ + private static long mix(long x) { + x ^= x >>> 33; + x *= 0xff51afd7ed558ccdL; + x ^= x >>> 33; + x *= 0xc4ceb9fe1a85ec53L; + x ^= x >>> 33; + return x; + } + + private Segment segmentFor(long key) { + return segments[(int) (mix(key) >>> (64 - segmentBits)) & segmentMask]; + } + + // ---- Read operations (read-lock) ---- + + @Override + public long get(long key) { + Segment seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.get(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + public long getOrDefault(long key, long defaultValue) { + Segment seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.containsKey(key) ? seg.map.get(key) : defaultValue; + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsKey(long key) { + Segment seg = segmentFor(key); + seg.lock.readLock().lock(); + try { + return seg.map.containsKey(key); + } finally { + seg.lock.readLock().unlock(); + } + } + + @Override + public boolean containsValue(long value) { + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + if (seg.map.containsValue(value)) { + return true; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return false; + } + + @Override + public int size() { + long total = 0; + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + total += seg.map.size(); + } finally { + seg.lock.readLock().unlock(); + } + } + return (int) Math.min(total, Integer.MAX_VALUE); + } + + @Override + public boolean isEmpty() { + for (Segment seg : segments) { + seg.lock.readLock().lock(); + try { + if (!seg.map.isEmpty()) { + return false; + } + } finally { + seg.lock.readLock().unlock(); + } + } + return true; + } + + // ---- Write operations (write-lock) ---- + + @Override + public long put(long key, long value) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.put(key, value); + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public long remove(long key) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + return seg.map.remove(key); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long putIfAbsent(long key, long value) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.get(key); + } + seg.map.put(key, value); + return defaultReturnValue(); + } finally { + seg.lock.writeLock().unlock(); + } + } + + public boolean replace(long key, long oldValue, long newValue) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key) && seg.map.get(key) == oldValue) { + seg.map.put(key, newValue); + return true; + } + return false; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long replace(long key, long value) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.put(key, value); + } + return defaultReturnValue(); + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public boolean remove(Object key, Object value) { + if (!(key instanceof Long) || !(value instanceof Long)) { + return false; + } + long k = (Long) key; + long v = (Long) value; + Segment seg = segmentFor(k); + seg.lock.writeLock().lock(); + try { + if (!seg.map.containsKey(k) || seg.map.get(k) != v) { + return false; + } + seg.map.remove(k); + return true; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public void clear() { + for (Segment seg : segments) { + seg.lock.writeLock().lock(); + try { + seg.map.clear(); + } finally { + seg.lock.writeLock().unlock(); + } + } + } + + @Override + public void putAll(Map<? extends Long, ? extends Long> m) { + for (Map.Entry<? extends Long, ? extends Long> entry : m.entrySet()) { + put(entry.getKey().longValue(), entry.getValue().longValue()); + } + } + + // ---- Atomic compound operations ---- + + /** + * Atomically adds the given increment to the value associated with the key. + * If the key is not present, the entry is created with the increment as value + * (starting from defaultReturnValue, which is 0L by default). + * + * @return the new value after the increment + */ + public long addTo(long key, long increment) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + long newValue = seg.map.addTo(key, increment) + increment; + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long computeIfAbsent(long key, LongUnaryOperator mappingFunction) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.get(key); + } + long newValue = mappingFunction.applyAsLong(key); + seg.map.put(key, newValue); + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + public long computeIfAbsent(long key, Long2LongFunction mappingFunction) { + Segment seg = segmentFor(key); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(key)) { + return seg.map.get(key); + } + long newValue = mappingFunction.get(key); + seg.map.put(key, newValue); + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } + + @Override + public Long computeIfAbsent(Long key, Function<? super Long, ? extends Long> mappingFunction) { + long k = key.longValue(); + Segment seg = segmentFor(k); + seg.lock.writeLock().lock(); + try { + if (seg.map.containsKey(k)) { + return seg.map.get(k); + } + Long newValue = mappingFunction.apply(key); + if (newValue != null) { + seg.map.put(k, newValue.longValue()); + } + return newValue; + } finally { + seg.lock.writeLock().unlock(); + } + } Review Comment: For `computeIfPresent`, `compute`, and `merge`, only primitive-returning overloads are provided. When used through a `Map<Long,Long>` reference, the boxed `Map` default methods may be called and will not be atomic. Add boxed overrides for these methods that implement standard `Map` semantics (returning `null` when absent/removed) while still taking the segment write lock. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
