Skip to content

Commit

Permalink
Merge pull request #42492 from gsmet/various-memory-improvements
Browse files Browse the repository at this point in the history
Memory improvements related to PathTree and Manifests
  • Loading branch information
gsmet authored Aug 14, 2024
2 parents 8c0b15b + 6780c2c commit 9872f92
Show file tree
Hide file tree
Showing 17 changed files with 363 additions and 230 deletions.
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
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;
}
}
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
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
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

0 comments on commit 9872f92

Please sign in to comment.