From e1cafca90f70a501033bd068be4c78d2088a9bca Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 14 Feb 2023 17:45:03 +0100 Subject: [PATCH] [MRESOLVER-320] Use strong references by default (#241) Add 3 type of pools and make them configurable: hard, weak and none. Hard increases memory usage (500MB vs 1.1G on tested project), but this is on par with Maven 3.8.7 behaviour and ceases the 10% slowdown as well (speed goes back on par as well). --- https://issues.apache.org/jira/browse/MRESOLVER-320 --- .../internal/impl/collect/DataPool.java | 161 +++++++++++++++--- .../internal/impl/collect/ObjectPool.java | 53 ------ src/site/markdown/configuration.md | 1 + 3 files changed, 137 insertions(+), 78 deletions(-) delete mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ObjectPool.java 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 a9f5aa86f..70b1d55dd 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 @@ -22,12 +22,12 @@ import java.lang.ref.WeakReference; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; 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.RepositoryCache; import org.eclipse.aether.RepositorySystemSession; @@ -45,6 +45,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.version.Version; import org.eclipse.aether.version.VersionConstraint; @@ -53,6 +54,7 @@ */ public final class DataPool { + private static final String CONFIG_PROP_COLLECTOR_POOL = "aether.dependencyCollector.pool"; private static final String ARTIFACT_POOL = DataPool.class.getName() + "$Artifact"; @@ -63,64 +65,90 @@ public final class DataPool public static final ArtifactDescriptorResult NO_DESCRIPTOR = new ArtifactDescriptorResult( new ArtifactDescriptorRequest() ); - private ObjectPool artifacts; + /** + * Artifact interning pool, lives across session (if session carries non-null {@link RepositoryCache}). + */ + private final InternPool artifacts; - private ObjectPool dependencies; + /** + * Dependency interning pool, lives across session (if session carries non-null {@link RepositoryCache}). + */ + private final InternPool dependencies; - private Map> descriptors; + /** + * Descriptor interning pool, lives across session (if session carries non-null {@link RepositoryCache}). + */ + private final InternPool descriptors; - private final Map constraints = new HashMap<>(); + /** + * Constraint cache, lives during single collection invocation (same as this DataPool instance). + */ + private final ConcurrentHashMap constraints; - private final Map> nodes = new HashMap<>( 256 ); + /** + * DependencyNode cache, lives during single collection invocation (same as this DataPool instance). + */ + private final ConcurrentHashMap> nodes; @SuppressWarnings( "unchecked" ) public DataPool( RepositorySystemSession session ) { - RepositoryCache cache = session.getCache(); + final RepositoryCache cache = session.getCache(); + final String poolType = ConfigUtils.getString( session, HARD, CONFIG_PROP_COLLECTOR_POOL ); + InternPool artifactsPool = null; + InternPool dependenciesPool = null; + InternPool descriptorsPool = null; if ( cache != null ) { - artifacts = (ObjectPool) cache.get( session, ARTIFACT_POOL ); - dependencies = (ObjectPool) cache.get( session, DEPENDENCY_POOL ); - descriptors = (Map>) cache.get( session, DESCRIPTORS ); + artifactsPool = (InternPool) cache.get( session, ARTIFACT_POOL ); + dependenciesPool = (InternPool) cache.get( session, DEPENDENCY_POOL ); + descriptorsPool = (InternPool) cache.get( session, DESCRIPTORS ); } - if ( artifacts == null ) + if ( artifactsPool == null ) { - artifacts = new ObjectPool<>(); + artifactsPool = createPool( poolType ); if ( cache != null ) { - cache.put( session, ARTIFACT_POOL, artifacts ); + cache.put( session, ARTIFACT_POOL, artifactsPool ); } } - if ( dependencies == null ) + if ( dependenciesPool == null ) { - dependencies = new ObjectPool<>(); + dependenciesPool = createPool( poolType ); if ( cache != null ) { - cache.put( session, DEPENDENCY_POOL, dependencies ); + cache.put( session, DEPENDENCY_POOL, dependenciesPool ); } } - if ( descriptors == null ) + if ( descriptorsPool == null ) { - descriptors = Collections.synchronizedMap( new WeakHashMap<>( 256 ) ); + descriptorsPool = createPool( poolType ); if ( cache != null ) { - cache.put( session, DESCRIPTORS, descriptors ); + cache.put( session, DESCRIPTORS, descriptorsPool ); } } + + this.artifacts = artifactsPool; + this.dependencies = dependenciesPool; + this.descriptors = descriptorsPool; + + this.constraints = new ConcurrentHashMap<>( 256 ); + this.nodes = new ConcurrentHashMap<>( 256 ); } public Artifact intern( Artifact artifact ) { - return artifacts.intern( artifact ); + return artifacts.intern( artifact, artifact ); } public Dependency intern( Dependency dependency ) { - return dependencies.intern( dependency ); + return dependencies.intern( dependency, dependency ); } public Object toKey( ArtifactDescriptorRequest request ) @@ -130,8 +158,7 @@ public Object toKey( ArtifactDescriptorRequest request ) public ArtifactDescriptorResult getDescriptor( Object key, ArtifactDescriptorRequest request ) { - WeakReference descriptorRef = descriptors.get( key ); - Descriptor descriptor = descriptorRef != null ? descriptorRef.get() : null; + Descriptor descriptor = descriptors.get( key ); if ( descriptor != null ) { return descriptor.toResult( request ); @@ -141,12 +168,12 @@ public ArtifactDescriptorResult getDescriptor( Object key, ArtifactDescriptorReq public void putDescriptor( Object key, ArtifactDescriptorResult result ) { - descriptors.put( key, new WeakReference<>( new GoodDescriptor( result ) ) ); + descriptors.intern( key, new GoodDescriptor( result ) ); } public void putDescriptor( Object key, ArtifactDescriptorException e ) { - descriptors.put( key, new WeakReference<>( BadDescriptor.INSTANCE ) ); + descriptors.intern( key, BadDescriptor.INSTANCE ); } public Object toKey( VersionRangeRequest request ) @@ -415,4 +442,88 @@ public int hashCode() return hashCode; } } + + private static InternPool createPool( String type ) + { + if ( HARD.equals( type ) ) + { + return new HardInternPool<>(); + } + else if ( WEAK.equals( type ) ) + { + return new WeakInternPool<>(); + } + else if ( NONE.equals( type ) ) + { + return new NoneInternPool<>(); + } + else + { + throw new IllegalArgumentException( "Unknown object pool type: '" + type + "'" ); + } + } + + private static final String HARD = "hard"; + + private static final String WEAK = "weak"; + + private static final String NONE = "none"; + + private interface InternPool + { + V get( K key ); + + V intern( K key, V value ); + } + + private static class HardInternPool implements InternPool + { + private final ConcurrentHashMap map = new ConcurrentHashMap<>( 256 ); + + @Override + public V get( K key ) + { + return map.get( key ); + } + + @Override + public V intern( K key, V value ) + { + return map.computeIfAbsent( key, k -> value ); + } + } + + private static class WeakInternPool implements InternPool + { + private final Map> map = Collections.synchronizedMap( new WeakHashMap<>( 256 ) ); + + @Override + public V get( K key ) + { + WeakReference ref = map.get( key ); + return ref != null ? ref.get() : null; + } + + @Override + public V intern( K key, V value ) + { + return map.computeIfAbsent( key, k -> new WeakReference<>( value ) ).get(); + } + } + + private static class NoneInternPool implements InternPool + { + @Override + public V get( K key ) + { + return null; + } + + @Override + public V intern( K key, V value ) + { + return value; + } + } + } diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ObjectPool.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ObjectPool.java deleted file mode 100644 index 0d5f37997..000000000 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ObjectPool.java +++ /dev/null @@ -1,53 +0,0 @@ -package org.eclipse.aether.internal.impl.collect; - -/* - * 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. - */ - -import java.lang.ref.Reference; -import java.lang.ref.WeakReference; -import java.util.Map; -import java.util.WeakHashMap; - -/** - * Pool of immutable object instances, used to avoid excessive memory consumption of (dirty) dependency graph which - * tends to have many duplicate artifacts/dependencies. - * Internal helper class for collector implementations. - */ -class ObjectPool -{ - - private final Map> objects = new WeakHashMap<>( 256 ); - - public synchronized T intern( T object ) - { - Reference pooledRef = objects.get( object ); - if ( pooledRef != null ) - { - T pooled = pooledRef.get(); - if ( pooled != null ) - { - return pooled; - } - } - - objects.put( object, new WeakReference<>( object ) ); - return object; - } - -} diff --git a/src/site/markdown/configuration.md b/src/site/markdown/configuration.md index 18e147946..74749a438 100644 --- a/src/site/markdown/configuration.md +++ b/src/site/markdown/configuration.md @@ -52,6 +52,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix `aether.dependencyCollector.impl` | String | The name of the dependency collector implementation to use: depth-first (original) named `df`, and breadth-first (new in 1.8.0) named `bf`. Both collectors produce equivalent results, but they may differ performance wise, depending on project being applied to. Our experience shows that existing `df` is well suited for smaller to medium size projects, while `bf` may perform better on huge projects with many dependencies. Experiment (and come back to us!) to figure out which one suits you the better. | `"df"` | no `aether.dependencyCollector.bf.skipper` | boolean | Flag controlling whether to skip resolving duplicate/conflicting nodes during the breadth-first (`bf`) dependency collection process. | `true` | no `aether.dependencyCollector.bf.threads` or `maven.artifact.threads` | int | Number of threads to use for collecting POMs and version ranges in BF collector. | `5` | no +`aether.dependencyCollector.pool` | String | Flag controlling data pool type used by dependency collector regarding heap consumption. By default uses "hard" references (consume more heap, but is faster). Using "weak" will make resolver much more memory conservative, at the cost of up to 10% slower collecting dependency speed (system and Java dependent). Supported values: `"hard"`, `"weak"`, `"none"`. | `"hard"` | no `aether.dependencyManager.verbose` | boolean | Flag controlling the verbose mode for dependency management. If enabled, the original attributes of a dependency before its update due to dependency managemnent will be recorded in the node's `DependencyNode#getData()` when building a dependency graph. | `false` | no `aether.enhancedLocalRepository.localPrefix` | String | The prefix to use for locally installed artifacts. | `"installed"` | no `aether.enhancedLocalRepository.snapshotsPrefix` | String | The prefix to use for snapshot artifacts. | `"snapshots"` | no