Skip to content

Commit

Permalink
Attempt to reduce the number of ArchivePathTree instances for the sam…
Browse files Browse the repository at this point in the history
…e path and opening same JARs multiple times
  • Loading branch information
aloubyansky committed Aug 15, 2023
1 parent ca9b2e5 commit 5038101
Show file tree
Hide file tree
Showing 5 changed files with 244 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -37,7 +37,7 @@ public ArchivePathTree(Path archive) {

@Override
public Collection<Path> getRoots() {
return Collections.singletonList(archive);
return List.of(archive);
}

@Override
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -275,8 +283,9 @@ public void close() throws IOException {
e.addSuppressed(t);
}
throw e;
} finally {
lock.writeLock().unlock();
}
lock.writeLock().unlock();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Path, SharedArchivePathTree> 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}.
* <p>
* 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<Path> getRoots() {
return delegate.getRoots();
}

@Override
public Manifest getManifest() {
return delegate.getManifest();
}

@Override
public void walk(PathVisitor visitor) {
delegate.walk(visitor);
}

@Override
public <T> T apply(String relativePath, Function<PathVisit, T> func) {
return delegate.apply(relativePath, func);
}

@Override
public void accept(String relativePath, Consumer<PathVisit> 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();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down

0 comments on commit 5038101

Please sign in to comment.