You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2022/05/10 07:53:43 UTC

[GitHub] [netbeans] mbien opened a new pull request, #4095: maven indexing: local repo indexing optimizations

mbien opened a new pull request, #4095:
URL: https://github.com/apache/netbeans/pull/4095

   flamegraphs showed that ClassDependencyIndexCreator.addDependencyToMap
   had some potential for optimizations.
   
    - use stream instead of HashSet which gives a very small perf improvement
      but also slightly more readable code IMO
    - use simple startsWith() loop for prefix filtering instead of
      stateful impl (MatchWords), the JVM is likely very good at
      optimizing/vectorizing String intrinsics
   
   benchmark showed ~20% speedup
   
   full indexing takes on my system and with my current local maven repo ~53s (down from ~66s)
   
   @timboudreau what is your opinion on this? You wrote most of the original optimizations (including the `MatchWords` class). I can't see you in the reviewer list for some reason.
   
   sidenote:
   I did also write a parallel version using an `ExecutorService` which could reduce it further to ~38s, but it made the code slightly more complex which is probably not worth the trouble. Parallelizing on a higher level (outside of `ClassDependencyIndexCreator`) might give better results (better scaling) since this is only a small slice of the task.
   
   flamegraphs:
   ![stream_match](https://user-images.githubusercontent.com/114367/167572356-dfd1bd71-56ac-4b47-99ee-3c19cac45541.png)
   ![stream_startWith](https://user-images.githubusercontent.com/114367/167572372-734a556f-e413-4fea-98fb-10934d4c7797.png)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869969952


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   @vieiro prefix tree, thats the name, now I remember! I might give it a try but I am skeptical that the results will impress :)
    - even if we make this method a no-op, it won't help a lot performance wise. There is a lot more going on, the screenshots don't show the full graph, it is zoomed into a particular method.
    - string ops like startWith are super fast, they are all intrinsified and can be compiled to vector operations
    - the list is likely not big enough to justify the overhead of a more sophisticated data structure with HasMap lookups and object trees
    
   here is the full tree for the thread, marked in red is the method post-optimization, even if the marked segment wouldn't be there, it would maybe only reduce it down to 50s or so. My hope is multithreading on a higher level to do the rest. This PR is a complete accident btw, I actually wanted to optimize the remote repo indexing which takes significantly longer.
   ![full](https://user-images.githubusercontent.com/114367/167792577-a46a9538-a2b3-4afc-975f-ace9dfadb778.png)
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] vieiro commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
vieiro commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869920081


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   Nothing lke a cup of coffee after a good sleep to see things clear :-) 
   We want to stop iterating if the `JDK_CLASS_TEST[i]` is greater than the `referee`, that is, your `< 0`, that gains us little.
   The best data structure we could use is a [prefix tree/trie](https://www.baeldung.com/cs/tries-prefix-trees) (there's a  [Java implementation here](https://www.baeldung.com/trie-java). That's the fastest one we could use, we could create it once and finds would then be in `O(n)`, where `n` is the length of `referee`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r879707469


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   i am going to add the braces the next time I look at this class if I remember. I usually try to keep Stream expressions as compact as possible, in this case it would be even clearer if the filter lambda is extracted to a method - with all the braces there.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] vieiro commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
vieiro commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869942376


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   @mbien  D'oh, we do have a trie in the NetBeans source code base [StringPrefixTree](https://github.com/apache/netbeans/blob/master/enterprise/payara.tooling/src/org/netbeans/modules/payara/tooling/utils/StringPrefixTree.java)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#issuecomment-1133837557

   rebased to latest master so that its easier to test together with the latest fix #4136


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869969952


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   @vieiro prefix tree, thats the name, now I remember! I might give it a try but I am skeptical that the results will impress :)
    - even if we make this method a no-op, it won't help a lot performance wise. There is a lot more going on, the screenshots don't show the full graph, it is zoomed into a particular method.
    - string ops like startWith are super fast, they are all intrinsified and can be compiled to vector operations
    - the list is likely not big enough to justify the overhead of a more sophisticated data structure with HasMap lookups and object trees
    
   here is the full tree for the thread, marked in red is post-optimization, even if the marked segment wouldn't be there, it would maybe only reduce it down to 50s or so. My hope is multithreading on a higher level to do the rest. This PR is a complete accident btw, I actually wanted to optimize the remote repo indexing which takes significantly longer.
   ![full](https://user-images.githubusercontent.com/114367/167792577-a46a9538-a2b3-4afc-975f-ace9dfadb778.png)
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] vieiro commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
vieiro commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869657209


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   @mbien I was meaning to check for a prefix first, and to discard if `referee > JDK_TEST_CLASSES[i]`.
   
   I was thinking something like this (can't think clearly now, though, dinner time!):
   
   ```java
   .filter((referee) -> {
           for (int i = 0; i < JDK_CLASS_TEST.length; i++) {
               if (referee.startsWith(JDK_CLASS_TEST[i])) {
                   // We've found a prefix!
                   return false;
               }
               if (referee.compareTo(JDK_CLASS_TEST[i]) > 0) {
                   // referee is bigger than JDK_CLASS_TEST[i]
                   // but JDK_CLASS_TEST[i] is not a prefix
                   // so it's not worth iterating for i+1 ... JDK_CLASS_TEST.length
                   return true;
               }
           }
           return true;
       })
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869643642


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   update:
   * pattern `"^(prefix1|prefix2...)"` is slower (phew that would have been embarrassing) **(~67s)**
   * array sorted by String length and added fast path results in the same time **(53s)**
   ```java
   static{
       Arrays.sort(JDK_CLASS_TEST, (p1, p2) -> p1.length() - p2.length());
   }
   ...
       .filter((referee) -> {
           for (int i = 0; i < JDK_CLASS_TEST.length; i++) {
               if (referee.length() < JDK_CLASS_TEST[i].length())
                   return true;
               if (referee.startsWith(JDK_CLASS_TEST[i]))
                   return false;
           }
           return true;
       })
   ```
   * array alphabetically sorted caused a slight regression over the brute force approach **(~55s)**
   ```java
   static{
       Arrays.sort(JDK_CLASS_TEST);
   }
   ...
       .filter((referee) -> {
           for (int i = 0; i < JDK_CLASS_TEST.length; i++) {
               if (referee.compareTo(JDK_CLASS_TEST[i]) < 0)
                   return true;
               if (referee.startsWith(JDK_CLASS_TEST[i]))
                   return false;
           }
           return true;
       })
   ```
   @vieiro is this how you meant it? (I negated your suggestion since `true` means no-match -> keep)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] vieiro commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
vieiro commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r870065682


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   @mbien Looking at the [code of StringPrefixTree](https://github.com/apache/netbeans/blob/master/enterprise/payara.tooling/src/org/netbeans/modules/payara/tooling/utils/StringPrefixTree.java) (that uses TreeMaps, and has a lookup for each letter) I don't think the gain would be much, and can even be worse.
   
   I think it's worth moving on and taking a look at those remote indexing, let me know if I can help.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien merged pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
mbien merged PR #4095:
URL: https://github.com/apache/netbeans/pull/4095


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] vieiro commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
vieiro commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869639030


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   @mbien What about a TreeSet? This quick & dirty stuff seems to be working fine (didn't test performance nor bugs, though)...
    https://gist.github.com/vieiro/b4151d7b12821998549767214dc11e9e



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869693805


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   unfortunately I don't think this is correct (but i am starting to get confused now too :)).
   example: if referee is `java/lang/Boolean` and the prefix `apple/applescript` (which is the first entry btw) `java/lang` would have to match which comes much later in the array. 
   
   However `"java/lang/Boolean".compareTo("apple/applescript")` is 9 so it would quit early without filtering the item out.
   
   Its a little bit slower too with 55s (i tested < 0 too)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] vieiro commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
vieiro commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869392403


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   While we are at it, it seems `JDK_CLASS_TEST` elements are nicely ordered alphabetically (aren't they?).
   
   Maybe we want to check if `referee.compareTo(JDK_CLASS_TEST[i]) > 0` and `return false`? We'll have more slow iterations on the loop, but we can avoid iterating over all elements. What would the flamegraphs then?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r869567234


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   I tried two optimizations already which I didn't mention above:
   * sorted array with the most common prefixes first (setup by hand). The benchmark gave identical results (53s).
   * a small switch over short prefixes (java, javax, com) as pre filter before separate loops over arrays to create a tree structure and early elimination, that was actually slower since probably more difficult to optimize for the JVM and a longer worst case path
   
   what I didn't try yet:
   * put it into a Pattern, compile it and see what happens
   * and I also didn't do a prefix length comparison fast-path, because i believe this would only be very rarely a fast-path since the array has true prefixes, the actual paths should be longer - this might also interfere with JVM vectorization attempts - but i might give it a try to have data



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] matthiasblaesing commented on a diff in pull request #4095: maven indexing: local repo indexing optimizations

Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on code in PR #4095:
URL: https://github.com/apache/netbeans/pull/4095#discussion_r878904558


##########
java/maven.indexer/src/org/netbeans/modules/maven/indexer/ClassDependencyIndexCreator.java:
##########
@@ -256,34 +256,36 @@ private static Set<String> parseField(String refereeCRC, String field, String re
         "org/omg/CosNaming", "org/omg/Dynamic", "org/omg/DynamicAny", "org/omg/IOP", "org/omg/Messaging",
         "org/omg/PortableInterceptor", "org/omg/PortableServer", "org/omg/SendingContext", "org/omg/stub",
         "org/w3c/dom", "org/xml/sax"
-    });
+    };
 
     /**
      * @param referrer a referring class, as {@code pkg/Outer$Inner}
-     * @param data its bytecode
+     * @param classData its bytecode
      * @param depsMap map from referring outer classes (as {@code pkg/Outer}) to referred-to classes (as {@code pkg/Outer$Inner})
      * @param siblings other referring classes in the same artifact (including this one), as {@code pkg/Outer$Inner}
      */
-    private static void addDependenciesToMap(String referrer, InputStream data, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+    private static void addDependenciesToMap(String referrer, InputStream classData, Map<String, Set<String>> depsMap, Set<String> siblings) throws IOException {
+
         int shell = referrer.indexOf('$', referrer.lastIndexOf('/') + 1);
         String referrerTopLevel = shell == -1 ? referrer : referrer.substring(0, shell);
-        for (String referee : dependencies(data)) {
-            if (referrer.equals(referee)) {
-                continue;
-            }
-            if (siblings.contains(referee)) {
-                continue; // in same JAR, not interesting
-            }
-            if (JDK_CLASS_TEST.test(referee)) {
-                continue;
-            }
-            Set<String> referees = depsMap.get(referrerTopLevel);
-            if (referees == null) {
-                referees = new HashSet<>();
-                depsMap.put(referrerTopLevel, referees);
-            }
-            referees.add(referee);
+
+        Set<String> tmp = depsMap.get(referrerTopLevel);
+        if (tmp == null) {
+            tmp = new HashSet<>();
+            depsMap.put(referrerTopLevel, tmp);
         }
+        Set<String> referees = tmp;
+
+        dependenciesOf(classData)
+            .filter((referee) -> !referrer.equals(referee))
+            .filter((referee) -> !siblings.contains(referee)) // in same JAR, not interesting
+            .filter((referee) -> {
+                for (int i = 0; i < JDK_CLASS_TEST.length; i++)
+                    if (referee.startsWith(JDK_CLASS_TEST[i]))
+                        return false;

Review Comment:
   It would be good to have braces in lines 283 and 284. My eye is the quicker to find the right scope and verify the right behavior.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists