You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by sl...@apache.org on 2019/04/15 13:51:44 UTC

[maven] 01/01: [MNG-6633] - Reduce memory usage of excludes

This is an automated email from the ASF dual-hosted git repository.

slachiewicz pushed a commit to branch MNG-6633
in repository https://gitbox.apache.org/repos/asf/maven.git

commit 10a16a3c35bf587ad214e5f70da38ad7c5a53eb8
Author: Stefan Oehme <st...@gmail.com>
AuthorDate: Thu Apr 11 18:10:44 2019 +0200

    [MNG-6633] - Reduce memory usage of excludes
    
    ExcludesArtifactFilter was highly inefficient.
    It took the group and artifact ID of an Exclusion, concatenated them into
    a new String, which was kept in memory for the whole duration
    of the build and then compared that String against the concatenation
    of group and artifact ID of each incoming artifact, adding more
    CPU cycles than necessary.
    
    Instead it now just takes the group and artifact ID from the Exclusion
    object and compares them against the group and artifact ID of the Artifact.
    
    The public API was kept the same, although it would be really worth
    overhauling. E.g. things like Exclude extends Include violate all kinds
    of design principles.
---
 .../repository/legacy/LegacyRepositorySystem.java  | 18 ++---
 .../resolver/filter/ExcludesArtifactFilter.java    | 23 +++++-
 .../resolver/filter/IncludesArtifactFilter.java    | 45 ++++++------
 .../artifact/resolver/filter/ModuleIdentifier.java | 81 ++++++++++++++++++++++
 .../apache/maven/bridge/MavenRepositorySystem.java | 10 +--
 .../project/artifact/MavenMetadataSource.java      | 10 +--
 6 files changed, 131 insertions(+), 56 deletions(-)

diff --git a/maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java b/maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java
index e3e7781..4c3b3f9 100644
--- a/maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java
+++ b/maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java
@@ -35,7 +35,6 @@ import org.apache.maven.artifact.InvalidRepositoryException;
 import org.apache.maven.artifact.factory.ArtifactFactory;
 import org.apache.maven.artifact.metadata.ArtifactMetadata;
 import org.apache.maven.artifact.repository.ArtifactRepository;
-import org.apache.maven.repository.legacy.repository.ArtifactRepositoryFactory;
 import org.apache.maven.artifact.repository.ArtifactRepositoryPolicy;
 import org.apache.maven.artifact.repository.Authentication;
 import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout;
@@ -46,18 +45,18 @@ import org.apache.maven.artifact.resolver.filter.ExcludesArtifactFilter;
 import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
 import org.apache.maven.artifact.versioning.VersionRange;
 import org.apache.maven.model.Dependency;
-import org.apache.maven.model.Exclusion;
 import org.apache.maven.model.Plugin;
 import org.apache.maven.model.Repository;
 import org.apache.maven.model.RepositoryPolicy;
+import org.apache.maven.repository.ArtifactDoesNotExistException;
+import org.apache.maven.repository.ArtifactTransferFailedException;
+import org.apache.maven.repository.ArtifactTransferListener;
 import org.apache.maven.repository.DelegatingLocalArtifactRepository;
 import org.apache.maven.repository.LocalArtifactRepository;
-import org.apache.maven.repository.ArtifactTransferListener;
 import org.apache.maven.repository.MirrorSelector;
 import org.apache.maven.repository.Proxy;
 import org.apache.maven.repository.RepositorySystem;
-import org.apache.maven.repository.ArtifactDoesNotExistException;
-import org.apache.maven.repository.ArtifactTransferFailedException;
+import org.apache.maven.repository.legacy.repository.ArtifactRepositoryFactory;
 import org.apache.maven.settings.Mirror;
 import org.apache.maven.settings.Server;
 import org.apache.maven.settings.building.SettingsProblem;
@@ -161,14 +160,7 @@ public class LegacyRepositorySystem
 
         if ( !d.getExclusions().isEmpty() )
         {
-            List<String> exclusions = new ArrayList<>();
-
-            for ( Exclusion exclusion : d.getExclusions() )
-            {
-                exclusions.add( exclusion.getGroupId() + ':' + exclusion.getArtifactId() );
-            }
-
-            artifact.setDependencyFilter( new ExcludesArtifactFilter( exclusions ) );
+            artifact.setDependencyFilter( ExcludesArtifactFilter.forExclusions( d.getExclusions() ) );
         }
 
         return artifact;
