From f96c4d79984bda384eb3d8fa73c371c787d27291 Mon Sep 17 00:00:00 2001 From: Mickael Istria Date: Thu, 10 Oct 2024 21:49:22 +0200 Subject: [PATCH] Share/deduplicate jar contents Introduce a subclass of JavacFileManager that is capable of caching and sharing jar contents. Also ensure some references are removed from context upon usage to encourage garbage collection. Note that JDK content is still duplicated as it's using another filemanager that seems harder to configure, using direct call to `PlatformUtils.lookupPlatformDescription().getFileManager()`. Also note that the cached entries only get disposed when all filemanagers are closed/GCed. --- .../jdt/core/dom/JavacBindingResolver.java | 5 +- .../dom/JavacCompilationUnitResolver.java | 106 +++++--- .../javac/CachingJarsJavaFileManager.java | 99 ++++++++ .../jdt/internal/javac/JavacUtils.java | 3 +- .../javac/ZipFileSystemProviderWithCache.java | 233 ++++++++++++++++++ 5 files changed, 411 insertions(+), 35 deletions(-) create mode 100644 org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/CachingJarsJavaFileManager.java create mode 100644 org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/ZipFileSystemProviderWithCache.java diff --git a/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacBindingResolver.java b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacBindingResolver.java index bb90260f0da..6b61c2ee841 100644 --- a/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacBindingResolver.java +++ b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacBindingResolver.java @@ -101,7 +101,7 @@ */ public class JavacBindingResolver extends BindingResolver { - private final JavacTask javac; // TODO evaluate memory cost of storing the instance + private JavacTask javac; // TODO evaluate memory cost of storing the instance // it will probably be better to run the `Enter` and then only extract interesting // date from it. public final Context context; @@ -443,7 +443,10 @@ private void resolve() { ILog.get().error(e.getMessage(), e); } } + // some cleanups to encourage garbage collection + JavacCompilationUnitResolver.cleanup(context); } + this.javac = null; synchronized (this) { if (this.symbolToDeclaration == null) { Map wipSymbolToDeclaration = new HashMap<>(); diff --git a/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacCompilationUnitResolver.java b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacCompilationUnitResolver.java index 782222108e8..412843cd975 100644 --- a/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacCompilationUnitResolver.java +++ b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacCompilationUnitResolver.java @@ -25,12 +25,13 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.SortedSet; +import java.util.TreeSet; import java.util.function.Function; import java.util.stream.Collectors; import javax.lang.model.element.TypeElement; import javax.tools.Diagnostic; -import javax.tools.Diagnostic.Kind; import javax.tools.DiagnosticListener; import javax.tools.JavaFileManager; import javax.tools.JavaFileObject; @@ -71,6 +72,7 @@ import org.eclipse.jdt.internal.core.JavaProject; import org.eclipse.jdt.internal.core.dom.ICompilationUnitResolver; import org.eclipse.jdt.internal.core.util.BindingKeyParser; +import org.eclipse.jdt.internal.javac.CachingJarsJavaFileManager; import org.eclipse.jdt.internal.javac.JavacProblemConverter; import org.eclipse.jdt.internal.javac.JavacUtils; import org.eclipse.jdt.internal.javac.UnusedProblemFactory; @@ -104,12 +106,55 @@ * @implNote Cannot move to another package because parent class is package visible only */ public class JavacCompilationUnitResolver implements ICompilationUnitResolver { - public JavacCompilationUnitResolver() { - // 0-arg constructor + + private final class ForwardDiagnosticsAsDOMProblems implements DiagnosticListener { + public final Map filesToUnits; + private final JavacProblemConverter problemConverter; + + private ForwardDiagnosticsAsDOMProblems(Map filesToUnits, + JavacProblemConverter problemConverter) { + this.filesToUnits = filesToUnits; + this.problemConverter = problemConverter; + } + + @Override + public void report(Diagnostic diagnostic) { + findTargetDOM(filesToUnits, diagnostic).ifPresent(dom -> { + var newProblem = problemConverter.createJavacProblem(diagnostic); + if (newProblem != null) { + IProblem[] previous = dom.getProblems(); + IProblem[] newProblems = Arrays.copyOf(previous, previous.length + 1); + newProblems[newProblems.length - 1] = newProblem; + dom.setProblems(newProblems); + } + }); + } + + private static Optional findTargetDOM(Map filesToUnits, Object obj) { + if (obj == null) { + return Optional.empty(); + } + if (obj instanceof JavaFileObject o) { + return Optional.ofNullable(filesToUnits.get(o)); + } + if (obj instanceof DiagnosticSource source) { + return findTargetDOM(filesToUnits, source.getFile()); + } + if (obj instanceof Diagnostic diag) { + return findTargetDOM(filesToUnits, diag.getSource()); + } + return Optional.empty(); + } } + private interface GenericRequestor { public void acceptBinding(String bindingKey, IBinding binding); } + + public JavacCompilationUnitResolver() { + // 0-arg constructor + } + private List createSourceUnitList(String[] sourceFilePaths, String[] encodings) { // make list of source unit int length = sourceFilePaths.length; @@ -489,23 +534,12 @@ private Map result = new HashMap<>(sourceUnits.length, 1.f); Map filesToUnits = new HashMap<>(); final UnusedProblemFactory unusedProblemFactory = new UnusedProblemFactory(new DefaultProblemFactory(), compilerOptions); var problemConverter = new JavacProblemConverter(compilerOptions, context); - boolean[] hasParseError = new boolean[] { false }; - DiagnosticListener diagnosticListener = diagnostic -> { - findTargetDOM(filesToUnits, diagnostic).ifPresent(dom -> { - hasParseError[0] |= diagnostic.getKind() == Kind.ERROR; - var newProblem = problemConverter.createJavacProblem(diagnostic); - if (newProblem != null) { - IProblem[] previous = dom.getProblems(); - IProblem[] newProblems = Arrays.copyOf(previous, previous.length + 1); - newProblems[newProblems.length - 1] = newProblem; - dom.setProblems(newProblems); - } - }); - }; + DiagnosticListener diagnosticListener = new ForwardDiagnosticsAsDOMProblems(filesToUnits, problemConverter); MultiTaskListener.instance(context).add(new TaskListener() { @Override public void finished(TaskEvent e) { @@ -653,7 +687,8 @@ public Void visitClass(ClassTree node, Void p) { try { rawText = fileObjects.get(i).getCharContent(true).toString(); } catch( IOException ioe) { - // ignore + ILog.get().error(ioe.getMessage(), ioe); + return null; } CompilationUnit res = result.get(sourceUnits[i]); AST ast = res.ast; @@ -740,6 +775,9 @@ public void postVisit(ASTNode node) { ILog.get().error(thrown.getMessage(), thrown); } } + if (!resolveBindings) { + destroy(context); + } if (cachedThrown != null) { throw new RuntimeException(cachedThrown); } @@ -750,6 +788,24 @@ public void postVisit(ASTNode node) { return result; } + /// cleans up context after analysis (nothing left to process) + /// but remain it usable by bindings by keeping filemanager available. + public static void cleanup(Context context) { + MultiTaskListener.instance(context).clear(); + if (context.get(DiagnosticListener.class) instanceof ForwardDiagnosticsAsDOMProblems listener) { + listener.filesToUnits.clear(); // no need to keep handle on generated ASTs in the context + } + } + /// destroys the context, it's not usable at all after + public void destroy(Context context) { + cleanup(context); + try { + context.get(JavaFileManager.class).close(); + } catch (IOException e) { + ILog.get().error(e.getMessage(), e); + } + } + private void addProblemsToDOM(CompilationUnit dom, Collection problems) { if (problems == null) { return; @@ -764,22 +820,6 @@ private void addProblemsToDOM(CompilationUnit dom, Collection findTargetDOM(Map filesToUnits, Object obj) { - if (obj == null) { - return Optional.empty(); - } - if (obj instanceof JavaFileObject o) { - return Optional.ofNullable(filesToUnits.get(o)); - } - if (obj instanceof DiagnosticSource source) { - return findTargetDOM(filesToUnits, source.getFile()); - } - if (obj instanceof Diagnostic diag) { - return findTargetDOM(filesToUnits, diag.getSource()); - } - return Optional.empty(); - } - private AST createAST(Map options, int level, Context context, int flags) { AST ast = AST.newAST(level, JavaCore.ENABLED.equals(options.get(JavaCore.COMPILER_PB_ENABLE_PREVIEW_FEATURES))); ast.setFlag(flags); diff --git a/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/CachingJarsJavaFileManager.java b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/CachingJarsJavaFileManager.java new file mode 100644 index 00000000000..5d6ac5445c4 --- /dev/null +++ b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/CachingJarsJavaFileManager.java @@ -0,0 +1,99 @@ +/******************************************************************************* +* Copyright (c) 2024 Red Hat, Inc. and others. +* All rights reserved. This program and the accompanying materials +* are made available under the terms of the Eclipse Public License 2.0 +* which accompanies this distribution, and is available at +* https://www.eclipse.org/legal/epl-2.0/ +* +* SPDX-License-Identifier: EPL-2.0 +*******************************************************************************/ +package org.eclipse.jdt.internal.javac; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.nio.file.FileSystem; +import java.util.Collection; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import javax.tools.JavaFileManager; + +import org.eclipse.core.runtime.ILog; + +import com.sun.tools.javac.api.ClientCodeWrapper; +import com.sun.tools.javac.file.FSInfo; +import com.sun.tools.javac.file.JavacFileManager; +import com.sun.tools.javac.util.Context; +import com.sun.tools.javac.util.Context.Factory; + +/// An implementation of [JavacFileManager] suitable for local parsing/resolution. +/// It allows to reuse the filesystems for the referenced jars so they're not duplicated, +/// +/// Note that the main goal is to override the [#close()] method so it does _not_ close +/// the underlying ZipFileSystem which might still be in use. +public class CachingJarsJavaFileManager extends JavacFileManager { + + private static final ZipFileSystemProviderWithCache zipCache = new ZipFileSystemProviderWithCache(); + + /** + * Register a Context.Factory to create a JavacFileManager. + */ + public static void preRegister(Context context) { + context.put(FSInfo.class, new FSInfo() { + @Override + public synchronized java.nio.file.spi.FileSystemProvider getJarFSProvider() { + return CachingJarsJavaFileManager.zipCache; + } + }); + context.put(ClientCodeWrapper.class, new ClientCodeWrapper(context) { + @Override + protected boolean isTrusted(Object o) { + return super.isTrusted(o) || o.getClass().getClassLoader() == CachingJarsJavaFileManager.class.getClassLoader(); + } + }); + context.put(JavaFileManager.class, (Factory)c -> new CachingJarsJavaFileManager(c)); + } + + /** + * Create a JavacFileManager using a given context, optionally registering + * it as the JavaFileManager for that context. + */ + public CachingJarsJavaFileManager(Context context) { + super(context, true, null); + zipCache.register(this); + } + + @Override + public void close() throws IOException { + // closes as much as possible, except the containers as they have a + // ref to the shared ZipFileSystem that we don't want to close + // To improve: close non-ArchiveContainer containers + flush(); + locations.close(); + resetOutputFilesWritten(); + zipCache.closeFileManager(System.identityHashCode(this)); + } + + Collection listFilesystemsInArchiveContainers() { + Set res = new HashSet<>(); + try { + Field containerField = JavacFileManager.class.getDeclaredField("containers"); + containerField.setAccessible(true); + if (containerField.get(this) instanceof Map containersMap) { + for (Object o : containersMap.values()) { + if (o.getClass().getName().equals(JavacFileManager.class.getName() + "$ArchiveContainer")) { + Field filesystemField = o.getClass().getDeclaredField("fileSystem"); + filesystemField.setAccessible(true); + if (filesystemField.get(o) instanceof FileSystem fs) { + res.add(fs); + } + } + } + } + } catch (Exception ex) { + ILog.get().error(ex.getMessage(), ex); + } + return res; + } +} diff --git a/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/JavacUtils.java b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/JavacUtils.java index ddf1e6fd88f..a794e9446be 100644 --- a/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/JavacUtils.java +++ b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/JavacUtils.java @@ -28,6 +28,7 @@ import java.util.stream.Stream; import javax.tools.JavaFileManager; +import javax.tools.StandardJavaFileManager; import javax.tools.StandardLocation; import org.eclipse.core.resources.IProject; @@ -205,7 +206,7 @@ private static void addDebugInfos(Map compilerOptions, Options o private static void configurePaths(JavaProject javaProject, Context context, JavacConfig compilerConfig, File output, boolean isTest) { - JavacFileManager fileManager = (JavacFileManager)context.get(JavaFileManager.class); + var fileManager = (StandardJavaFileManager)context.get(JavaFileManager.class); try { if (compilerConfig != null && !isEmpty(compilerConfig.annotationProcessorPaths())) { fileManager.setLocation(StandardLocation.ANNOTATION_PROCESSOR_PATH, diff --git a/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/ZipFileSystemProviderWithCache.java b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/ZipFileSystemProviderWithCache.java new file mode 100644 index 00000000000..f28ce9712eb --- /dev/null +++ b/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/ZipFileSystemProviderWithCache.java @@ -0,0 +1,233 @@ +/******************************************************************************* +* Copyright (c) 2024 Red Hat, Inc. and others. +* All rights reserved. This program and the accompanying materials +* are made available under the terms of the Eclipse Public License 2.0 +* which accompanies this distribution, and is available at +* https://www.eclipse.org/legal/epl-2.0/ +* +* SPDX-License-Identifier: EPL-2.0 +*******************************************************************************/ +package org.eclipse.jdt.internal.javac; + +import java.io.IOException; +import java.lang.ref.Cleaner; +import java.net.URI; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.AccessMode; +import java.nio.file.CopyOption; +import java.nio.file.DirectoryStream; +import java.nio.file.DirectoryStream.Filter; +import java.nio.file.FileStore; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.FileAttributeView; +import java.nio.file.attribute.FileTime; +import java.nio.file.spi.FileSystemProvider; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CompletableFuture; + +import org.eclipse.core.runtime.ILog; + +import com.sun.tools.javac.file.CacheFSInfo; +import com.sun.tools.javac.file.JavacFileManager; + +/// A filesystem provider for Zip/Jar files that is capable of caching content so it +/// can be reused by multiple contexts (as long as the cached objects don't get close +/// while still in use). +/// +/// Caveats: +/// - cached objects are currenlty never released (some commented code attempts it, but +/// it requires the consumers to declare they consume a particular path/filesystem). +/// - This is currently only hooked for the main JavacFileManager. Some other filemanagers +/// are used (eg `JDKPlatformProvider.getFileManager()`) which don't take advantage of caching +public class ZipFileSystemProviderWithCache extends FileSystemProvider { + + private final Cleaner cleaner = Cleaner.create(); + private final Map cachedFilesystems = new HashMap<>(); + private final Map lastModificationOfCache = new HashMap<>(); + private final FileSystemProvider delegate = new CacheFSInfo().getJarFSProvider(); + /// a Set of int for output of `System.identityHashCode()` for given + /// active {@link JavacFileManager}s. + /// We actually store `int` instead of an actual reference to allow the underlying + /// file managers to be garbage collected (and closed). + private final Set fileManagersIdentitieis = new HashSet<>(); + + public void closeFileManager(int cachingJarsJavaFileManagerIdentityHashCode) { + // We cannot keep a reference to JavaFileManager easily or it create leak in the context + // we instead keep refs to ids + + // One important limitation is that we can only clear the cache when we know that + // no relevant filesystem is still in use (called `.close()` or got GCed). + // Ideally, we would have finer grain cache that would clear the unused filesystems + // according to the filemanagers still in use. But as we can't keep ref on FileManagers + // to not block GC, that seems impossible. + Set toClear = new HashSet<>(); + synchronized (this) { + this.fileManagersIdentitieis.remove(cachingJarsJavaFileManagerIdentityHashCode); + if (this.fileManagersIdentitieis.isEmpty()) { + toClear.addAll(lastModificationOfCache.keySet()); + toClear.addAll(cachedFilesystems.values()); + lastModificationOfCache.clear(); + cachedFilesystems.clear(); + // GC can then happen + } + } + CompletableFuture.runAsync(() -> toClear.forEach(fs -> { + try { + fs.close(); + } catch (IOException ex) { + ILog.get().error(ex.getMessage(), ex); + } + })); + } + + @Override + public String getScheme() { + return delegate.getScheme(); + } + + @Override + public FileSystem newFileSystem(URI uri, Map env) throws IOException { + return newFileSystem(getPath(uri), env); + } + @Override + public FileSystem newFileSystem(Path path, Map env) throws IOException { + synchronized (this) { + var cached = getCachedFileSystem(path); + if (cached != null) { + return cached; + } + var lastMod = Files.getLastModifiedTime(path); + var res = delegate.newFileSystem(path, env); + this.cachedFilesystems.put(path, res); + this.lastModificationOfCache.put(res, lastMod); + return res; + } + } + + @Override + public FileSystem getFileSystem(URI uri) { + var res = getCachedFileSystem(getPath(uri)); + if (res == null) { + res = delegate.getFileSystem(uri); + } + return res; + } + /// Get the cached FileSystem for given path + /// @param file the path of the archive + /// @return the cache filesystem, or `null` is filesystem + /// was not requested yet, or if the cached filesystem + /// is outdated and not suitable for usage any more. + private FileSystem getCachedFileSystem(Path file) { + var cached = this.cachedFilesystems.get(file); + if (cached == null) { + return null; + } + var cacheLastMod = this.lastModificationOfCache.get(cached); + FileTime lastMod; + try { + lastMod = Files.getLastModifiedTime(file); + } catch (IOException e) { + return null; + } + if (lastMod.compareTo(cacheLastMod) > 0) { // file changed, cache is not valid + // create and use a new container/filesystem + return null; + } + return cached; + } + + @Override + public Path getPath(URI uri) { + return delegate.getPath(uri); + } + + @Override + public SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) + throws IOException { + return delegate.newByteChannel(path, options, attrs); + } + + @Override + public DirectoryStream newDirectoryStream(Path dir, Filter filter) throws IOException { + return delegate.newDirectoryStream(dir, filter); + } + + @Override + public void createDirectory(Path dir, FileAttribute... attrs) throws IOException { + delegate.createDirectory(dir, attrs); + } + + @Override + public void delete(Path path) throws IOException { + delegate.delete(path); + } + + @Override + public void copy(Path source, Path target, CopyOption... options) throws IOException { + delegate.copy(source, target, options); + } + + @Override + public void move(Path source, Path target, CopyOption... options) throws IOException { + delegate.move(source, target, options); + } + + @Override + public boolean isSameFile(Path path, Path path2) throws IOException { + return delegate.isSameFile(path, path2); + } + + @Override + public boolean isHidden(Path path) throws IOException { + return delegate.isHidden(path); + } + + @Override + public FileStore getFileStore(Path path) throws IOException { + return delegate.getFileStore(path); + } + + @Override + public void checkAccess(Path path, AccessMode... modes) throws IOException { + delegate.checkAccess(path, modes); + } + + @Override + public V getFileAttributeView(Path path, Class type, LinkOption... options) { + return delegate.getFileAttributeView(path, type, options); + } + + @Override + public A readAttributes(Path path, Class type, LinkOption... options) + throws IOException { + return delegate.readAttributes(path, type, options); + } + + @Override + public Map readAttributes(Path path, String attributes, LinkOption... options) throws IOException { + return delegate.readAttributes(path, attributes, options); + } + + @Override + public void setAttribute(Path path, String attribute, Object value, LinkOption... options) throws IOException { + delegate.setAttribute(path, attribute, value, options); + } + + public void register(CachingJarsJavaFileManager cachingJarsJavaFileManager) { + int id = System.identityHashCode(cachingJarsJavaFileManager); + cleaner.register(cachingJarsJavaFileManager, () -> closeFileManager(id)); + synchronized (this) { + this.fileManagersIdentitieis.add(id); + } + } + +}