Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Memory improvements related to PathTree and Manifests #42492

Merged
merged 2 commits into from
Aug 14, 2024
Merged

Memory improvements related to PathTree and Manifests #42492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1b3fa9
Do not keep the whole Manifest in memory
gsmet Aug 12, 2024
6780c2c
Make sure we don't keep a reference to the ZipPath in ArchivePathTree
gsmet Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 30 additions & 24 deletions independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/ArchivePathTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,15 +224,30 @@ public boolean equals(Object obj) {
&& manifestEnabled == other.manifestEnabled;
}

protected class OpenArchivePathTree extends DirectoryPathTree {
protected class OpenArchivePathTree extends OpenContainerPathTree {

// we don't make the field final as we want to nullify it on close
private volatile FileSystem fs;
private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();

// we don't make these fields final as we want to nullify them on close
private FileSystem fs;
private Path rootPath;

private volatile boolean open = true;

protected OpenArchivePathTree(FileSystem fs) {
super(fs.getPath("/"), pathFilter, ArchivePathTree.this);
super(ArchivePathTree.this.pathFilter, ArchivePathTree.this);
this.fs = fs;
this.rootPath = fs.getPath("/");
}

@Override
protected Path getContainerPath() {
return ArchivePathTree.this.archive;
}

@Override
protected Path getRootPath() {
return rootPath;
}

protected ReentrantReadWriteLock.ReadLock readLock() {
Expand Down Expand Up @@ -272,7 +287,7 @@ protected void initMultiReleaseMapping(Map<String, String> mrMapping) {
public boolean isOpen() {
lock.readLock().lock();
try {
return fs != null && fs.isOpen();
return open;
} finally {
lock.readLock().unlock();
}
Expand Down Expand Up @@ -349,7 +364,7 @@ public Path getPath(String relativePath) {
*/
private void ensureOpen() {
// let's not use isOpen() as ensureOpen() is always used inside a read lock
if (fs != null && fs.isOpen()) {
if (open) {
return;
}
throw new RuntimeException("Failed to access " + ArchivePathTree.this.getRoots()
Expand All @@ -358,28 +373,19 @@ private void ensureOpen() {

@Override
public void close() throws IOException {
Throwable t = null;
lock.writeLock().lock();
try {
super.close();
} catch (Throwable e) {
t = e;
open = false;
rootPath = null;
fs.close();
} catch (IOException e) {
throw e;
} finally {
try {
fs.close();
} catch (IOException e) {
if (t != null) {
e.addSuppressed(t);
}
throw e;
} finally {
// even when we close the fs, everything is kept as is in the fs instance
// and typically the cen, which is quite large
// let's make sure the fs is nullified for it to be garbage collected
fs = null;
lock.writeLock().unlock();
}
// even when we close the fs, everything is kept as is in the fs instance
// and typically the cen, which is quite large
// let's make sure the fs is nullified for it to be garbage collected
fs = null;
lock.writeLock().unlock();
}
}

Expand Down
155 changes: 12 additions & 143 deletions ...endent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/DirectoryPathTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,55 +2,14 @@

import java.io.IOException;
import java.io.Serializable;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.regex.Pattern;

public class DirectoryPathTree extends PathTreeWithManifest implements OpenPathTree, Serializable {
public class DirectoryPathTree extends OpenContainerPathTree implements Serializable {

private static final long serialVersionUID = 2255956884896445059L;

private static final boolean USE_WINDOWS_ABSOLUTE_PATH_PATTERN = !FileSystems.getDefault().getSeparator().equals("/");

private static volatile Pattern windowsAbsolutePathPattern;

private static Pattern windowsAbsolutePathPattern() {
return windowsAbsolutePathPattern == null ? windowsAbsolutePathPattern = Pattern.compile("[a-zA-Z]:\\\\.*")
: windowsAbsolutePathPattern;
}

static boolean isAbsolutePath(String path) {
return path != null && !path.isEmpty()
&& (path.charAt(0) == '/' // we want to check for '/' on every OS
|| USE_WINDOWS_ABSOLUTE_PATH_PATTERN
&& (windowsAbsolutePathPattern().matcher(path).matches())
|| path.startsWith(FileSystems.getDefault().getSeparator()));
}

static void ensureResourcePath(FileSystem fs, String path) {
if (isAbsolutePath(path)) {
throw new IllegalArgumentException("Expected a path relative to the root of the path tree but got " + path);
}
// this is to disallow reading outside the path tree root
if (path != null && path.contains("..")) {
for (Path pathElement : fs.getPath(path)) {
if (pathElement.toString().equals("..")) {
throw new IllegalArgumentException("'..' cannot be used in resource paths, but got " + path);
}
}
}
}

private Path dir;
private PathFilter pathFilter;

/**
* For deserialization
Expand All @@ -68,90 +27,37 @@ public DirectoryPathTree(Path dir, PathFilter pathFilter) {
}

public DirectoryPathTree(Path dir, PathFilter pathFilter, boolean manifestEnabled) {
super(manifestEnabled);
super(pathFilter, manifestEnabled);
this.dir = dir;
this.pathFilter = pathFilter;
}

protected DirectoryPathTree(Path dir, PathFilter pathFilter, PathTreeWithManifest pathTreeWithManifest) {
super(pathTreeWithManifest);
super(pathFilter, pathTreeWithManifest);
this.dir = dir;
this.pathFilter = pathFilter;
}

@Override
public Collection<Path> getRoots() {
return List.of(dir);
protected Path getRootPath() {
return dir;
}

@Override
public void walk(PathVisitor visitor) {
PathTreeVisit.walk(dir, dir, dir, pathFilter, getMultiReleaseMapping(), visitor);
protected Path getContainerPath() {
return dir;
}

@Override
public void walkIfContains(String relativePath, PathVisitor visitor) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return;
}
final Path walkDir = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
if (!Files.exists(walkDir)) {
return;
}
PathTreeVisit.walk(dir, dir, walkDir, pathFilter, getMultiReleaseMapping(), visitor);
}

private void ensureResourcePath(String path) {
ensureResourcePath(dir.getFileSystem(), path);
}

@Override
protected <T> T apply(String relativePath, Function<PathVisit, T> func, boolean manifestEnabled) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return func.apply(null);
}
final Path path = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
if (!Files.exists(path)) {
return func.apply(null);
}
return PathTreeVisit.process(dir, dir, path, pathFilter, func);
}

@Override
public void accept(String relativePath, Consumer<PathVisit> consumer) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
consumer.accept(null);
return;
}
final Path path = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
if (!Files.exists(path)) {
consumer.accept(null);
return;
}
PathTreeVisit.consume(dir, dir, path, pathFilter, consumer);
public boolean isOpen() {
return true;
}

@Override
public boolean contains(String relativePath) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return false;
}
final Path path = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
return Files.exists(path);
public void close() throws IOException {
}

@Override
public Path getPath(String relativePath) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return null;
}
final Path path = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
return Files.exists(path) ? path : null;
public PathTree getOriginalTree() {
return this;
}

private void writeObject(java.io.ObjectOutputStream out) throws IOException {
Expand All @@ -165,41 +71,4 @@ private void readObject(java.io.ObjectInputStream in) throws IOException, ClassN
pathFilter = (PathFilter) in.readObject();
manifestEnabled = in.readBoolean();
}

@Override
public OpenPathTree open() {
return this;
}

@Override
public boolean isOpen() {
return true;
}

@Override
public void close() throws IOException {
}

@Override
public PathTree getOriginalTree() {
return this;
}

@Override
public int hashCode() {
return Objects.hash(dir, pathFilter, manifestEnabled);
}

@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
DirectoryPathTree other = (DirectoryPathTree) obj;
return Objects.equals(dir, other.dir) && Objects.equals(pathFilter, other.pathFilter)
&& manifestEnabled == other.manifestEnabled;
}
}
3 changes: 1 addition & 2 deletions independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/EmptyPathTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import java.util.Collections;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.jar.Manifest;

public class EmptyPathTree implements OpenPathTree {

Expand All @@ -22,7 +21,7 @@ public Collection<Path> getRoots() {
}

@Override
public Manifest getManifest() {
public ManifestAttributes getManifestAttributes() {
return null;
}

Expand Down
3 changes: 1 addition & 2 deletions independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/FilePathTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import java.util.Objects;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.jar.Manifest;

class FilePathTree implements OpenPathTree {

Expand All @@ -29,7 +28,7 @@ public Collection<Path> getRoots() {
}

@Override
public Manifest getManifest() {
public ManifestAttributes getManifestAttributes() {
return null;
}

Expand Down
5 changes: 2 additions & 3 deletions ...pendent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/FilteredPathTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import java.util.Objects;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.jar.Manifest;

public class FilteredPathTree implements PathTree {

Expand All @@ -24,8 +23,8 @@ public Collection<Path> getRoots() {
}

@Override
public Manifest getManifest() {
return original.getManifest();
public ManifestAttributes getManifestAttributes() {
return original.getManifestAttributes();
}

@Override
Expand Down
Loading
Loading