diff --git a/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/ExcludesArtifactFilter.java b/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/ExcludesArtifactFilter.java
index e448af4..79aa07b 100644
--- a/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/ExcludesArtifactFilter.java
+++ b/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/ExcludesArtifactFilter.java
@@ -19,24 +19,43 @@ package org.apache.maven.artifact.resolver.filter;
  * under the License.
  */
 
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 import org.apache.maven.artifact.Artifact;
+import org.apache.maven.model.Exclusion;
 
 /**
  * Filter to exclude from a list of artifact patterns.
  *
  * @author <a href="mailto:brett@apache.org">Brett Porter</a>
- * TODO I think this is equiv. to exclusion set filter in maven-core
  */
 public class ExcludesArtifactFilter
-    extends IncludesArtifactFilter
+        extends IncludesArtifactFilter
 {
+
+    public static ArtifactFilter forExclusions( List<Exclusion> exclusions )
+    {
+        List<ModuleIdentifier> modules = new ArrayList<>();
+
+        for ( Exclusion exclusion : exclusions )
+        {
+            modules.add( new ModuleIdentifier( exclusion.getGroupId(), exclusion.getArtifactId() ) );
+        }
+        return new ExcludesArtifactFilter( modules );
+    }
+
     public ExcludesArtifactFilter( List<String> patterns )
     {
         super( patterns );
     }
 
+    private ExcludesArtifactFilter( Collection<ModuleIdentifier> modules )
+    {
+        super( modules );
+    }
+
     public boolean include( Artifact artifact )
     {
         return !super.include( artifact );
diff --git a/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/IncludesArtifactFilter.java b/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/IncludesArtifactFilter.java
index 7a3b6c7..6a4dd01 100644
--- a/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/IncludesArtifactFilter.java
+++ b/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/IncludesArtifactFilter.java
@@ -20,7 +20,7 @@ package org.apache.maven.artifact.resolver.filter;
  */
 
 import java.util.ArrayList;
-import java.util.Iterator;
+import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
@@ -33,43 +33,43 @@ import org.apache.maven.artifact.Artifact;
  * @author <a href="mailto:brett@apache.org">Brett Porter</a>
  */
 public class IncludesArtifactFilter
-    implements ArtifactFilter
+        implements ArtifactFilter
 {
-    private final Set<String> patterns;
+    private final Set<ModuleIdentifier> modules;
 
     public IncludesArtifactFilter( List<String> patterns )
     {
-        this.patterns = new LinkedHashSet<>( patterns );
+        this.modules = new LinkedHashSet<>();
+        for ( String pattern : patterns )
+        {
+            this.modules.add( ModuleIdentifier.fromString( pattern ) );
+        }
     }
 
-    public boolean include( Artifact artifact )
+    IncludesArtifactFilter( Collection<ModuleIdentifier> modules )
     {
-        String id = artifact.getGroupId() + ":" + artifact.getArtifactId();
+        this.modules = new LinkedHashSet<>( modules );
+    }
 
-        boolean matched = false;
-        for ( Iterator<String> i = patterns.iterator(); i.hasNext() & !matched; )
-        {
-            // TODO what about wildcards? Just specifying groups? versions?
-            if ( id.equals( i.next() ) )
-            {
-                matched = true;
-            }
-        }
-        return matched;
+    public boolean include( Artifact artifact )
+    {
+        return modules.contains( new ModuleIdentifier( artifact.getGroupId(), artifact.getArtifactId() ) );
     }
 
     public List<String> getPatterns()
     {
-        return new ArrayList<>( patterns );
+        List<String> result = new ArrayList<>();
+        for ( ModuleIdentifier module : modules )
+        {
+            result.add( module.toString() );
+        }
+        return result;
     }
 
     @Override
     public int hashCode()
     {
-        int hash = 17;
-        hash = hash * 31 + patterns.hashCode();
-
-        return hash;
+        return modules.hashCode();
     }
 
     @Override
@@ -80,7 +80,6 @@ public class IncludesArtifactFilter
             return true;
         }
 
-        // make sure IncludesArtifactFilter is not equal ExcludesArtifactFilter!
         if ( obj == null || getClass() != obj.getClass() )
         {
             return false;
@@ -88,6 +87,6 @@ public class IncludesArtifactFilter
 
         IncludesArtifactFilter other = (IncludesArtifactFilter) obj;
 
-        return patterns.equals( other.patterns );
+        return modules.equals( other.modules );
     }
 }
diff --git a/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/ModuleIdentifier.java b/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/ModuleIdentifier.java
new file mode 100644
index 0000000..5afe4af
--- /dev/null
+++ b/maven-core/src/main/java/org/apache/maven/artifact/resolver/filter/ModuleIdentifier.java
@@ -0,0 +1,81 @@
+package org.apache.maven.artifact.resolver.filter;
+
+/*
+ * 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.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+
+class ModuleIdentifier
+{
+
+    static ModuleIdentifier fromString( String s )
+    {
+        String[] split = StringUtils.split( s, ':' );
+        return new ModuleIdentifier( split[ 0 ], split.length > 1 ? split[ 1 ] : "" );
+    }
+
+    private final String groupId;
+    private final String artifactId;
+
+    ModuleIdentifier( String groupId, String artifactId )
+    {
+        this.groupId = groupId;
+        this.artifactId = artifactId;
+    }
+
+    public String getGroupId()
+    {
+        return groupId;
+    }
+
+    public String getArtifactId()
+    {
+        return artifactId;
+    }
+
+    @Override
+    public boolean equals( Object o )
+    {
+        if ( this == o )
+        {
+            return true;
+        }
+        if ( o == null || getClass() != o.getClass() )
+        {
+            return false;
+        }
+        ModuleIdentifier that = (ModuleIdentifier) o;
+        return groupId.equals( that.groupId )
+                && artifactId.equals( that.artifactId );
+    }
+
+    @Override
+    public int hashCode()
+    {
+        return Objects.hash( groupId, artifactId );
+    }
+
+    @Override
+    public String toString()
+    {
+        return groupId + ( artifactId.isEmpty() ? "" : ":" + artifactId );
+    }
+}
diff --git a/maven-core/src/main/java/org/apache/maven/bridge/MavenRepositorySystem.java b/maven-core/src/main/java/org/apache/maven/bridge/MavenRepositorySystem.java
index 8558ae4..9f44a36 100644
--- a/maven-core/src/main/java/org/apache/maven/bridge/MavenRepositorySystem.java
+++ b/maven-core/src/main/java/org/apache/maven/bridge/MavenRepositorySystem.java
@@ -49,7 +49,6 @@ import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException
 import org.apache.maven.artifact.versioning.VersionRange;
 import org.apache.maven.execution.MavenExecutionRequest;
 import org.apache.maven.model.Dependency;
-import org.apache.maven.model.Exclusion;
 import org.apache.maven.model.Plugin;
 import org.apache.maven.repository.Proxy;
 import org.apache.maven.repository.RepositorySystem;
@@ -116,14 +115,7 @@ public class MavenRepositorySystem
 
         if ( !d.getExclusions().isEmpty() )
         {
-            List<String> exclusions = new ArrayList<>();
-
-            for ( Exclusion exclusion : d.getExclusions() )
-            {
-                exclusions.add( exclusion.getGroupId() + ':' + exclusion.getArtifactId() );
-            }
-
-            artifact.setDependencyFilter( new ExcludesArtifactFilter( exclusions ) );
+            artifact.setDependencyFilter( ExcludesArtifactFilter.forExclusions( d.getExclusions() ) );
         }
 
         return artifact;
diff --git a/maven-core/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java b/maven-core/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
index 2dc9372..48b38fb 100644
--- a/maven-core/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
+++ b/maven-core/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java
@@ -55,7 +55,6 @@ import org.apache.maven.artifact.versioning.VersionRange;
 import org.apache.maven.model.Dependency;
 import org.apache.maven.model.DependencyManagement;
 import org.apache.maven.model.DistributionManagement;
-import org.apache.maven.model.Exclusion;
 import org.apache.maven.model.Relocation;
 import org.apache.maven.model.building.ModelBuildingException;
 import org.apache.maven.model.building.ModelBuildingRequest;
@@ -394,14 +393,7 @@ public class MavenMetadataSource
 
         if ( !dependency.getExclusions().isEmpty() )
         {
-            List<String> exclusions = new ArrayList<>();
-
-            for ( Exclusion e : dependency.getExclusions() )
-            {
-                exclusions.add( e.getGroupId() + ':' + e.getArtifactId() );
-            }
-
-            effectiveFilter = new ExcludesArtifactFilter( exclusions );
+            effectiveFilter = ExcludesArtifactFilter.forExclusions( dependency.getExclusions() );
 
             if ( inheritedFilter != null )
             {