This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new 89cd11d6d Fix thread contention in GenericVersionScheme and
WeakInternPool (#1937)
89cd11d6d is described below
commit 89cd11d6d1e073fd1d2de5399a0b9bd17aa9f863
Author: Guillaume Nodet <[email protected]>
AuthorDate: Sat Jun 27 16:03:50 2026 +0200
Fix thread contention in GenericVersionScheme and WeakInternPool (#1937)
## Problem
PR #1902 introduced `synchronized(versionCache)` and
`synchronized(this.map)` blocks around compound cache operations to fix a
thread-safety issue with `computeIfAbsent()` on `synchronizedMap(WeakHashMap)`.
While correct, this serialized all threads on hot paths, causing a **2x build
time regression** for Quarkus (~6min → ~12min).
### Root cause analysis
`Collections.synchronizedMap(new WeakHashMap<>())` wraps each individual
method (`get`, `put`) in `synchronized(mutex)`, but does NOT make compound
operations like `computeIfAbsent()` atomic. PR #1902 fixed this by adding outer
`synchronized` blocks — correct but serializing.
The original pre-#1902 code used separate `get()` + `put()` calls where
each individually acquires/releases the lock for microseconds. The compound
race is **benign for a cache** — at worst a duplicate value is created and one
wins the put.
## Solution
### 1. `ConcurrentWeakCache` — new lightweight concurrent cache
Introduces `ConcurrentWeakCache<K,V>` — a stripped-down version of
maven-impl's `Cache`, optimized for hot-path performance:
- **Lock-free reads** — `ConcurrentHashMap.get()` is a volatile read, no
lock acquisition
- **Zero allocation on `get()`** — uses a `ThreadLocal` reusable lookup key
instead of allocating a new wrapper per read (the main perf issue with the full
`Cache` class)
- **O(1) stale entry cleanup** — identity-based removal from
`ReferenceQueue` instead of `entrySet().removeIf()` full-map scan
- **Weak keys** — entries GC'd when key is no longer strongly referenced
(same as `WeakHashMap`)
- **Weak values** — values also held via `WeakReference`
### 2. `PathConflictResolver` memory explosion fix
The `PathConflictResolver` had an O(n²) memory issue from copying
`HashSet<Object> conflictIdsSinceRoot` at every graph node. Replaced with:
- Parent-chain walk (`hasConflictIdOnPathToRoot()`) — O(depth) per check,
no copying
- `LinkedHashSet` partitions — O(1) removal instead of O(n)
`ArrayList.remove()`
### 3. Tracking file read cache
`EnhancedLocalRepositoryManager.readRepos()` re-reads
`_remote.repositories` tracking files from disk (with file locking) on every
artifact resolution, even for artifacts in the same directory. In a primed
build, this causes thousands of redundant file reads with synchronized blocks
and FileLock acquisition.
Added a `ConcurrentHashMap<Path, Properties>` cache keyed by tracking file
path. Multiple artifacts in the same directory (e.g. jar + pom) share the same
tracking file, so the cache hit rate is high. The cache is invalidated (not
updated) on writes via `addRepo()` to avoid a race where two concurrent writes
could reorder their cache puts.
### 4. Lock-free fast path for `NamedLockFactorySupport`
`getLockAndRefTrack()` used `ConcurrentHashMap.compute()` on every call,
which takes a per-bucket exclusive lock even when the holder already exists. In
the common case (lock exists, just increment refcount), this serialized all
threads hashing to the same bucket.
Added a lock-free fast path: `ConcurrentHashMap.get()` (volatile read) +
`tryIncRef()` (CAS loop on `AtomicInteger`). Only falls back to `compute()`
when the holder is absent or being closed.
The close/acquire race is handled by a CAS sentinel: `closeLock()` marks
the holder as closed (CAS refcount `0 → MIN_VALUE`) before destroying it.
`tryIncRef()` rejects `refcount ≤ 0`, preventing revival of a destroyed lock.
### 5. `InhibitingNameMapper` stream→loop conversion
Replaced `Stream.filter().collect()` with imperative loops and added
empty-list short-circuit to skip the filtering entirely when no
`LockingInhibitor`s are registered (the common case).
### 6. `FileLockNamedLockFactory` FileChannel reuse
`destroyLock()` was closing the `FileChannel` on every lock release,
forcing a new `open()` syscall on every lock acquisition. Since the lock
factory is session-scoped, the channels can be kept open for reuse across lock
acquire/release cycles. Channels are now closed only during factory
`shutdown()`.
## Profiling results (async-profiler wall-clock, 5ms interval)
### End-to-end build time (Quarkus primed build, 8-thread)
| Configuration | Build time | vs Baseline |
|---|---|---|
| Baseline (2.0.19-SNAPSHOT) | 169.7s | — |
| + tracking file cache | ~160s | −6% |
| + lock-free getLockAndRefTrack | 149.1s | −12% |
| **+ FileChannel reuse + InhibitingNameMapper** | **93.5s** | **−45%** |
### Method-level sample comparison (baseline → all patches)
| Method | Baseline | Patched | Change |
|---|---|---|---|
| `EnhancedLocalRepositoryManager.find()` | 275 | 109 | **−60%** |
| `FileLockNamedLockFactory.createLock()` | 246 | 107 | **−56%** |
| `readRepos()` | 208 | 71 | **−66%** |
| `LegacyTrackingFileManager.read()` | 201 | 54 | **−73%** |
| `Retry.retry()` | 167 | 86 | **−49%** |
| `InhibitingNameMapper.nameLocks()` | 141 | 85 | **−40%** |
| `getLockAndRefTrack()` | 128 | 67 | **−48%** |
| `destroyLock()` | 46 | 0 | **−100%** |
| `mutex()` (String.intern lock) | 46 | 0 | **eliminated** |
### ConcurrentWeakCache microbenchmark (4 threads, 8M ops/iteration)
| Approach | Version Cache | Intern Pool |
|---|---|---|
| **Master** (synchronized blocks from #1902) | 1668ms (4.8M ops/sec) |
916ms (8.7M ops/sec) |
| Original synchronizedMap (pre-#1902) | 360ms (22.2M ops/sec) | 457ms
(17.5M ops/sec) |
| **ConcurrentWeakCache (this PR)** | **155ms (51.6M ops/sec)** | **176ms
(45.5M ops/sec)** |
**10.8x faster than master** for version parsing, **5.2x faster** for
artifact interning, while preserving weak reference semantics for GC-friendly
memory behavior.
## Files changed
- `ConcurrentWeakCache.java` — new lightweight concurrent cache with weak
keys/values
- `GenericVersionScheme.java` — use `ConcurrentWeakCache` for version
parsing cache
- `DataPool.java` — use `ConcurrentWeakCache` for `WeakInternPool`
- `PathConflictResolver.java` — fix O(n²) memory from HashSet copying
- `EnhancedLocalRepositoryManager.java` — cache tracking file reads to
eliminate redundant disk I/O
- `NamedLockFactorySupport.java` — lock-free fast path for ref-counted lock
acquisition
- `InhibitingNameMapper.java` — stream→loop conversion with empty-list
short-circuit
- `FileLockNamedLockFactory.java` — FileChannel reuse across lock lifecycle
---
.../impl/EnhancedLocalRepositoryManager.java | 47 +++-
.../aether/internal/impl/collect/DataPool.java | 34 +--
.../synccontext/named/InhibitingNameMapper.java | 47 +++-
.../named/providers/FileLockNamedLockFactory.java | 100 +++++----
.../named/support/NamedLockFactorySupport.java | 45 +++-
.../util/concurrency/ConcurrentWeakCache.java | 250 +++++++++++++++++++++
.../graph/transformer/PathConflictResolver.java | 59 +++--
.../aether/util/version/GenericVersionScheme.java | 106 +--------
8 files changed, 482 insertions(+), 206 deletions(-)
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManager.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManager.java
index dd314fed9..14bf81b3b 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManager.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/EnhancedLocalRepositoryManager.java
@@ -27,6 +27,7 @@ import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
@@ -62,12 +63,46 @@ class EnhancedLocalRepositoryManager extends
SimpleLocalRepositoryManager {
private static final String LOCAL_REPO_ID = "";
+ /**
+ * Shared sentinel for "no tracking data". Mutation is forbidden: the
instance is shared
+ * across threads via {@link #trackingFileCache} and returned directly
from {@link #readRepos}.
+ */
+ private static final Properties EMPTY_PROPERTIES = new Properties() {
+ @Override
+ public synchronized Object put(Object key, Object value) {
+ throw new UnsupportedOperationException("EMPTY_PROPERTIES is
read-only");
+ }
+
+ @Override
+ public synchronized Object remove(Object key) {
+ throw new UnsupportedOperationException("EMPTY_PROPERTIES is
read-only");
+ }
+
+ @Override
+ public synchronized void clear() {
+ throw new UnsupportedOperationException("EMPTY_PROPERTIES is
read-only");
+ }
+ };
+
private final String trackingFilename;
private final TrackingFileManager trackingFileManager;
private final LocalPathPrefixComposer localPathPrefixComposer;
+ /**
+ * Cache of tracking file contents, keyed by tracking file path.
Eliminates redundant disk I/O
+ * when multiple artifacts in the same directory are resolved — they all
share the same
+ * {@code _remote.repositories} tracking file. Invalidated on writes via
{@link #addRepo}.
+ * <p>
+ * Cached {@link Properties} instances are shared across threads and must
be treated as
+ * read-only by callers of {@link #readRepos}. The cache is scoped to this
manager instance
+ * (one per session), so concurrent builds in separate JVMs each maintain
independent caches.
+ * If another process updates a tracking file after it has been cached
here, the stale entry
+ * may cause a redundant (but harmless) download — no data corruption.
+ */
+ private final ConcurrentHashMap<Path, Properties> trackingFileCache = new
ConcurrentHashMap<>();
+
EnhancedLocalRepositoryManager(
Path basedir,
LocalPathComposer localPathComposer,
@@ -215,10 +250,10 @@ class EnhancedLocalRepositoryManager extends
SimpleLocalRepositoryManager {
private Properties readRepos(Path artifactPath) {
Path trackingFile = getTrackingFile(artifactPath);
-
- Properties props = trackingFileManager.read(trackingFile);
-
- return (props != null) ? props : new Properties();
+ return trackingFileCache.computeIfAbsent(trackingFile, tf -> {
+ Properties props = trackingFileManager.read(tf);
+ return (props != null) ? props : EMPTY_PROPERTIES;
+ });
}
private void addRepo(Path artifactPath, Collection<String> repositories) {
@@ -229,6 +264,10 @@ class EnhancedLocalRepositoryManager extends
SimpleLocalRepositoryManager {
Path trackingPath = getTrackingFile(artifactPath);
+ // Invalidate cache before write: using put() with the returned
Properties would be
+ // racy — two concurrent addRepo() calls could reorder their puts,
leaving stale data.
+ // Invalidating forces the next readRepos() to re-read from disk.
+ trackingFileCache.remove(trackingPath);
trackingFileManager.update(trackingPath, updates);
}
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java
index 463dcf804..f82d589ec 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java
@@ -18,14 +18,10 @@
*/
package org.eclipse.aether.internal.impl.collect;
-import java.lang.ref.WeakReference;
import java.util.Collection;
-import java.util.Collections;
import java.util.Iterator;
import java.util.List;
-import java.util.Map;
import java.util.Objects;
-import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentHashMap;
import org.eclipse.aether.Keys;
@@ -46,6 +42,7 @@ import org.eclipse.aether.resolution.ArtifactDescriptorResult;
import org.eclipse.aether.resolution.VersionRangeRequest;
import org.eclipse.aether.resolution.VersionRangeResult;
import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.concurrency.ConcurrentWeakCache;
import org.eclipse.aether.version.Version;
import org.eclipse.aether.version.VersionConstraint;
@@ -554,33 +551,24 @@ public final class DataPool {
}
}
+ /**
+ * Intern pool backed by ConcurrentWeakCache with weak keys and weak
values.
+ * Lock-free reads (ConcurrentHashMap.get is a volatile read, zero
allocation via
+ * ThreadLocal lookup key), lock-striped writes, weak keys and values
allow GC of
+ * interned objects when no longer strongly referenced.
+ * Uses putIfAbsent to guarantee concurrent callers for the same key get
the same instance.
+ */
private static class WeakInternPool<K, V> implements InternPool<K, V> {
- private final Map<K, WeakReference<V>> map =
Collections.synchronizedMap(new WeakHashMap<>(256));
+ private final ConcurrentWeakCache<K, V> cache = new
ConcurrentWeakCache<>(256);
@Override
public V get(K key) {
- WeakReference<V> ref = map.get(key);
- return ref != null ? ref.get() : null;
+ return cache.get(key);
}
@Override
- @SuppressWarnings("unchecked")
public V intern(K key, V value) {
- Object[] result = new Object[1];
- synchronized (map) {
- map.compute(key, (k, existingRef) -> {
- if (existingRef != null) {
- V pooled = existingRef.get();
- if (pooled != null) {
- result[0] = pooled;
- return existingRef;
- }
- }
- result[0] = value;
- return new WeakReference<>(value);
- });
- }
- return (V) result[0];
+ return cache.putIfAbsent(key, value);
}
}
}
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/InhibitingNameMapper.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/InhibitingNameMapper.java
index 202232d51..1fc4b433a 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/InhibitingNameMapper.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/InhibitingNameMapper.java
@@ -18,9 +18,9 @@
*/
package org.eclipse.aether.internal.impl.synccontext.named;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
-import java.util.stream.Collectors;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
@@ -54,16 +54,45 @@ public class InhibitingNameMapper implements NameMapper {
RepositorySystemSession session,
Collection<? extends Artifact> artifacts,
Collection<? extends Metadata> metadatas) {
- if (artifacts != null) {
- artifacts = artifacts.stream()
- .filter(a -> lockingInhibitors.stream().noneMatch(i ->
i.preventArtifactLocking(a)))
- .collect(Collectors.toList());
+ if (lockingInhibitors.isEmpty()) {
+ return delegate.nameLocks(session, artifacts, metadatas);
}
- if (metadatas != null) {
- metadatas = metadatas.stream()
- .filter(m -> lockingInhibitors.stream().noneMatch(i ->
i.preventMetadataLocking(m)))
- .collect(Collectors.toList());
+ if (artifacts != null && !artifacts.isEmpty()) {
+ List<Artifact> filtered = new ArrayList<>(artifacts.size());
+ for (Artifact a : artifacts) {
+ if (!isArtifactInhibited(a)) {
+ filtered.add(a);
+ }
+ }
+ artifacts = filtered;
+ }
+ if (metadatas != null && !metadatas.isEmpty()) {
+ List<Metadata> filtered = new ArrayList<>(metadatas.size());
+ for (Metadata m : metadatas) {
+ if (!isMetadataInhibited(m)) {
+ filtered.add(m);
+ }
+ }
+ metadatas = filtered;
}
return delegate.nameLocks(session, artifacts, metadatas);
}
+
+ private boolean isArtifactInhibited(Artifact artifact) {
+ for (LockingInhibitor inhibitor : lockingInhibitors) {
+ if (inhibitor.preventArtifactLocking(artifact)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private boolean isMetadataInhibited(Metadata metadata) {
+ for (LockingInhibitor inhibitor : lockingInhibitors) {
+ if (inhibitor.preventMetadataLocking(metadata)) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java
index fa808987e..fcaf4583b 100644
---
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java
+++
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/providers/FileLockNamedLockFactory.java
@@ -108,60 +108,70 @@ public class FileLockNamedLockFactory extends
NamedLockFactorySupport {
@Override
protected NamedLockSupport createLock(final NamedLockKey key) {
Path path = Paths.get(URI.create(key.name()));
- FileChannel fileChannel = fileChannels.computeIfAbsent(key, k -> {
- try {
- Files.createDirectories(path.getParent());
- FileChannel channel = retry(
- ATTEMPTS,
- SLEEP_MILLIS,
- () -> {
- if (DELETE_LOCK_FILES) {
- return FileChannel.open(
- path,
- StandardOpenOption.READ,
- StandardOpenOption.WRITE,
- StandardOpenOption.CREATE,
- StandardOpenOption.DELETE_ON_CLOSE);
- } else {
- return FileChannel.open(
- path,
- StandardOpenOption.READ,
- StandardOpenOption.WRITE,
- StandardOpenOption.CREATE);
- }
- },
- null,
- null);
-
- if (channel == null) {
- throw new IllegalStateException(
- "Could not open file channel for '" + key + "'
after " + ATTEMPTS + " attempts; giving up");
- }
- return channel;
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- throw new RuntimeException("Interrupted while opening file
channel for '" + key + "'", e);
- } catch (IOException e) {
- throw new UncheckedIOException("Failed to open file channel
for '" + key + "'", e);
- }
- });
+ FileChannel fileChannel = fileChannels.computeIfAbsent(key, k ->
openFileChannel(key, path));
+ if (!fileChannel.isOpen()) {
+ // Channel was closed externally (I/O error, NFS hiccup, etc.).
Evict the stale entry
+ // and open a fresh one. remove(key, fileChannel) is atomic: it
only removes if the
+ // value is still this exact (stale) instance, avoiding races with
other threads that
+ // may have already replaced it.
+ fileChannels.remove(key, fileChannel);
+ fileChannel = fileChannels.computeIfAbsent(key, k ->
openFileChannel(key, path));
+ }
return new FileLockNamedLock(key, fileChannel, this);
}
+ private FileChannel openFileChannel(NamedLockKey key, Path path) {
+ try {
+ Files.createDirectories(path.getParent());
+ FileChannel channel = retry(
+ ATTEMPTS,
+ SLEEP_MILLIS,
+ () -> {
+ if (DELETE_LOCK_FILES) {
+ return FileChannel.open(
+ path,
+ StandardOpenOption.READ,
+ StandardOpenOption.WRITE,
+ StandardOpenOption.CREATE,
+ StandardOpenOption.DELETE_ON_CLOSE);
+ } else {
+ return FileChannel.open(
+ path, StandardOpenOption.READ,
StandardOpenOption.WRITE, StandardOpenOption.CREATE);
+ }
+ },
+ null,
+ null);
+
+ if (channel == null) {
+ throw new IllegalStateException(
+ "Could not open file channel for '" + key + "' after "
+ ATTEMPTS + " attempts; giving up");
+ }
+ return channel;
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException("Interrupted while opening file channel
for '" + key + "'", e);
+ } catch (IOException e) {
+ throw new UncheckedIOException("Failed to open file channel for '"
+ key + "'", e);
+ }
+ }
+
@Override
protected void destroyLock(final NamedLock namedLock) {
- if (namedLock instanceof FileLockNamedLock) {
- final NamedLockKey key = namedLock.key();
- FileChannel fileChannel = fileChannels.remove(key);
- if (fileChannel == null) {
- throw new IllegalStateException("File channel expected, but
does not exist: " + key);
- }
+ // Keep the FileChannel open in the fileChannels map for reuse by
future createLock() calls.
+ // Opening a FileChannel is a syscall (open/creat) that shows up as a
hotspot when locks are
+ // acquired and released frequently (e.g., per-artifact resolution in
primed builds).
+ // Channels are closed on factory shutdown via doShutdown().
+ }
+ @Override
+ protected void doShutdown() {
+ for (FileChannel channel : fileChannels.values()) {
try {
- fileChannel.close();
+ channel.close();
} catch (IOException e) {
- throw new UncheckedIOException("Failed to close file channel
for '" + key + "'", e);
+ logger.warn("Failed to close file channel", e);
}
}
+ fileChannels.clear();
}
}
diff --git
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java
index bee186afc..47fa4895b 100644
---
a/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java
+++
b/maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java
@@ -115,14 +115,28 @@ public abstract class NamedLockFactorySupport implements
NamedLockFactory {
}
protected NamedLock getLockAndRefTrack(final NamedLockKey key,
Supplier<NamedLockSupport> supplier) {
+ if (shutdown.get()) {
+ throw new IllegalStateException("factory already shut down");
+ }
+ // Fast path: lock-free volatile read + atomic CAS increment.
+ // ConcurrentHashMap.get() is a volatile read — no bucket locking.
+ // In the common case (lock already exists), this avoids compute()'s
+ // per-bucket exclusive lock entirely.
+ NamedLockHolder holder = locks.get(key);
+ if (holder != null && holder.tryIncRef()) {
+ return holder.namedLock;
+ }
+ // Slow path: holder absent or being closed (refcount hit 0).
+ // Use compute() to atomically create a new holder.
return locks.compute(key, (k, v) -> {
if (shutdown.get()) {
throw new IllegalStateException("factory already shut
down");
}
- if (v == null) {
+ if (v == null || !v.tryIncRef()) {
v = new NamedLockHolder(supplier.get());
+ v.incRef();
}
- return v.incRef();
+ return v;
})
.namedLock;
}
@@ -170,8 +184,14 @@ public abstract class NamedLockFactorySupport implements
NamedLockFactory {
public void closeLock(final NamedLockKey key) {
locks.compute(key, (k, v) -> {
if (v != null && v.decRef() == 0) {
- destroyLock(v.namedLock);
- return null;
+ // Mark as closed to prevent a concurrent tryIncRef (lock-free
fast path)
+ // from reviving this holder. CAS ensures atomicity: if
tryIncRef already
+ // incremented from 0→1, our CAS fails and we keep the holder
alive.
+ if (v.referenceCount.compareAndSet(0, Integer.MIN_VALUE)) {
+ destroyLock(v.namedLock);
+ return null;
+ }
+ // A concurrent tryIncRef succeeded — holder is still in use,
keep it
}
return v;
});
@@ -206,6 +226,23 @@ public abstract class NamedLockFactorySupport implements
NamedLockFactory {
return this;
}
+ /**
+ * Atomically tries to increment the reference count. Returns {@code
false} if the
+ * holder has been closed (refcount ≤ 0), preventing revival of a
destroyed lock.
+ * Used by the lock-free fast path in {@link #getLockAndRefTrack}.
+ */
+ private boolean tryIncRef() {
+ while (true) {
+ int current = referenceCount.get();
+ if (current <= 0) {
+ return false;
+ }
+ if (referenceCount.compareAndSet(current, current + 1)) {
+ return true;
+ }
+ }
+ }
+
private int decRef() {
return referenceCount.decrementAndGet();
}
diff --git
a/maven-resolver-util/src/main/java/org/eclipse/aether/util/concurrency/ConcurrentWeakCache.java
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/concurrency/ConcurrentWeakCache.java
new file mode 100644
index 000000000..4f15a0749
--- /dev/null
+++
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/concurrency/ConcurrentWeakCache.java
@@ -0,0 +1,250 @@
+/*
+ * 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.eclipse.aether.util.concurrency;
+
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * A concurrent cache with weak keys and weak values, inspired by maven-impl's
Cache class
+ * but stripped down and optimized for hot-path performance.
+ * <p>
+ * Design compared to the full Cache:
+ * <ul>
+ * <li><b>Zero allocation on get()</b> — uses a ThreadLocal reusable lookup
key
+ * instead of allocating a new wrapper on every read</li>
+ * <li><b>O(1) cleanup per stale entry</b> — uses identity-based removal from
+ * the ReferenceQueue instead of a full {@code entrySet().removeIf()}
scan</li>
+ * <li><b>Lock-free reads</b> — {@link ConcurrentHashMap#get} is a volatile
read
+ * with no lock acquisition</li>
+ * <li><b>Lock-striped writes</b> — ConcurrentHashMap uses fine-grained
locking</li>
+ * </ul>
+ * <p>
+ * Keys are held via {@link WeakReference}: when a key is no longer strongly
referenced
+ * elsewhere, its entry becomes eligible for garbage collection. Values are
also held
+ * via WeakReference. Stale entries are cleaned up lazily on {@link #put}.
+ *
+ * @param <K> the type of keys
+ * @param <V> the type of values
+ * @since 2.0.19
+ */
+public class ConcurrentWeakCache<K, V> {
+
+ private final ConcurrentHashMap<Object, WeakReference<V>> map;
+ private final ReferenceQueue<Object> queue = new ReferenceQueue<>();
+
+ @SuppressWarnings("rawtypes")
+ private static final ThreadLocal<LookupKey> LOOKUP_KEY = new
ThreadLocal<LookupKey>() {
+ @Override
+ protected LookupKey initialValue() {
+ return new LookupKey();
+ }
+ };
+
+ /**
+ * Creates a new cache with default initial capacity.
+ */
+ public ConcurrentWeakCache() {
+ this.map = new ConcurrentHashMap<>();
+ }
+
+ /**
+ * Creates a new cache with the given initial capacity.
+ *
+ * @param initialCapacity the initial capacity
+ */
+ public ConcurrentWeakCache(int initialCapacity) {
+ this.map = new ConcurrentHashMap<>(initialCapacity);
+ }
+
+ /**
+ * Returns the value for the given key, or {@code null} if the key is not
present
+ * or the value has been garbage collected.
+ * <p>
+ * This method is lock-free and allocates no objects.
+ *
+ * @param key the key to look up
+ * @return the cached value, or {@code null}
+ */
+ @SuppressWarnings("unchecked")
+ public V get(K key) {
+ LookupKey<K> lookupKey = LOOKUP_KEY.get();
+ lookupKey.set(key);
+ try {
+ WeakReference<V> ref = map.get(lookupKey);
+ return ref != null ? ref.get() : null;
+ } finally {
+ lookupKey.clear();
+ }
+ }
+
+ /**
+ * Stores a key-value pair in the cache. Both key and value are held via
weak references.
+ * <p>
+ * Also performs lazy cleanup of entries whose keys have been garbage
collected.
+ *
+ * @param key the key
+ * @param value the value
+ */
+ public void put(K key, V value) {
+ expungeStaleEntries();
+ map.put(new WeakKey<>(key, queue), new WeakReference<>(value));
+ }
+
+ /**
+ * If the key is not already present (or its value has been GC'd), stores
the key-value pair
+ * and returns the given value. If the key is already present with a live
value, returns the
+ * existing value without storing. Concurrent callers for the same key are
guaranteed to
+ * receive the same instance (the insert-or-keep decision is atomic per
key via
+ * {@link ConcurrentHashMap#merge}).
+ * <p>
+ * Also performs lazy cleanup of entries whose keys have been garbage
collected.
+ *
+ * @param key the key
+ * @param value the value to store if absent
+ * @return the existing value if present, or the given value if newly
stored
+ */
+ public V putIfAbsent(K key, V value) {
+ // Fast path: lock-free lookup, zero allocation (ThreadLocal LookupKey)
+ V existing = get(key);
+ if (existing != null) {
+ return existing;
+ }
+ // Slow path: atomic insert-or-keep via merge() — locks only the
target bucket,
+ // so contention is per-key, not global. Handles the case where an
existing entry's
+ // value has been GC'd by atomically replacing it with the new value.
+ expungeStaleEntries();
+ WeakReference<V> newRef = new WeakReference<>(value);
+ WeakReference<V> ref = map.merge(new WeakKey<>(key, queue), newRef,
(existingRef, incoming) -> {
+ V existingValue = existingRef.get();
+ return existingValue != null ? existingRef : incoming;
+ });
+ if (ref != newRef) {
+ V existingValue = ref.get();
+ if (existingValue != null) {
+ return existingValue;
+ }
+ }
+ return value;
+ }
+
+ /**
+ * Returns the number of entries in the cache (including possibly stale
ones).
+ *
+ * @return the cache size
+ */
+ public int size() {
+ return map.size();
+ }
+
+ /**
+ * Removes entries whose weak-reference keys have been garbage collected.
+ * Called automatically on {@link #put}. Each stale entry is removed in
O(1)
+ * via identity match — no map scan required.
+ */
+ private void expungeStaleEntries() {
+ Object staleKey;
+ while ((staleKey = queue.poll()) != null) {
+ // ConcurrentHashMap.remove() checks (storedKey == staleKey) first.
+ // Since the staleKey IS the same WeakKey object that was stored in
+ // the map, identity matches and the entry is removed in O(1).
+ map.remove(staleKey);
+ }
+ }
+
+ /**
+ * Reusable lookup key stored in a ThreadLocal for zero-allocation reads.
+ * Used only for {@link ConcurrentHashMap#get} lookups, never stored in
the map.
+ * <p>
+ * Implements equals/hashCode to match {@link WeakKey} entries in the map:
+ * ConcurrentHashMap.get(lookupKey) calls {@code
lookupKey.equals(storedWeakKey)},
+ * which delegates to {@code key.equals(storedWeakKey.get())}.
+ */
+ static class LookupKey<K> {
+ private K key;
+ private int hash;
+
+ void set(K key) {
+ this.key = key;
+ this.hash = key.hashCode();
+ }
+
+ void clear() {
+ this.key = null; // prevent leak via ThreadLocal
+ }
+
+ @Override
+ public int hashCode() {
+ return hash;
+ }
+
+ @Override
+ @SuppressWarnings("rawtypes")
+ public boolean equals(Object o) {
+ if (o instanceof WeakKey) {
+ Object otherKey = ((WeakKey) o).get();
+ return key != null && key.equals(otherKey);
+ }
+ return false;
+ }
+ }
+
+ /**
+ * Weak-reference key stored in the map. When the referent is garbage
collected,
+ * this object is enqueued in the {@link ReferenceQueue} for cleanup.
+ * <p>
+ * The cleanup path uses identity: {@code map.remove(staleWeakKey)}
matches because
+ * ConcurrentHashMap checks {@code storedKey == staleWeakKey} before
calling equals().
+ * Since the queued WeakKey IS the same object that's in the map, this is
always true.
+ */
+ static class WeakKey<K> extends WeakReference<K> {
+ private final int hash;
+
+ @SuppressWarnings("unchecked")
+ WeakKey(K key, ReferenceQueue<? super K> queue) {
+ super(key, (ReferenceQueue<? super K>) queue);
+ this.hash = key.hashCode();
+ }
+
+ @Override
+ public int hashCode() {
+ return hash;
+ }
+
+ @Override
+ @SuppressWarnings("rawtypes")
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true; // identity match — used for cleanup and normal
dedup
+ }
+ K k = get();
+ if (k == null) {
+ return false; // referent GC'd
+ }
+ if (o instanceof WeakKey) {
+ return k.equals(((WeakKey) o).get());
+ }
+ if (o instanceof LookupKey) {
+ return k.equals(((LookupKey) o).key);
+ }
+ return false;
+ }
+ }
+}
diff --git
a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java
index 92e3bd495..20b1b6f9a 100644
---
a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java
+++
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java
@@ -22,7 +22,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -175,7 +175,7 @@ public final class PathConflictResolver extends
ConflictResolver {
// loop over topographically sorted conflictIds
for (String conflictId : sortedConflictIds) {
// paths in given conflict group to consider
- List<Path> paths = state.partitions.get(conflictId);
+ Set<Path> paths = state.partitions.get(conflictId);
if (paths.isEmpty()) {
// this means that whole group "fall out of scope" (are all on
loser branches); skip
continue;
@@ -208,7 +208,7 @@ public final class PathConflictResolver extends
ConflictResolver {
}
state.resolvedIds.put(conflictId, winnerPath);
- // loop over considered paths and apply selection results; note:
node may remove itself from iterated list
+ // loop over considered paths and apply selection results; note:
node may remove itself from iterated set
for (Path path : new ArrayList<>(paths)) {
// apply selected properties scope/optional to winner (winner
carries version; others are losers)
if (path == winnerPath) {
@@ -230,7 +230,7 @@ public final class PathConflictResolver extends
ConflictResolver {
stats.put("ConflictResolver.totalTime", time2 - time1);
stats.put(
"ConflictResolver.conflictItemCount",
-
state.partitions.values().stream().map(List::size).reduce(0, Integer::sum));
+
state.partitions.values().stream().map(Set::size).reduce(0, Integer::sum));
}
return node;
@@ -279,8 +279,9 @@ public final class PathConflictResolver extends
ConflictResolver {
/**
* A mapping from conflictId to paths represented as {@link Path}s
that exist for each conflictId. In other
* words all paths to each {@link DependencyNode} that are member of
same conflictId group.
+ * Uses {@link LinkedHashSet} for O(1) removal in {@link
Path#moveOutOfScope()} while preserving insertion order.
*/
- private final Map<String, List<Path>> partitions;
+ private final Map<String, Set<Path>> partitions;
/**
* A mapping from conflictIds to winner {@link Path}, hence {@link
DependencyNode} for given conflictId.
@@ -322,12 +323,7 @@ public final class PathConflictResolver extends
ConflictResolver {
*/
private Path build(DependencyNode node) throws RepositoryException {
String nodeConflictId = this.conflictIds.get(node);
- Path root = new Path(
- this,
- node,
- nodeConflictId,
- nodeConflictId != null ?
Collections.singleton(nodeConflictId) : Collections.emptySet(),
- null);
+ Path root = new Path(this, node, nodeConflictId, null);
gatherCRNodes(root);
return root;
}
@@ -381,11 +377,6 @@ public final class PathConflictResolver extends
ConflictResolver {
private DependencyNode dn;
private final String conflictId;
private final Path parent;
- // Set of conflictIds that we "stepped over" from root to here; is a
set, and if duplicate element addition is
- // attempted, it signals we deal with a cycle, as we are about to
enter into same tree partition (nodes with
- // same conflictId) we already have been. This could be a list, but
for our purposes "duplication detection"
- // (loop) is perfectly enough.
- private final Set<String> conflictIdsSinceRoot;
// derived
private final int depth;
private final List<Path> children;
@@ -393,22 +384,37 @@ public final class PathConflictResolver extends
ConflictResolver {
private String scope;
private boolean optional;
- private Path(State state, DependencyNode dn, String conflictId,
Set<String> conflictIdsSinceRoot, Path parent) {
+ private Path(State state, DependencyNode dn, String conflictId, Path
parent) {
this.state = state;
this.dn = dn;
this.conflictId = conflictId;
this.parent = parent;
- this.conflictIdsSinceRoot = conflictIdsSinceRoot;
this.depth = parent != null ? parent.depth + 1 : 0;
this.children = new ArrayList<>();
pull(0);
this.state
.partitions
- .computeIfAbsent(this.conflictId, k -> new ArrayList<>())
+ .computeIfAbsent(this.conflictId, k -> new
LinkedHashSet<>())
.add(this);
}
+ /**
+ * Checks whether the given conflictId appears on the path from this
node to the root.
+ * This replaces the previous approach of copying a HashSet at every
child, which was the
+ * dominant source of memory consumption (millions of HashMap.Node
objects for large graphs).
+ */
+ private boolean hasConflictIdOnPathToRoot(String targetConflictId) {
+ Path current = this;
+ while (current != null) {
+ if (Objects.equals(current.conflictId, targetConflictId)) {
+ return true;
+ }
+ current = current.parent;
+ }
+ return false;
+ }
+
/**
* Pulls (possibly updated) scope and optional values from associated
{@link DependencyNode} to this instance,
* going down toward children recursively the required count of levels.
@@ -614,6 +620,7 @@ public final class PathConflictResolver extends
ConflictResolver {
/**
* Removes this and all child {@link Path} nodes from winner selection
scope; essentially marks whole subtree
* from "this and below" as loser, to not be considered in subsequent
winner selections.
+ * Uses {@link LinkedHashSet} for O(1) removal instead of the previous
O(N) ArrayList.remove.
*/
private void moveOutOfScope() {
this.state.partitions.get(this.conflictId).remove(this);
@@ -626,8 +633,9 @@ public final class PathConflictResolver extends
ConflictResolver {
* Adds node children: this method should be "batch" used, as all
(potential) children should be added at once.
* Method will return really added {@link Path} instances, as this
class avoids cycles. Those forming a cycle
* are added to {@link Path} structure but are not recursed (not
returned in list), keeping {@link Path} cycle
- * free. This method also maintains "conflictIdsSinceRoot", that is a
set of "conflict IDs" that were touched
- * while going from tree root to current node instance.
+ * free. Cycle detection is performed by walking up the parent chain
via
+ * {@link #hasConflictIdOnPathToRoot(String)} instead of maintaining a
per-node set, trading O(depth) per
+ * check for dramatically reduced memory allocation on large
dependency graphs.
* This implies that this conflict resolver, by its nature "redoes" the
* {@link TransformationContextKeys#CYCLIC_CONFLICT_IDS} calculated by
{@link ConflictIdSorter}.
*/
@@ -635,9 +643,12 @@ public final class PathConflictResolver extends
ConflictResolver {
ArrayList<Path> added = new ArrayList<>(children.size());
for (DependencyNode child : children) {
String childConflictId = this.state.conflictIds.get(child);
- Set<String> conflictIdsSinceRoot = new
HashSet<>(this.conflictIdsSinceRoot);
- boolean cycle = !conflictIdsSinceRoot.add(childConflictId);
- Path c = new Path(this.state, child, childConflictId,
conflictIdsSinceRoot, this);
+ // Detect cycles by walking up the parent chain instead of
copying a HashSet per child.
+ // This trades O(depth) per check for eliminating the dominant
memory consumer:
+ // previously, each Path copied its parent's entire
conflict-ID set, creating millions
+ // of HashMap.Node objects for large graphs.
+ boolean cycle = hasConflictIdOnPathToRoot(childConflictId);
+ Path c = new Path(this.state, child, childConflictId, this);
this.children.add(c);
c.derive(0, false);
if (!cycle) {
diff --git
a/maven-resolver-util/src/main/java/org/eclipse/aether/util/version/GenericVersionScheme.java
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/version/GenericVersionScheme.java
index 5a8428f56..01772fa3f 100644
---
a/maven-resolver-util/src/main/java/org/eclipse/aether/util/version/GenericVersionScheme.java
+++
b/maven-resolver-util/src/main/java/org/eclipse/aether/util/version/GenericVersionScheme.java
@@ -19,12 +19,8 @@
package org.eclipse.aether.util.version;
import java.nio.charset.StandardCharsets;
-import java.util.Collections;
-import java.util.Map;
-import java.util.WeakHashMap;
-import java.util.concurrent.atomic.AtomicLong;
-import org.eclipse.aether.ConfigurationProperties;
+import org.eclipse.aether.util.concurrency.ConcurrentWeakCache;
import org.eclipse.aether.version.InvalidVersionSpecificationException;
/**
@@ -54,102 +50,18 @@ import
org.eclipse.aether.version.InvalidVersionSpecificationException;
*/
public class GenericVersionScheme extends VersionSchemeSupport {
- // Using WeakHashMap wrapped in synchronizedMap for thread safety and
memory-sensitive caching
- private final Map<String, GenericVersion> versionCache =
Collections.synchronizedMap(new WeakHashMap<>());
-
- // Cache statistics
- private final AtomicLong cacheHits = new AtomicLong(0);
- private final AtomicLong cacheMisses = new AtomicLong(0);
- private final AtomicLong totalRequests = new AtomicLong(0);
-
- // Static statistics across all instances
- private static final AtomicLong GLOBAL_CACHE_HITS = new AtomicLong(0);
- private static final AtomicLong GLOBAL_CACHE_MISSES = new AtomicLong(0);
- private static final AtomicLong GLOBAL_TOTAL_REQUESTS = new AtomicLong(0);
- private static final AtomicLong INSTANCE_COUNT = new AtomicLong(0);
-
- static {
- // Register shutdown hook to print statistics if enabled
- if (isStatisticsEnabled()) {
- Runtime.getRuntime().addShutdownHook(new
Thread(GenericVersionScheme::printGlobalStatistics));
- }
- }
-
- public GenericVersionScheme() {
- INSTANCE_COUNT.incrementAndGet();
- }
-
- /**
- * Checks if version scheme cache statistics should be printed.
- * This checks both the system property and the configuration property.
- */
- private static boolean isStatisticsEnabled() {
- // Check system property first (for backwards compatibility and ease
of use)
- String sysProp =
System.getProperty(ConfigurationProperties.VERSION_SCHEME_CACHE_DEBUG);
- if (sysProp != null) {
- return Boolean.parseBoolean(sysProp);
- }
-
- // Default to false if not configured
- return ConfigurationProperties.DEFAULT_VERSION_SCHEME_CACHE_DEBUG;
- }
+ // Concurrent cache with weak keys and weak values: lock-free reads
(volatile read, no lock
+ // acquisition via ConcurrentHashMap), lock-striped writes, zero
allocation on get() via
+ // ThreadLocal lookup key, weak references allow GC under memory pressure.
+ private final ConcurrentWeakCache<String, GenericVersion> versionCache =
new ConcurrentWeakCache<>();
@Override
public GenericVersion parseVersion(final String version) throws
InvalidVersionSpecificationException {
- totalRequests.incrementAndGet();
- GLOBAL_TOTAL_REQUESTS.incrementAndGet();
-
- boolean[] created = {false};
- GenericVersion result;
- synchronized (versionCache) {
- result = versionCache.computeIfAbsent(version, v -> {
- created[0] = true;
- return new GenericVersion(v);
- });
- }
- if (created[0]) {
- cacheMisses.incrementAndGet();
- GLOBAL_CACHE_MISSES.incrementAndGet();
- } else {
- cacheHits.incrementAndGet();
- GLOBAL_CACHE_HITS.incrementAndGet();
+ GenericVersion v = versionCache.get(version);
+ if (v == null) {
+ v = versionCache.putIfAbsent(version, new GenericVersion(version));
}
- return result;
- }
-
- /**
- * Get cache statistics for this instance.
- */
- public String getCacheStatistics() {
- long hits = cacheHits.get();
- long misses = cacheMisses.get();
- long total = totalRequests.get();
- double hitRate = total > 0 ? (double) hits / total * 100.0 : 0.0;
-
- return String.format(
- "GenericVersionScheme Cache Stats: hits=%d, misses=%d,
total=%d, hit-rate=%.2f%%, cache-size=%d",
- hits, misses, total, hitRate, versionCache.size());
- }
-
- /**
- * Print global statistics across all instances.
- */
- private static void printGlobalStatistics() {
- long hits = GLOBAL_CACHE_HITS.get();
- long misses = GLOBAL_CACHE_MISSES.get();
- long total = GLOBAL_TOTAL_REQUESTS.get();
- long instances = INSTANCE_COUNT.get();
- double hitRate = total > 0 ? (double) hits / total * 100.0 : 0.0;
-
- System.err.println("=== GenericVersionScheme Global Cache Statistics
(WeakHashMap) ===");
- System.err.println(String.format("Total instances created: %d",
instances));
- System.err.println(String.format("Total requests: %d", total));
- System.err.println(String.format("Cache hits: %d", hits));
- System.err.println(String.format("Cache misses: %d", misses));
- System.err.println(String.format("Hit rate: %.2f%%", hitRate));
- System.err.println(
- String.format("Average requests per instance: %.2f", instances
> 0 ? (double) total / instances : 0.0));
- System.err.println("=== End Cache Statistics ===");
+ return v;
}
/**
