karaf-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (KARAF-5604) karaf:features-generate-descriptor takes long when faced with complex feature dependencies
Date Fri, 09 Feb 2018 06:35:00 GMT

    [ https://issues.apache.org/jira/browse/KARAF-5604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16357983#comment-16357983
] 

ASF GitHub Bot commented on KARAF-5604:
---------------------------------------

jbonofre closed pull request #444: [KARAF-5604] Speed up features-generate-descriptor
URL: https://github.com/apache/karaf/pull/444
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/features/GenerateDescriptorMojo.java
b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/features/GenerateDescriptorMojo.java
index 896c12834e..f38c2be9be 100644
--- a/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/features/GenerateDescriptorMojo.java
+++ b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/features/GenerateDescriptorMojo.java
@@ -38,7 +38,6 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.WeakHashMap;
 import java.util.jar.JarInputStream;
 import java.util.jar.Manifest;
 
@@ -59,6 +58,7 @@
 import org.apache.karaf.tooling.utils.ManifestUtils;
 import org.apache.karaf.tooling.utils.MavenUtil;
 import org.apache.karaf.tooling.utils.MojoSupport;
+import org.apache.karaf.tooling.utils.SimpleLRUCache;
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.artifact.resolver.ArtifactNotFoundException;
 import org.apache.maven.artifact.resolver.ArtifactResolutionException;
@@ -90,7 +90,7 @@
 /**
  * Generates the features XML file starting with an optional source feature.xml and adding
  * project dependencies as bundles and feature/car dependencies.
- * 
+ *
  * NB this requires a recent maven-install-plugin such as 2.3.1
  */
 @Mojo(name = "features-generate-descriptor", defaultPhase = LifecyclePhase.COMPILE, requiresDependencyResolution
= ResolutionScope.RUNTIME, threadSafe = true)
@@ -234,14 +234,14 @@
      */
     @Parameter(defaultValue = "${project.artifactId}")
     private String primaryFeatureName;
-    
+
     /**
      * Flag indicating whether bundles should use the version range declared in the POM.
If <code>false</code>,
      * the actual version of the resolved artifacts will be used.
      */
     @Parameter(defaultValue = "false")
     private boolean useVersionRange;
-    
+
     /**
      * Flag indicating whether the plugin should determine whether transitive dependencies
are declared with
      * a version range. If this flag is set to <code>true</code> and a transitive
dependency has been found
@@ -263,6 +263,19 @@
     @Parameter(defaultValue = "false")
     private boolean simplifyBundleDependencies;
 
+    /**
+     * Maximum size of the artifact LRU cache. This cache is used to prevent repeated artifact-to-file
resolution.
+     */
+    @Parameter(defaultValue = "1024")
+    private int artifactCacheSize;
+
+    /**
+     * Maximum size of the Features LRU cache. This cache is used to prevent repeated deserialization
of features
+     * XML files.
+     */
+    @Parameter(defaultValue = "256")
+    private int featuresCacheSize;
+
     /**
      * Name of features which are prerequisites (they still need to be defined separately).
      */
@@ -285,7 +298,7 @@
      */
     @Component
     private PlexusContainer container;
-    
+
     @Component
     private RepositorySystem repoSystem;
 
@@ -294,7 +307,7 @@
 
     @Component
     protected MavenFileFilter mavenFileFilter;
-    
+
 	@Component
 	private ProjectBuilder mavenProjectBuilder;
 
@@ -309,11 +322,12 @@
 
     // maven log
     private Log log;
-    
+
     // If useVersionRange is true, this map will be used to cache
     // resolved MavenProjects
     private final Map<Artifact, MavenProject> resolvedProjects = new HashMap<>();
 
