From 6ed6f6b30532e94fad5d3fd92a1920700c1c5de2 Mon Sep 17 00:00:00 2001 From: NSAmelchev Date: Mon, 19 Oct 2026 18:22:29 +0300 Subject: [PATCH] IGNITE-29169 Use CopyOnWriteIntMap for rarely updated maps on cache hot paths --- .../cache/GridCacheConcurrentMapImpl.java | 8 +-- .../processors/cache/GridCacheContext.java | 12 ---- .../cache/GridCacheLocalConcurrentMap.java | 2 +- .../cache/IgniteCacheOffheapManagerImpl.java | 4 +- .../CacheObjectBinaryProcessorImpl.java | 4 +- .../dht/topology/GridDhtLocalPartition.java | 10 ++-- .../persistence/DataRegionMetricsImpl.java | 57 ++++--------------- .../util/collection/CopyOnWriteIntMap.java | 9 ++- 8 files changed, 35 insertions(+), 71 deletions(-) diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheConcurrentMapImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheConcurrentMapImpl.java index 471a17ecfb55e..f860030414f8c 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheConcurrentMapImpl.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheConcurrentMapImpl.java @@ -53,7 +53,7 @@ public GridCacheConcurrentMapImpl(GridCacheMapEntryFactory factory) { /** {@inheritDoc} */ @Nullable @Override public GridCacheMapEntry getEntry(GridCacheContext ctx, KeyCacheObject key) { - CacheMapHolder hld = entriesMapIfExists(ctx.cacheIdBoxed()); + CacheMapHolder hld = entriesMapIfExists(ctx.cacheId()); key = (KeyCacheObject)ctx.kernalContext().cacheObjects().prepareForCache(key, ctx); @@ -87,7 +87,7 @@ protected final GridCacheMapEntry putEntryIfObsoleteOrAbsent( final boolean skipReserve ) { if (hld == null) - hld = entriesMapIfExists(ctx.cacheIdBoxed()); + hld = entriesMapIfExists(ctx.cacheId()); GridCacheMapEntry cur = null; GridCacheMapEntry created = null; @@ -247,7 +247,7 @@ else if (sizeChange == -1) * @param cacheId Cache ID. * @return Map for given cache ID. */ - @Nullable protected abstract CacheMapHolder entriesMapIfExists(Integer cacheId); + @Nullable protected abstract CacheMapHolder entriesMapIfExists(int cacheId); /** * @@ -279,7 +279,7 @@ else if (sizeChange == -1) @Override public boolean removeEntry(final GridCacheEntryEx entry) { GridCacheContext ctx = entry.context(); - CacheMapHolder hld = entriesMapIfExists(ctx.cacheIdBoxed()); + CacheMapHolder hld = entriesMapIfExists(ctx.cacheId()); boolean rmv = hld != null && hld.map.remove(entry.key(), entry); diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheContext.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheContext.java index 90893e6e09886..68843a88050f5 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheContext.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheContext.java @@ -223,9 +223,6 @@ public class GridCacheContext implements Externalizable { /** Cache ID. */ private int cacheId; - /** Cache ID. */ - private Integer cacheIdBoxed; - /** Cache type. */ private CacheType cacheType; @@ -404,8 +401,6 @@ public GridCacheContext( cacheId = CU.cacheId(cacheName); - cacheIdBoxed = cacheId; - plc = cacheType.ioPolicy(); Factory factory = cacheCfg.getExpiryPolicyFactory(); @@ -596,13 +591,6 @@ public int cacheId() { return cacheId; } - /** - * @return Cache ID. - */ - public Integer cacheIdBoxed() { - return cacheIdBoxed; - } - /** * @return {@code True} if should use system transactions which are isolated from user transactions. */ diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheLocalConcurrentMap.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheLocalConcurrentMap.java index b4c93cf9db1a3..2438d816304e0 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheLocalConcurrentMap.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheLocalConcurrentMap.java @@ -55,7 +55,7 @@ public GridCacheLocalConcurrentMap(GridCacheContext cctx, GridCacheMapEntryFacto } /** {@inheritDoc} */ - @Nullable @Override protected CacheMapHolder entriesMapIfExists(Integer cacheId) { + @Nullable @Override protected CacheMapHolder entriesMapIfExists(int cacheId) { return entryMap; } diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java index c97aefd68213f..4be5a225b43dc 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java @@ -76,9 +76,9 @@ import org.apache.ignite.internal.util.GridLongList; import org.apache.ignite.internal.util.GridSpinBusyLock; import org.apache.ignite.internal.util.GridStripedLock; +import org.apache.ignite.internal.util.collection.CopyOnWriteIntMap; import org.apache.ignite.internal.util.collection.ImmutableIntSet; import org.apache.ignite.internal.util.collection.IntMap; -import org.apache.ignite.internal.util.collection.IntRWHashMap; import org.apache.ignite.internal.util.collection.IntSet; import org.apache.ignite.internal.util.lang.GridCloseableIterator; import org.apache.ignite.internal.util.lang.GridCursor; @@ -1226,7 +1226,7 @@ public static class CacheDataStoreImpl implements CacheDataStore { private final LongAdder storageSize = new LongAdder(); /** */ - private final IntMap cacheSizes = new IntRWHashMap<>(); + private final IntMap cacheSizes = new CopyOnWriteIntMap<>(); /** */ private final IgniteLogger log; diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessorImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessorImpl.java index 1d2dd384531e5..cab90a3f30d13 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessorImpl.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessorImpl.java @@ -86,6 +86,8 @@ import org.apache.ignite.internal.processors.query.QueryUtils; import org.apache.ignite.internal.systemview.BinaryMetadataViewWalker; import org.apache.ignite.internal.util.MutableSingletonList; +import org.apache.ignite.internal.util.collection.CopyOnWriteIntMap; +import org.apache.ignite.internal.util.collection.IntMap; import org.apache.ignite.internal.util.future.GridFutureAdapter; import org.apache.ignite.internal.util.lang.GridMapEntry; import org.apache.ignite.internal.util.tostring.GridToStringExclude; @@ -168,7 +170,7 @@ public class CacheObjectBinaryProcessorImpl extends GridProcessorAdapter impleme private BinaryMetadataTransport transport; /** Cached affinity key field names. */ - private final ConcurrentHashMap> affKeyFields = new ConcurrentHashMap<>(); + private final IntMap> affKeyFields = new CopyOnWriteIntMap<>(); /** Dummy {@code CacheObjectValueContext} used for mocking. */ private CacheObjectValueContext fakeCacheObjCtx; diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/topology/GridDhtLocalPartition.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/topology/GridDhtLocalPartition.java index 6fa941cfa9ae8..803b806aecf9f 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/topology/GridDhtLocalPartition.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/topology/GridDhtLocalPartition.java @@ -60,8 +60,8 @@ import org.apache.ignite.internal.processors.cache.transactions.TxCounters; import org.apache.ignite.internal.processors.cache.version.GridCacheVersion; import org.apache.ignite.internal.util.GridLongList; +import org.apache.ignite.internal.util.collection.CopyOnWriteIntMap; import org.apache.ignite.internal.util.collection.IntMap; -import org.apache.ignite.internal.util.collection.IntRWHashMap; import org.apache.ignite.internal.util.future.GridFinishedFuture; import org.apache.ignite.internal.util.future.GridFutureAdapter; import org.apache.ignite.internal.util.lang.GridIterator; @@ -198,7 +198,7 @@ public GridDhtLocalPartition( if (grp.sharedGroup()) { singleCacheEntryMap = null; - cacheMaps = new IntRWHashMap<>(); + cacheMaps = new CopyOnWriteIntMap<>(); } else { GridCacheContext cctx = grp.singleCacheContext(); @@ -276,7 +276,7 @@ private ConcurrentMap createEntriesMap() { } /** {@inheritDoc} */ - @Nullable @Override protected CacheMapHolder entriesMapIfExists(Integer cacheId) { + @Nullable @Override protected CacheMapHolder entriesMapIfExists(int cacheId) { return grp.sharedGroup() ? cacheMaps.get(cacheId) : singleCacheEntryMap; } @@ -287,7 +287,7 @@ private ConcurrentMap createEntriesMap() { private CacheMapHolder cacheMapHolder(GridCacheContext cctx) { assert grp.sharedGroup(); - CacheMapHolder hld = cacheMaps.get(cctx.cacheIdBoxed()); + CacheMapHolder hld = cacheMaps.get(cctx.cacheId()); if (hld != null) return hld; @@ -295,7 +295,7 @@ private CacheMapHolder cacheMapHolder(GridCacheContext cctx) { if (cctx.isNear()) cctx = cctx.near().dht().context(); - CacheMapHolder old = cacheMaps.putIfAbsent(cctx.cacheIdBoxed(), hld = ctx.kernalContext().resource().resolve( + CacheMapHolder old = cacheMaps.putIfAbsent(cctx.cacheId(), hld = ctx.kernalContext().resource().resolve( new CacheMapHolder(cctx, createEntriesMap()))); if (old != null) diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/DataRegionMetricsImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/DataRegionMetricsImpl.java index 3302e260808fa..18f85c9afc7b8 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/DataRegionMetricsImpl.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/DataRegionMetricsImpl.java @@ -36,7 +36,7 @@ import org.apache.ignite.internal.processors.metric.impl.LongAdderWithDelegateMetric; import org.apache.ignite.internal.processors.metric.impl.MetricUtils; import org.apache.ignite.internal.processors.metric.impl.PeriodicHistogramMetricImpl; -import org.apache.ignite.internal.util.collection.IntHashMap; +import org.apache.ignite.internal.util.collection.CopyOnWriteIntMap; import org.apache.ignite.internal.util.collection.IntMap; import org.apache.ignite.internal.util.typedef.internal.U; import org.apache.ignite.lang.IgniteBiTuple; @@ -95,16 +95,8 @@ private static LongAdderMetricDelegate delegate(LongAdderMetric delegate) { /** */ private final DataRegionMetricsProvider dataRegionMetricsProvider; - /** - * Cache group ID -> Cache group memory page metrics. - *

- * This is effectively a copy-on-write hash map, all write operations should create a new copy under the - * {@link #cacheGrpMetricsLock} lock. - */ - private volatile IntMap cacheGrpMetrics = new IntHashMap<>(); - - /** Lock for the {@link #cacheGrpMetrics} field. */ - private final Object cacheGrpMetricsLock = new Object(); + /** Cache group ID -> Cache group memory page metrics. */ + private final IntMap cacheGrpMetrics = new CopyOnWriteIntMap<>(); /** Memory page metrics for the whole region. */ private final PageMetrics dataRegionPageMetrics; @@ -606,31 +598,14 @@ public PageMetrics cacheGrpPageMetrics(int cacheGrpId) { if (pageMetrics != null) return pageMetrics; - synchronized (cacheGrpMetricsLock) { - IntMap locCacheGrpMetrics = cacheGrpMetrics; - - // double check - PageMetrics doubleCheckPageMetrics = locCacheGrpMetrics.get(cacheGrpId); - if (doubleCheckPageMetrics != null) - return doubleCheckPageMetrics; - - IntMap copy = new IntHashMap<>(locCacheGrpMetrics); - - PageMetrics newMetrics = Optional.of(kernalCtx) - // both cache and group descriptor can be null - .map(GridKernalContext::cache) - .map(cache -> cache.cacheGroupDescriptors().get(cacheGrpId)) - .map(decs -> createCacheGrpPageMetrics(decs.cacheOrGroupName())) - // return region-wide metrics for a non-existent cache group ID. This is needed in some scenarios - // when per-group data structures are used on a per-region basis and in tests. - .orElse(dataRegionPageMetrics); - - copy.put(cacheGrpId, newMetrics); - - cacheGrpMetrics = copy; - - return newMetrics; - } + return cacheGrpMetrics.computeIfAbsent(cacheGrpId, id -> Optional.of(kernalCtx) + // both cache and group descriptor can be null + .map(GridKernalContext::cache) + .map(cache -> cache.cacheGroupDescriptors().get(id)) + .map(decs -> createCacheGrpPageMetrics(decs.cacheOrGroupName())) + // return region-wide metrics for a non-existent cache group ID. This is needed in some scenarios + // when per-group data structures are used on a per-region basis and in tests. + .orElse(dataRegionPageMetrics)); } /** @@ -650,15 +625,7 @@ private PageMetrics createCacheGrpPageMetrics(String cacheGrpName) { * Removes all memory page metrics associated with the given cache group. */ public void removeCacheGrpPageMetrics(Integer grpId) { - PageMetrics rmvMetrics; - - synchronized (cacheGrpMetricsLock) { - IntMap copy = new IntHashMap<>(cacheGrpMetrics); - - rmvMetrics = copy.remove(grpId); - - cacheGrpMetrics = copy; - } + PageMetrics rmvMetrics = cacheGrpMetrics.remove(grpId); // we don't decrease the total pages counter for historical reasons // (it hasn't been done in previous implementations) diff --git a/modules/core/src/main/java/org/apache/ignite/internal/util/collection/CopyOnWriteIntMap.java b/modules/core/src/main/java/org/apache/ignite/internal/util/collection/CopyOnWriteIntMap.java index c648942fa5e8d..d1d536cfd249e 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/util/collection/CopyOnWriteIntMap.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/util/collection/CopyOnWriteIntMap.java @@ -107,7 +107,14 @@ public class CopyOnWriteIntMap implements IntMap { } /** {@inheritDoc} */ - @Override public synchronized @Nullable V computeIfAbsent(int key, IntFunction mappingFunction) { + @Override public @Nullable V computeIfAbsent(int key, IntFunction mappingFunction) { + V val = get(key); + + return val != null ? val : computeIfAbsentUnderLock(key, mappingFunction); + } + + /** Slow path of {@link #computeIfAbsent}: the mapping function runs under the map lock. */ + private synchronized V computeIfAbsentUnderLock(int key, IntFunction mappingFunction) { // The default implementation is a lookup followed by a put, the lock makes the pair atomic. return IntMap.super.computeIfAbsent(key, mappingFunction); }