You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@karaf.apache.org by cs...@apache.org on 2017/08/21 08:25:03 UTC

[1/3] karaf git commit: KARAF-5314 Added a features cache that increases the performance of the filtering of the required features from the available features.

Repository: karaf
Updated Branches:
  refs/heads/master 658ee488f -> 5a8133dbb


KARAF-5314 Added a features cache that increases the performance of the filtering of the required features from the available features.

The performance of the filtering logic decresed when the java streams API was used in 4.1 compared to for loops in 4.0. I reverted the filtering logic to use for loops and also introduced a features cache. This increased the performance by a huge margin, almost 20X faster when there are ~900 available features and ~300 required features from a highly complex and huge feature dependency tree.


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/3cbd810e
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/3cbd810e
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/3cbd810e

Branch: refs/heads/master
Commit: 3cbd810ea37e837950f1f8427f793c878423ba7a
Parents: 658ee48
Author: Vinay Shankar (e24113) <Vi...@cmegroup.com>
Authored: Fri Aug 18 13:55:58 2017 -0500
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Fri Aug 18 23:22:25 2017 +0200

----------------------------------------------------------------------
 .../apache/karaf/profile/assembly/Builder.java  | 38 +++++++++++++++-----
 1 file changed, 30 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/3cbd810e/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
----------------------------------------------------------------------
diff --git a/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java b/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
index 27b83a6..e9cc67e 100644
--- a/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
+++ b/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
@@ -39,6 +39,7 @@ import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
@@ -49,7 +50,6 @@ import java.util.function.Function;
 import java.util.jar.Attributes;
 import java.util.jar.Manifest;
 import java.util.regex.Pattern;
-import java.util.stream.Collectors;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
@@ -805,10 +805,11 @@ public class Builder {
             allInstalledFeatures.addAll(repo.getFeature());
         }
         Set<Feature> installedFeatures = new LinkedHashSet<>();
