Reduce the number of ArchivePathTree instances for the same path and opening same JARs multiple times

[aloubyansky]

Related to #35280

This change reduces the number of ArchivePathTree instances (pretty much to 1 per JAR) and enables sharing of ZIP FS instances between Quarkus classloaders.

The new behavior is enabled by default but can be disabled by adding -Dbootstrap.quarkus.disable-jar-cache to the command line launching dev mode, for example.

With this change DevMojoIT#testThatTheApplicationIsReloadedMultiModule passes on my laptop with -Xmx95m instead of -Xmx115 running with -Dbootstrap.quarkus.disable-jar-cache.

[gsmet]

I think I would refrain to backport to 3.2 for now but I will backport it to 3.3.

[gsmet]

I suggested some minor improvements, let me know what you think.

[gsmet]

Wouldn't it make more sense to test if it is enabled here and fall back to standard ArchivePathTree if not?

[aloubyansky]

Perhaps SharedArchivePathTree could be enhanced with a static boolean isEnabled() and then yes.

[gsmet]

I think applying the following patch would avoid the lambdas:

diff --git a/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/SharedArchivePathTree.java b/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/SharedArchivePathTree.java
index 63c5a39b529..372dd1401d0 100644
--- a/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/SharedArchivePathTree.java
+++ b/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/SharedArchivePathTree.java
@@ -40,9 +40,11 @@ class SharedArchivePathTree extends ArchivePathTree {
      * @return instance of {@link ArchivePathTree}, never null
      */
     static ArchivePathTree forPath(Path path) {
-        return ENABLE_SHARING
-                ? cache.computeIfAbsent(path, k -> new SharedArchivePathTree(path))
-                : new ArchivePathTree(path);
+        if (!ENABLE_SHARING) {
+            return new ArchivePathTree(path);
+        }
+
+        return cache.computeIfAbsent(path, SharedArchivePathTree::new);
     }
 
     /**
@@ -57,9 +59,11 @@ static ArchivePathTree forPath(Path path) {
      * @return instance of {@link ArchivePathTree}, never null
      */
     static ArchivePathTree forPath(Path path, PathFilter filter) {
-        return filter == null && ENABLE_SHARING
-                ? cache.computeIfAbsent(path, k -> new SharedArchivePathTree(path))
-                : new ArchivePathTree(path, filter);
+        if (filter != null) {
+            return new ArchivePathTree(path, filter);
+        }
+
+        return forPath(path);
     }
 
     /**
@@ -69,6 +73,10 @@ static ArchivePathTree forPath(Path path, PathFilter filter) {
      * @param path path to remove from the cache
      */
     static void removeFromCache(Path path) {
+        if (!ENABLE_SHARING) {
+            return;
+        }
+
         cache.remove(path);
     }
 

I can push to your branch if it makes sense to you.

[aloubyansky]

Looks good, thanks

[aloubyansky]

I'll apply the changes now, thanks.

[gsmet]

Nice team work :).

I think we should refrain from backporting this tbh. And make sure we have it included in a CR before pushing it in a release.

Not having it is just some extra memory consumed and we have lived with it since forever.

[aloubyansky]

Agreed

[quarkus-bot]

Failing Jobs - Building 240e0d9

Status	Name	Step	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11	Build	⚠️ Check →	Logs	Raw logs
✖	JVM Tests - JDK 17	Build	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 20

Failures

⚙️ JVM Tests - JDK 17 #

- Failing: integration-tests/hibernate-search-orm-elasticsearch-coordination-outbox-polling 

📦 integration-tests/hibernate-search-orm-elasticsearch-coordination-outbox-polling

✖ Failed to execute goal org.codehaus.mojo:exec-maven-plugin:3.0.0:exec (docker-prune) on project quarkus-integration-test-hibernate-search-orm-elasticsearch-coordination-outbox-polling: Command execution failed.