+    @Override
     public void execute() throws MojoExecutionException, MojoFailureException {
         try {
             if (enableGeneration == null) {
@@ -333,7 +347,8 @@ public void execute() throws MojoExecutionException, MojoFailureException
{
                 }
             }
 
-            this.dependencyHelper = DependencyHelperFactory.createDependencyHelper(this.container,
this.project, this.mavenSession, getLog());
+            this.dependencyHelper = DependencyHelperFactory.createDependencyHelper(this.container,
this.project,
+                this.mavenSession, this.artifactCacheSize, getLog());
             this.dependencyHelper.getDependencies(project, includeTransitiveDependency);
             this.localDependencies = dependencyHelper.getLocalDependencies();
             this.treeListing = dependencyHelper.getTreeListing();
@@ -359,11 +374,11 @@ private MavenProject resolveProject(final Object artifact) throws MojoExecutionE
 			resolvedProject = resolvedProjects.get(artifact);
 			if (resolvedProject == null) {
 				final ProjectBuildingRequest request = new DefaultProjectBuildingRequest();
-				
-				// Fixes KARAF-4626; if the system properties are not transferred to the request, 
+
+				// Fixes KARAF-4626; if the system properties are not transferred to the request,
 				// test-feature-use-version-range-transfer-properties will fail
 				request.setSystemProperties(System.getProperties());
-				
+
 				request.setResolveDependencies(true);
 				request.setRemoteRepositories(project.getPluginArtifactRepositories());
 				request.setLocalRepository(localRepo);
@@ -398,7 +413,7 @@ private String getVersionOrRange(final Object parent, final Object artifact)
thr
 		}
 		return versionOrRange;
 	}
-    
+
     /*
      * Write all project dependencies as feature
      */
@@ -456,7 +471,7 @@ private void writeFeatures(PrintStream out) throws ArtifactResolutionException,
         // TODO Initialise the repositories from the existing feature file if any
         Map<Dependency, Feature> otherFeatures = new HashMap<>();
         Map<Feature, String> featureRepositories = new HashMap<>();
-        FeaturesCache cache = new FeaturesCache();
+        FeaturesCache cache = new FeaturesCache(featuresCacheSize);
         for (final LocalDependency entry : localDependencies) {
             Object artifact = entry.getArtifact();
 
@@ -532,7 +547,7 @@ private void writeFeatures(PrintStream out) throws ArtifactResolutionException,
             wrapDependency.setPrerequisite(true);
             feature.getFeature().add(wrapDependency);
         }
-        
+
         if ((!feature.getBundle().isEmpty() || !feature.getFeature().isEmpty()) &&
!features.getFeature().contains(feature)) {
             features.getFeature().add(feature);
         }
@@ -668,14 +683,16 @@ private Manifest getManifest(File file) throws IOException {
         }
     }
 
-    private static Features readFeaturesFile(File featuresFile) throws XMLStreamException,
JAXBException, IOException {
+    static Features readFeaturesFile(File featuresFile) throws XMLStreamException, JAXBException,
IOException {
         return JaxbUtil.unmarshal(featuresFile.toURI().toASCIIString(), false);
     }
 
+    @Override
     public void setLog(Log log) {
         this.log = log;
     }
 
+    @Override
     public Log getLog() {
         if (log == null) {
             setLog(new SystemStreamLog());
@@ -927,16 +944,20 @@ protected String saveTreeListing() throws IOException {
     }
 
     private static final class FeaturesCache {
-        private final Map<File, Features> map = new WeakHashMap<>();
+        private final SimpleLRUCache<File, Features> cache;
+
+        FeaturesCache(int featuresCacheSize) {
+            cache = new SimpleLRUCache<>(featuresCacheSize);
+        }
 
         Features get(final File featuresFile) throws XMLStreamException, JAXBException, IOException
{
-            final Features existing = map.get(featuresFile);
+            final Features existing = cache.get(featuresFile);
             if (existing != null) {
                 return existing;
             }
 
             final Features computed = readFeaturesFile(featuresFile);
-            map.put(featuresFile, computed);
+            cache.put(featuresFile, computed);
             return computed;
         }
     }
diff --git a/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/Dependency31Helper.java
b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/Dependency31Helper.java
index d1bfb7055b..3c139008ff 100644
--- a/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/Dependency31Helper.java
+++ b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/Dependency31Helper.java
@@ -71,25 +71,32 @@
      */
     private final List<RemoteRepository> projectRepositories;
 
+    private final SimpleLRUCache<Artifact, ArtifactResult> artifactCache;
+
     // dependencies we are interested in
     protected Map<Artifact, LocalDependency> localDependencies;
     // log of what happened during search
     protected String treeListing;
 
     @SuppressWarnings("unchecked")
-    public Dependency31Helper(List<?> repositories, Object session, RepositorySystem
repositorySystem) {
+    public Dependency31Helper(List<?> repositories, Object session, RepositorySystem
repositorySystem, int cacheSize) {
         this.projectRepositories = (List<RemoteRepository>) repositories;
         this.repositorySystemSession = (RepositorySystemSession) session;
         this.repositorySystem = repositorySystem;
+        this.artifactCache = new SimpleLRUCache<>(cacheSize);
+    }
+
+    public Dependency31Helper(List<?> repositories, Object session, RepositorySystem
repositorySystem) {
+        this(repositories, session, repositorySystem, 32);
+    }
+
+    public void setRepositorySession(final ProjectBuildingRequest request) throws MojoExecutionException
{
+        try {
+            invokeMethod(request, "setRepositorySession", repositorySystemSession);
+        } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException
e) {
+            throw new MojoExecutionException("Cannot set repository session on project building
request", e);
+        }
     }
-    
-	public void setRepositorySession(final ProjectBuildingRequest request) throws MojoExecutionException
{
-		try {
-			invokeMethod(request, "setRepositorySession", repositorySystemSession);
-		} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e)
{
-			throw new MojoExecutionException("Cannot set repository session on project building request",
e);
-		}
-	}
 
     @Override
     public Collection<LocalDependency> getLocalDependencies() {
@@ -281,7 +288,7 @@ public static boolean isFeature(Artifact artifact) {
     public boolean isArtifactAFeature(Object artifact) {
         return Dependency31Helper.isFeature((Artifact) artifact);
     }
-    
+
 	@Override
 	public String getBaseVersion(Object artifact) {
 		return ((Artifact) artifact).getBaseVersion();
@@ -302,17 +309,30 @@ public String getClassifier(Object artifact) {
         return ((Artifact) artifact).getClassifier();
     }
 
-    @Override
-    public File resolve(Object artifact, Log log) {
+    private ArtifactResult resolveArtifact(Artifact artifact) throws ArtifactResolutionException
{
+        ArtifactResult result = artifactCache.get(artifact);
+        if (result != null) {
+            return result;
+        }
+
         ArtifactRequest request = new ArtifactRequest();
-        request.setArtifact((Artifact) artifact);
+        request.setArtifact(artifact);
         request.setRepositories(projectRepositories);
 
+        result = repositorySystem.resolveArtifact(repositorySystemSession, request);
+        if (result != null) {
+            artifactCache.put(artifact, result);
+        }
+        return result;
+    }
+
+    @Override
+    public File resolve(Object artifact, Log log) {
         log.debug("Resolving artifact " + artifact + " from " + projectRepositories);
 
         ArtifactResult result;
         try {
-            result = repositorySystem.resolveArtifact(repositorySystemSession, request);
+            result = resolveArtifact((Artifact) artifact);
         } catch (ArtifactResolutionException e) {
             log.warn("Cound not resolve " + artifact, e);
             return null;
@@ -334,15 +354,12 @@ public File resolveById(String id, Log log) throws MojoFailureException
{
             }
         }
         id = MavenUtil.mvnToAether(id);
-        ArtifactRequest request = new ArtifactRequest();
-        request.setArtifact(new DefaultArtifact(id));
-        request.setRepositories(projectRepositories);
 
         log.debug("Resolving artifact " + id + " from " + projectRepositories);
 
         ArtifactResult result;
         try {
-            result = repositorySystem.resolveArtifact(repositorySystemSession, request);
+            result = resolveArtifact(new DefaultArtifact(id));
         } catch (ArtifactResolutionException e) {
             log.warn("Could not resolve " + id, e);
             throw new MojoFailureException(format("Couldn't resolve artifact %s", id), e);
diff --git a/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/DependencyHelperFactory.java
b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/DependencyHelperFactory.java
index d091c960ea..52bea523f6 100644
--- a/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/DependencyHelperFactory.java
+++ b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/DependencyHelperFactory.java
@@ -44,6 +44,7 @@
      * @param container    The Maven Plexus container to use.
      * @param mavenProject The Maven project to use.
      * @param mavenSession The Maven session.
+     * @param cacheSize    Size of the artifact->file LRU cache
      * @param log          The log to use for the messages.
      *
      * @return The {@link DependencyHelper} depending of the Maven version used.
@@ -51,13 +52,13 @@
      * @throws MojoExecutionException If the plugin execution fails.
      */
     public static DependencyHelper createDependencyHelper(
-            PlexusContainer container, MavenProject mavenProject, MavenSession mavenSession,
Log log
-                                                         ) throws MojoExecutionException
{
+            PlexusContainer container, MavenProject mavenProject, MavenSession mavenSession,
int cacheSize, Log log
+            ) throws MojoExecutionException {
         try {
             final RepositorySystem system = container.lookup(RepositorySystem.class);
             final RepositorySystemSession session = mavenSession.getRepositorySession();
             final List<RemoteRepository> repositories = mavenProject.getRemoteProjectRepositories();
-            return new Dependency31Helper(repositories, session, system);
+            return new Dependency31Helper(repositories, session, system, cacheSize);
         } catch (ComponentLookupException e) {
             throw new MojoExecutionException(e.getMessage(), e);
         }
diff --git a/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/SimpleLRUCache.java
b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/SimpleLRUCache.java
new file mode 100644
index 0000000000..98ad90f494
--- /dev/null
+++ b/tooling/karaf-maven-plugin/src/main/java/org/apache/karaf/tooling/utils/SimpleLRUCache.java
@@ -0,0 +1,45 @@
+/**
+ *
+ * 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.karaf.tooling.utils;
+
+import java.util.LinkedHashMap;
+import java.util.Map.Entry;
+
+/**
+ * A very simplistic LRU cache based on LinkedHashMap. It grows up to the size specified
in the constructor,
+ * evicting least recently accessed entries to keep the size there.
+ */
+public final class SimpleLRUCache<K, V> extends LinkedHashMap<K, V> {
+    private static final long serialVersionUID = 1L;
+
+    private final int maxEntries;
+
+    public SimpleLRUCache(int maxEntries) {
+        this(16, 0.75f, maxEntries);
+    }
+
+    public SimpleLRUCache(int initialSize, float loadFactor, int maxEntries) {
+        super(initialSize, loadFactor, true);
+        this.maxEntries = maxEntries;
+    }
+
+    @Override
+    protected boolean removeEldestEntry(Entry<K, V> eldest) {
+        return size() > maxEntries;
+    }
+}


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> karaf:features-generate-descriptor takes long when faced with complex feature dependencies
> ------------------------------------------------------------------------------------------
>
>                 Key: KARAF-5604
>                 URL: https://issues.apache.org/jira/browse/KARAF-5604
>             Project: Karaf
>          Issue Type: Improvement
>          Components: karaf-tooling
>    Affects Versions: 4.1.3
>            Reporter: Robert Varga
>            Assignee: Jean-Baptiste Onofré
>            Priority: Major
>             Fix For: 4.2.0, 4.1.5
>
>
> Opendaylight's distribution-check jobs generate features which have complex feature dependencies,
which exposes scaling issues in karaf:features-generate-descriptor.
> [https://github.com/opendaylight/integration-distribution/tree/master/features/singles/odl-integration-all]
takes ~270 seconds to generate:
> real 4m28.834s
> user 3m40.287s
> sys 1m23.629s
> [https://github.com/opendaylight/integration-distribution/tree/master/features/repos/index]
takes ~638 seconds to generate:
> real 10m38.859s
> user 7m55.004s
> sys 3m17.269s
> Running profiling shows that this time is dominated by short-lived FileInputStreams being
generated at a rate of 7K-8Kps – which are coming from both feature reading and from artifact
resolution.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message