+        Map<String, Map<Version, Feature>> featuresCache = new HashMap<>();
         // Add boot features for search
         allInstalledFeatures.addAll(allBootFeatures);
         for (String feature : installedEffective.getFeatures()) {
-            addFeatures(allInstalledFeatures, feature, installedFeatures, true);
+            addFeatures(allInstalledFeatures, feature, installedFeatures, true, featuresCache);
         }
         ArtifactInstaller installer = new ArtifactInstaller(systemDirectory, downloader, blacklistedBundles);
         for (Feature feature : installedFeatures) {
@@ -882,7 +883,8 @@ public class Builder {
 
         // Compute startup feature dependencies
         Set<Feature> bootFeatures = new HashSet<>();
-        addFeatures(allBootFeatures, generated.getName(), bootFeatures, true);
+        Map<String, Map<Version, Feature>> featuresCache = new HashMap<>();
+        addFeatures(allBootFeatures, generated.getName(), bootFeatures, true, featuresCache);
         for (Feature feature : bootFeatures) {
             // the feature is a startup feature, updating startup.properties file
             LOGGER.info("   Feature " + feature.getId() + " is defined as a boot feature");
@@ -1239,14 +1241,16 @@ public class Builder {
         return startupEffective;
     }
 
-    private void addFeatures(Set<Feature> allFeatures, String feature, Set<Feature> features, boolean mandatory) {
+    private void addFeatures(Set<Feature> allFeatures, String feature, Set<Feature> features, boolean mandatory, Map<String, Map<Version, Feature>> featuresCache) {
         String name;
+        Version osgiVersion;
         VersionRange range;
         int idx = feature.indexOf('/');
         if (idx > 0) {
             name = feature.substring(0, idx);
             String version = feature.substring(idx + 1);
             version = version.trim();
+            osgiVersion = VersionTable.getVersion(version);
             if (version.equals(org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION)) {
                 range = new VersionRange(Version.emptyVersion);
             } else {
@@ -1254,18 +1258,36 @@ public class Builder {
             }
         } else {
             name = feature;
+            osgiVersion = Version.emptyVersion;
             range = new VersionRange(Version.emptyVersion);
         }
-        Set<Feature> set = allFeatures.stream()
-                .filter(f -> f.getName().equals(name) && range.contains(VersionTable.getVersion(f.getVersion())))
-                .collect(Collectors.toSet());
+        Set<Feature> set = new HashSet<>();
+        boolean featurePresentInCache = false;
+        Optional<Map<Version, Feature>> optionalVersionFeatureMap = Optional.ofNullable(featuresCache.get(name));
+        if(optionalVersionFeatureMap.isPresent()) {
+            Optional<Feature> cachedFeature = Optional.ofNullable(optionalVersionFeatureMap.get().get(osgiVersion));
+            if(cachedFeature.isPresent()){
+                set.add(cachedFeature.get());
+                featurePresentInCache = true;
+            }
+        }
+        if(!featurePresentInCache) {
+            for (Feature f : allFeatures) {
+                if (f.getName().equals(name) && range.contains(VersionTable.getVersion(f.getVersion()))) {
+                    set.add(f);
+                    Map<Version, Feature> versionFeatureMap = Optional.ofNullable(featuresCache.get(name)).orElse(new HashMap<>());
+                    versionFeatureMap.put(osgiVersion, f);
+                    featuresCache.put(name, versionFeatureMap);
+                }
+            }
+        }
         if (mandatory && set.isEmpty()) {
             throw new IllegalStateException("Could not find matching feature for " + feature);
         }
         for (Feature f : set) {
             features.add(f);
             for (Dependency dep : f.getFeature()) {
-                addFeatures(allFeatures, dep.toString(), features, !dep.isDependency() && !dep.isPrerequisite());
+                addFeatures(allFeatures, dep.toString(), features, !dep.isDependency() && !dep.isPrerequisite(), featuresCache);
             }
         }
     }


[2/3] karaf git commit: [KARAF-5314] Extract addFeatures into class FeatureSelector

Posted by cs...@apache.org.
[KARAF-5314] Extract addFeatures into class FeatureSelector


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/0ed141da
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/0ed141da
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/0ed141da

Branch: refs/heads/master
Commit: 0ed141dafd5ffb321bc851886e88457b616c1c06
Parents: 3cbd810
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Sun Aug 20 12:14:31 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Sun Aug 20 12:14:31 2017 +0200

----------------------------------------------------------------------
 .../apache/karaf/profile/assembly/Builder.java  | 71 ++-------------
 .../karaf/profile/assembly/FeatureSelector.java | 94 ++++++++++++++++++++
 2 files changed, 100 insertions(+), 65 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/0ed141da/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
----------------------------------------------------------------------
diff --git a/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java b/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
index e9cc67e..2fdd5f0 100644
--- a/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
+++ b/profile/src/main/java/org/apache/karaf/profile/assembly/Builder.java
@@ -39,7 +39,6 @@ import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
@@ -56,8 +55,6 @@ import java.util.zip.ZipInputStream;
 import org.apache.felix.resolver.ResolverImpl;
 import org.apache.felix.utils.manifest.Clause;
 import org.apache.felix.utils.properties.Properties;
-import org.apache.felix.utils.version.VersionRange;
-import org.apache.felix.utils.version.VersionTable;
 import org.apache.karaf.features.FeaturesService;
 import org.apache.karaf.features.Library;
 import org.apache.karaf.features.internal.download.DownloadCallback;
@@ -89,13 +86,13 @@ import org.apache.karaf.util.maven.Parser;
 import org.ops4j.pax.url.mvn.MavenResolver;
 import org.ops4j.pax.url.mvn.MavenResolvers;
 import org.osgi.framework.Constants;
-import org.osgi.framework.Version;
 import org.osgi.framework.wiring.BundleRevision;
 import org.osgi.resource.Resource;
 import org.osgi.service.resolver.Resolver;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static java.util.Collections.singletonList;
 import static java.util.jar.JarFile.MANIFEST_NAME;
 import static org.apache.karaf.features.internal.service.Blacklist.TYPE_REPOSITORY;
 import static org.apache.karaf.profile.assembly.Builder.Stage.Startup;
@@ -804,13 +801,11 @@ public class Builder {
         for (Features repo : installedRepositories.values()) {
             allInstalledFeatures.addAll(repo.getFeature());
         }
-        Set<Feature> installedFeatures = new LinkedHashSet<>();
-        Map<String, Map<Version, Feature>> featuresCache = new HashMap<>();
+        
         // Add boot features for search
         allInstalledFeatures.addAll(allBootFeatures);
-        for (String feature : installedEffective.getFeatures()) {
-            addFeatures(allInstalledFeatures, feature, installedFeatures, true, featuresCache);
-        }
+        FeatureSelector selector = new FeatureSelector(allInstalledFeatures);
+        Set<Feature> installedFeatures = selector.selectMatching(installedEffective.getFeatures());
         ArtifactInstaller installer = new ArtifactInstaller(systemDirectory, downloader, blacklistedBundles);
         for (Feature feature : installedFeatures) {
             LOGGER.info("   Feature {} is defined as an installed feature", feature.getId());
@@ -882,9 +877,8 @@ public class Builder {
         Downloader downloader = manager.createDownloader();
 
         // Compute startup feature dependencies
-        Set<Feature> bootFeatures = new HashSet<>();
-        Map<String, Map<Version, Feature>> featuresCache = new HashMap<>();
-        addFeatures(allBootFeatures, generated.getName(), bootFeatures, true, featuresCache);
+        FeatureSelector selector = new FeatureSelector(allBootFeatures);
+        Set<Feature> bootFeatures = selector.selectMatching(singletonList(generated.getName()));
         for (Feature feature : bootFeatures) {
             // the feature is a startup feature, updating startup.properties file
             LOGGER.info("   Feature " + feature.getId() + " is defined as a boot feature");
@@ -1241,57 +1235,6 @@ public class Builder {
         return startupEffective;
     }
 
-    private void addFeatures(Set<Feature> allFeatures, String feature, Set<Feature> features, boolean mandatory, Map<String, Map<Version, Feature>> featuresCache) {
-        String name;
-        Version osgiVersion;
-        VersionRange range;
-        int idx = feature.indexOf('/');
-        if (idx > 0) {
-            name = feature.substring(0, idx);
-            String version = feature.substring(idx + 1);
-            version = version.trim();
-            osgiVersion = VersionTable.getVersion(version);
-            if (version.equals(org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION)) {
-                range = new VersionRange(Version.emptyVersion);
-            } else {
-                range = new VersionRange(version, true, true);
-            }
-        } else {
-            name = feature;
-            osgiVersion = Version.emptyVersion;
-            range = new VersionRange(Version.emptyVersion);
-        }
-        Set<Feature> set = new HashSet<>();
-        boolean featurePresentInCache = false;
-        Optional<Map<Version, Feature>> optionalVersionFeatureMap = Optional.ofNullable(featuresCache.get(name));
-        if(optionalVersionFeatureMap.isPresent()) {
-            Optional<Feature> cachedFeature = Optional.ofNullable(optionalVersionFeatureMap.get().get(osgiVersion));
-            if(cachedFeature.isPresent()){
-                set.add(cachedFeature.get());
-                featurePresentInCache = true;
-            }
-        }
-        if(!featurePresentInCache) {
-            for (Feature f : allFeatures) {
-                if (f.getName().equals(name) && range.contains(VersionTable.getVersion(f.getVersion()))) {
-                    set.add(f);
-                    Map<Version, Feature> versionFeatureMap = Optional.ofNullable(featuresCache.get(name)).orElse(new HashMap<>());
-                    versionFeatureMap.put(osgiVersion, f);
-                    featuresCache.put(name, versionFeatureMap);
-                }
-            }
-        }
-        if (mandatory && set.isEmpty()) {
-            throw new IllegalStateException("Could not find matching feature for " + feature);
-        }
-        for (Feature f : set) {
-            features.add(f);
-            for (Dependency dep : f.getFeature()) {
-                addFeatures(allFeatures, dep.toString(), features, !dep.isDependency() && !dep.isPrerequisite(), featuresCache);
-            }
-        }
-    }
-
     private List<String> getStaged(Stage stage, Map<String, Stage> data) {
         List<String> staged = new ArrayList<>();
         for (String s : data.keySet()) {
@@ -1488,6 +1431,4 @@ public class Builder {
         throw new IllegalArgumentException("Resource " + provider.getUrl() + " does not contain a manifest");
     }
 
-
-
 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/0ed141da/profile/src/main/java/org/apache/karaf/profile/assembly/FeatureSelector.java
----------------------------------------------------------------------
diff --git a/profile/src/main/java/org/apache/karaf/profile/assembly/FeatureSelector.java b/profile/src/main/java/org/apache/karaf/profile/assembly/FeatureSelector.java
new file mode 100644
index 0000000..1e453d6
--- /dev/null
+++ b/profile/src/main/java/org/apache/karaf/profile/assembly/FeatureSelector.java
@@ -0,0 +1,94 @@
+/*
+ * 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.profile.assembly;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.felix.utils.version.VersionRange;
+import org.apache.felix.utils.version.VersionTable;
+import org.apache.karaf.features.internal.model.Dependency;
+import org.apache.karaf.features.internal.model.Feature;
+import org.osgi.framework.Version;
+
+public class FeatureSelector {
+    Map<String, Set<Feature>> featuresCache;
+
+    public FeatureSelector(Set<Feature> features) {
+        featuresCache = new HashMap<>();
+        for (Feature feature : features) {
+            featuresCache.computeIfAbsent(feature.getName(), fn -> new HashSet<>())
+                .add(feature);
+        }
+    }
+    
+    public Set<Feature> selectMatching(List<String> features) {
+        Set<Feature> selected = new HashSet<>();
+        for (String feature : features) {
+            addFeatures(feature, selected, true);
+        }
+        return selected;
+    }
+    
+    private void addFeatures(String feature, Set<Feature> features, boolean mandatory) {
+        Set<Feature> set = getMatching(feature);
+        if (mandatory && set.isEmpty()) {
+            throw new IllegalStateException("Could not find matching feature for " + feature);
+        }
+        for (Feature f : set) {
+            features.add(f);
+            for (Dependency dep : f.getFeature()) {
+                addFeatures(dep.toString(), features, isMandatory(dep));
+            }
+        }
+    }
+
+    private boolean isMandatory(Dependency dep) {
+        return !dep.isDependency() && !dep.isPrerequisite();
+    }
+
+    private Set<Feature> getMatching(String nameAndVersion) {
+        VersionRange range;
+        String name;
+        int idx = nameAndVersion.indexOf('/');
+        if (idx > 0) {
+            name = nameAndVersion.substring(0, idx);
+            String version = nameAndVersion.substring(idx + 1);
+            version = version.trim();
+            if (version.equals(org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION)) {
+                range = new VersionRange(Version.emptyVersion);
+            } else {
+                range = new VersionRange(version, true, true);
+            }
+        } else {
+            name = nameAndVersion;
+            range = new VersionRange(Version.emptyVersion);
+        }
+        Set<Feature> versionToFeatures = featuresCache.get(name);
+        if (versionToFeatures == null) {
+            return Collections.emptySet();
+        }
+        return versionToFeatures.stream()
+            .filter(f -> f.getName().equals(name) && range.contains(VersionTable.getVersion(f.getVersion())))
+            .collect(Collectors.toSet());  
+    }
+}


[3/3] karaf git commit: [KARAF-5314] Only recurse into dependencies once per feature

Posted by cs...@apache.org.
[KARAF-5314] Only recurse into dependencies once per feature


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/5a8133db
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/5a8133db
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/5a8133db

Branch: refs/heads/master
Commit: 5a8133dbbfe7ede7366bcb943e2bd04937d8eff5
Parents: 0ed141d
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Sun Aug 20 16:20:09 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Sun Aug 20 16:20:09 2017 +0200

----------------------------------------------------------------------
 .../org/apache/karaf/profile/assembly/FeatureSelector.java    | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/5a8133db/profile/src/main/java/org/apache/karaf/profile/assembly/FeatureSelector.java
----------------------------------------------------------------------
diff --git a/profile/src/main/java/org/apache/karaf/profile/assembly/FeatureSelector.java b/profile/src/main/java/org/apache/karaf/profile/assembly/FeatureSelector.java
index 1e453d6..156cb46 100644
--- a/profile/src/main/java/org/apache/karaf/profile/assembly/FeatureSelector.java
+++ b/profile/src/main/java/org/apache/karaf/profile/assembly/FeatureSelector.java
@@ -55,9 +55,10 @@ public class FeatureSelector {
             throw new IllegalStateException("Could not find matching feature for " + feature);
         }
         for (Feature f : set) {
-            features.add(f);
-            for (Dependency dep : f.getFeature()) {
-                addFeatures(dep.toString(), features, isMandatory(dep));
+            if (features.add(f)) {
+                for (Dependency dep : f.getFeature()) {
+                    addFeatures(dep.toString(), features, isMandatory(dep));
+                }
             }
         }
     }