From 503810167ef093e5cce9a1ea7beab288af51af81 Mon Sep 17 00:00:00 2001 From: Alexey Loubyansky Date: Thu, 10 Aug 2023 15:29:59 +0200 Subject: [PATCH] Attempt to reduce the number of ArchivePathTree instances for the same path and opening same JARs multiple times --- .../quarkus/deployment/BootstrapConfig.java | 10 + .../io/quarkus/paths/ArchivePathTree.java | 23 +- .../main/java/io/quarkus/paths/PathTree.java | 4 +- .../quarkus/paths/SharedArchivePathTree.java | 215 ++++++++++++++++++ .../maven/it/RunAndCheckMojoTestBase.java | 2 +- 5 files changed, 244 insertions(+), 10 deletions(-) create mode 100644 independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/SharedArchivePathTree.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/BootstrapConfig.java b/core/deployment/src/main/java/io/quarkus/deployment/BootstrapConfig.java index 94207f085d8825..12448f389fcd04 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/BootstrapConfig.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/BootstrapConfig.java @@ -24,6 +24,16 @@ public class BootstrapConfig { @ConfigItem(defaultValue = "false") Boolean workspaceDiscovery; + /** + * By default, the bootstrap mechanism will create a shared cache of open JARs for + * Quarkus classloaders to reduce the total number of opened ZIP FileSystems in dev and test modes. + * Setting system property {@code quarkus.bootstrap.disable-jar-cache} to {@code true} will make + * Quarkus classloaders create a new ZIP FileSystem for each JAR classpath element every time it is added + * to a Quarkus classloader. + */ + @ConfigItem(defaultValue = "false") + boolean disableJarCache; + /** * Whether to throw an error, warn or silently ignore misaligned platform BOM imports */ diff --git a/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/ArchivePathTree.java b/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/ArchivePathTree.java index 6a34410efe4a9d..53ae8bca677482 100644 --- a/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/ArchivePathTree.java +++ b/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/ArchivePathTree.java @@ -6,7 +6,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Collection; -import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -18,7 +18,7 @@ public class ArchivePathTree extends PathTreeWithManifest implements PathTree { - private final Path archive; + protected final Path archive; private final PathFilter pathFilter; public ArchivePathTree(Path archive) { @@ -37,7 +37,7 @@ public ArchivePathTree(Path archive) { @Override public Collection getRoots() { - return Collections.singletonList(archive); + return List.of(archive); } @Override @@ -124,7 +124,7 @@ public boolean contains(String relativePath) { return false; } - private FileSystem openFs() throws IOException { + protected FileSystem openFs() throws IOException { return ZipUtils.newFileSystem(archive); } @@ -155,16 +155,24 @@ public boolean equals(Object obj) { && manifestEnabled == other.manifestEnabled; } - private class OpenArchivePathTree extends DirectoryPathTree { + protected class OpenArchivePathTree extends DirectoryPathTree { private final FileSystem fs; private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); - private OpenArchivePathTree(FileSystem fs) { + protected OpenArchivePathTree(FileSystem fs) { super(fs.getPath("/"), pathFilter, ArchivePathTree.this); this.fs = fs; } + protected ReentrantReadWriteLock.ReadLock readLock() { + return lock.readLock(); + } + + protected ReentrantReadWriteLock.WriteLock writeLock() { + return lock.writeLock(); + } + @Override protected void initManifest(Manifest m) { super.initManifest(m); @@ -275,8 +283,9 @@ public void close() throws IOException { e.addSuppressed(t); } throw e; + } finally { + lock.writeLock().unlock(); } - lock.writeLock().unlock(); } } diff --git a/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/PathTree.java b/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/PathTree.java index 5fb638773c2fb5..cb303e6e284fe0 100644 --- a/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/PathTree.java +++ b/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/PathTree.java @@ -47,7 +47,7 @@ static PathTree ofDirectoryOrArchive(Path p) { static PathTree ofDirectoryOrArchive(Path p, PathFilter filter) { try { final BasicFileAttributes fileAttributes = Files.readAttributes(p, BasicFileAttributes.class); - return fileAttributes.isDirectory() ? new DirectoryPathTree(p, filter) : new ArchivePathTree(p, filter); + return fileAttributes.isDirectory() ? new DirectoryPathTree(p, filter) : SharedArchivePathTree.forPath(p, filter); } catch (IOException e) { throw new IllegalArgumentException(p + " does not exist", e); } @@ -76,7 +76,7 @@ static PathTree ofArchive(Path archive, PathFilter filter) { if (!Files.exists(archive)) { throw new IllegalArgumentException(archive + " does not exist"); } - return new ArchivePathTree(archive, filter); + return SharedArchivePathTree.forPath(archive, filter); } /** 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 new file mode 100644 index 00000000000000..63c5a39b52995c --- /dev/null +++ b/independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/SharedArchivePathTree.java @@ -0,0 +1,215 @@ +package io.quarkus.paths; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.FileSystem; +import java.nio.file.Path; +import java.util.Collection; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.jar.Manifest; + +/** + * While {@link ArchivePathTree} implementation is thread-safe, this implementation + * provides a way for multiple concurrent users to share the same instance of an open archive, + * instead of every user opening its own copy of it. + */ +class SharedArchivePathTree extends ArchivePathTree { + + private static final String DISABLE_JAR_CACHE_PROPERTY = "quarkus.bootstrap.disable-jar-cache"; + + private static final boolean ENABLE_SHARING; + + // It would probably be better to have a weak hash map based cache, + // however as the first iteration it'll already be more efficient than before + private static final Map cache; + + static { + ENABLE_SHARING = !Boolean.getBoolean(DISABLE_JAR_CACHE_PROPERTY); + cache = ENABLE_SHARING ? new ConcurrentHashMap<>() : Map.of(); + } + + /** + * Returns an instance of {@link ArchivePathTree} for the {@code path} either from a cache + * or creates a new instance for it and puts it in the cache. + * + * @param path path to an archive + * @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); + } + + /** + * Caching of archive path trees with {@link PathFilter}'s isn't currently supported. + * If {@code filter} argument is not null, the method will return a non-cacheable implementation + * of {@link ArchivePathTree}. + *

+ * Otherwise, the method returns an instance of {@link ArchivePathTree} for the {@code path} either from a cache + * or creates a new instance for it and puts it in the cache. + * + * @param path path to an archive + * @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); + } + + /** + * Removes an entry for {@code path} from the path tree cache. If the cache + * does not contain an entry for the {@code path}, the method will return silently. + * + * @param path path to remove from the cache + */ + static void removeFromCache(Path path) { + cache.remove(path); + } + + private final AtomicInteger openCount = new AtomicInteger(); + private volatile SharedOpenArchivePathTree lastOpen; + + SharedArchivePathTree(Path archive) { + super(archive); + } + + @Override + public OpenPathTree open() { + var lastOpen = this.lastOpen; + if (lastOpen != null && lastOpen.acquire()) { + return new CallerOpenPathTree(lastOpen); + } + try { + this.lastOpen = new SharedOpenArchivePathTree(openFs()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + return new CallerOpenPathTree(this.lastOpen); + } + + private class SharedOpenArchivePathTree extends OpenArchivePathTree { + + private final AtomicInteger users = new AtomicInteger(1); + + protected SharedOpenArchivePathTree(FileSystem fs) { + super(fs); + openCount.incrementAndGet(); + } + + private boolean acquire() { + readLock().lock(); + final boolean result = lastOpen == this && isOpen(); + if (result) { + users.incrementAndGet(); + } + readLock().unlock(); + return result; + } + + @Override + public void close() throws IOException { + writeLock().lock(); + final boolean close = users.decrementAndGet() == 0; + try { + if (close) { + if (lastOpen == this) { + lastOpen = null; + } + if (openCount.decrementAndGet() == 0) { + removeFromCache(archive); + } + super.close(); + } + } finally { + writeLock().unlock(); + } + } + } + + /** + * This is a caller "view" of an underlying {@link OpenPathTree} instance that + * delegates only the first {@link #close()} call by the caller to the underlying {@link OpenPathTree} instance + * with subsequent {@link #close()} calls ignored. + */ + private static class CallerOpenPathTree implements OpenPathTree { + + private final SharedOpenArchivePathTree delegate; + private volatile boolean closed; + + private CallerOpenPathTree(SharedOpenArchivePathTree delegate) { + this.delegate = delegate; + } + + @Override + public PathTree getOriginalTree() { + return delegate.getOriginalTree(); + } + + @Override + public boolean isOpen() { + return !closed && delegate.isOpen(); + } + + @Override + public Path getPath(String relativePath) { + return delegate.getPath(relativePath); + } + + @Override + public Collection getRoots() { + return delegate.getRoots(); + } + + @Override + public Manifest getManifest() { + return delegate.getManifest(); + } + + @Override + public void walk(PathVisitor visitor) { + delegate.walk(visitor); + } + + @Override + public T apply(String relativePath, Function func) { + return delegate.apply(relativePath, func); + } + + @Override + public void accept(String relativePath, Consumer consumer) { + delegate.accept(relativePath, consumer); + } + + @Override + public boolean contains(String relativePath) { + return delegate.contains(relativePath); + } + + @Override + public OpenPathTree open() { + return delegate.open(); + } + + @Override + public void close() throws IOException { + if (closed) { + return; + } + delegate.writeLock().lock(); + try { + if (!closed) { + closed = true; + delegate.close(); + } + } finally { + delegate.writeLock().unlock(); + } + } + } +} diff --git a/test-framework/maven/src/main/java/io/quarkus/maven/it/RunAndCheckMojoTestBase.java b/test-framework/maven/src/main/java/io/quarkus/maven/it/RunAndCheckMojoTestBase.java index 80bf2c3118d5ad..5ea0c98be1315f 100644 --- a/test-framework/maven/src/main/java/io/quarkus/maven/it/RunAndCheckMojoTestBase.java +++ b/test-framework/maven/src/main/java/io/quarkus/maven/it/RunAndCheckMojoTestBase.java @@ -101,7 +101,7 @@ protected void run(boolean performCompile, LaunchMode mode, boolean skipAnalytic //running at once, if they add default to 75% of total mem we can easily run out //of physical memory as they will consume way more than what they need instead of //just running GC - args.add("-Djvm.args=-Xmx192m"); + args.add("-Djvm.args=-Xmx128m"); running.execute(args, Map.of()); }