Skip to content

Commit

Permalink
Performance: Add public API for Batch Reads in UI - closes #1614
Browse files Browse the repository at this point in the history
During Batches:
* cache Zip Files
* enable JavaModelManager.temporaryCache

Also:
* uses Batch Reads during some core actions that benefit from it.
* adds trace logging when caching is missing.

#1614
  • Loading branch information
EcljpseB0T authored and jukzi committed Dec 7, 2023
1 parent 071f4fe commit 9fb036c
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 4 deletions.
3 changes: 3 additions & 0 deletions org.eclipse.jdt.core/.options
Expand Up @@ -83,6 +83,9 @@ org.eclipse.jdt.core/debug/selection=false
# Reports access to zip and jar files through the Java model
org.eclipse.jdt.core/debug/zipaccess=false

# Reports repetitive opening of zip and jar files through the Java model without caching
org.eclipse.jdt.core/debug/zipaccessWarning=false

# Reports the time to perform code completion.
org.eclipse.jdt.core/perf/completion=300

Expand Down
Expand Up @@ -47,6 +47,7 @@
import org.eclipse.jdt.internal.core.BinaryType;
import org.eclipse.jdt.internal.core.ClassFileWorkingCopy;
import org.eclipse.jdt.internal.core.DefaultWorkingCopyOwner;
import org.eclipse.jdt.internal.core.JavaModelManager;
import org.eclipse.jdt.internal.core.PackageFragment;
import org.eclipse.jdt.internal.core.dom.util.DOMASTUtil;
import org.eclipse.jdt.internal.core.util.CodeSnippetParsingUtil;
Expand Down Expand Up @@ -1116,6 +1117,9 @@ public IBinding[] createBindings(IJavaElement[] elements, IProgressMonitor monit
}

private ASTNode internalCreateAST(IProgressMonitor monitor) {
return JavaModelManager.cacheZipFiles(() -> internalCreateASTCached(monitor));
}
private ASTNode internalCreateASTCached(IProgressMonitor monitor) {
boolean needToResolveBindings = (this.bits & CompilationUnitResolver.RESOLVE_BINDING) != 0;
switch(this.astKind) {
case K_CLASS_BODY_DECLARATIONS :
Expand Down
63 changes: 63 additions & 0 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/core/JavaCore.java
Expand Up @@ -5963,6 +5963,7 @@ public static void run(IWorkspaceRunnable action, IProgressMonitor monitor) thro
* @since 3.0
*/
public static void run(IWorkspaceRunnable action, ISchedulingRule rule, IProgressMonitor monitor) throws CoreException {
JavaModelManager.assertModelModifiable();
IWorkspace workspace = ResourcesPlugin.getWorkspace();
if (workspace.isTreeLocked()) {
new BatchOperation(action).run(monitor);
Expand All @@ -5971,6 +5972,68 @@ public static void run(IWorkspaceRunnable action, ISchedulingRule rule, IProgres
workspace.run(new BatchOperation(action), rule, IWorkspace.AVOID_UPDATE, monitor);
}
}
/**
* @since 3.37
*/
@FunctionalInterface
public static interface JavaCallable<V, E extends Exception> {
/**
* Computes a value or throws an exception.
*
* @return the result
* @throws E the Exception of given type
*/
V call() throws E;
}
/**
* @since 3.37
*/
@FunctionalInterface
public static interface JavaRunnable<E extends Exception> {
/**
* Runs or throws an exception.
*
* @throws E the Exception of given type
*/
void run() throws E;
}


/**
* Calls the argument and returns its result or its Exception. The argument's {@code call()} is supposed to query
* Java model and must not modify it. This method will try to run Java Model queries in optimized way (Using caches
* during the operation). It is safe to nest multiple calls - but not necessary.
*
*
* @param callable
* A JavaCallable that can throw an Exception
* @return the result
* @exception E
* An {@link Exception} that is thrown by the {@code callable}.
* @since 3.37
*/
public static <T, E extends Exception> T callReadOnly(JavaCallable<T, E> callable) throws E {
return JavaModelManager.callReadOnly(callable);
}

/**
* Runs the argument and will forward its Exception. The argument's {@code run()} is supposed to query Java model
* and must not modify it. This method will try to run Java Model queries in optimized way (caching things during
* the operation). It is safe to nest multiple calls - but not necessary.
*
* @param runnable
* A JavaRunnable that can throw an Exception
* @exception E
* An {@link Exception} that is thrown by the {@code runnable}.
* @since 3.37
*/
public static <T, E extends Exception> void runReadOnly(JavaRunnable<E> runnable) throws E {
callReadOnly(() -> {
runnable.run();
return null;
});
}

/**
* Bind a container reference path to some actual containers (<code>IClasspathContainer</code>).
* This API must be invoked whenever changes in container need to be reflected onto the JavaModel.
Expand Down
Expand Up @@ -2034,6 +2034,7 @@ public void resetProjectCaches() {
* Registers the given delta with this delta processor.
*/
public void registerJavaModelDelta(IJavaElementDelta delta) {
JavaModelManager.assertModelModifiable();
this.javaModelDeltas.add(delta);
}
/*
Expand Down
Expand Up @@ -39,8 +39,11 @@
import java.io.StringReader;
import java.net.URI;
import java.text.MessageFormat;
import java.time.Instant;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Deque;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -126,6 +129,7 @@
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.WorkingCopyOwner;
import org.eclipse.jdt.core.JavaCore.JavaCallable;
import org.eclipse.jdt.core.compiler.CharOperation;
import org.eclipse.jdt.core.compiler.CompilationParticipant;
import org.eclipse.jdt.core.compiler.IProblem;
Expand Down Expand Up @@ -377,6 +381,7 @@ public void setCache(IPath path, ZipFile zipFile) {
private static final String CP_RESOLVE_ADVANCED_DEBUG = JavaCore.PLUGIN_ID + "/debug/cpresolution/advanced" ; //$NON-NLS-1$
private static final String CP_RESOLVE_FAILURE_DEBUG = JavaCore.PLUGIN_ID + "/debug/cpresolution/failure" ; //$NON-NLS-1$
private static final String ZIP_ACCESS_DEBUG = JavaCore.PLUGIN_ID + "/debug/zipaccess" ; //$NON-NLS-1$
private static final String ZIP_ACCESS_WARNING_DEBUG_ = JavaCore.PLUGIN_ID + "/debug/zipaccessWarning" ; //$NON-NLS-1$
private static final String DELTA_DEBUG =JavaCore.PLUGIN_ID + "/debug/javadelta" ; //$NON-NLS-1$
private static final String DELTA_DEBUG_VERBOSE =JavaCore.PLUGIN_ID + "/debug/javadelta/verbose" ; //$NON-NLS-1$
private static final String DOM_AST_DEBUG = JavaCore.PLUGIN_ID + "/debug/dom/ast" ; //$NON-NLS-1$
Expand Down Expand Up @@ -1682,6 +1687,7 @@ public String toString() {
public static boolean CP_RESOLVE_VERBOSE = false;
public static boolean CP_RESOLVE_VERBOSE_ADVANCED = false;
public static boolean CP_RESOLVE_VERBOSE_FAILURE = false;
public static boolean ZIP_ACCESS_WARNING= false;
public static boolean ZIP_ACCESS_VERBOSE = false;
public static boolean JRT_ACCESS_VERBOSE = false;

Expand Down Expand Up @@ -1986,6 +1992,7 @@ public void optionsChanged(DebugOptions options) {
BasicSearchEngine.VERBOSE = debug && options.getBooleanOption(SEARCH_DEBUG, false);
SelectionEngine.DEBUG = debug && options.getBooleanOption(SELECTION_DEBUG, false);
JavaModelManager.ZIP_ACCESS_VERBOSE = debug && options.getBooleanOption(ZIP_ACCESS_DEBUG, false);
JavaModelManager.ZIP_ACCESS_WARNING = debug && options.getBooleanOption(ZIP_ACCESS_WARNING_DEBUG_, false);
SourceMapper.VERBOSE = debug && options.getBooleanOption(SOURCE_MAPPER_DEBUG_VERBOSE, false);
DefaultCodeFormatter.DEBUG = debug && options.getBooleanOption(FORMATTER_DEBUG, false);

Expand Down Expand Up @@ -2976,19 +2983,54 @@ public ZipFile getZipFile(IPath path) throws CoreException {
*/
public static boolean throwIoExceptionsInGetZipFile = false;

/** for tracing only **/
private final ThreadLocal<Map<IPath, Deque<Instant>>> lastAccessByPath = ThreadLocal.withInitial(HashMap::new);
/** for tracing only **/
private final ThreadLocal<Instant> lastWarning = new ThreadLocal<>();

private void traceZipAccessWarning(IPath path) {
Instant now = Instant.now();
Deque<Instant> lastAcesses = this.lastAccessByPath.get().compute(path,
(p, l) -> (l == null) ? new ArrayDeque<>() : l);
Instant recentAccess = lastAcesses.peekFirst();
Instant lastAccess = lastAcesses.peekLast();
lastAcesses.offerLast(now);
if (lastAccess != null) {
long elapsedMs = lastAccess.until(now, java.time.temporal.ChronoUnit.MILLIS);
if (elapsedMs <= 100000) {
trace(path + " opened again in this thread after " + elapsedMs + "ms"); //$NON-NLS-1$ //$NON-NLS-2$
}
}
// only warn if there have recently multiple accesses, because it's common but not that bad to have 2 of them
if (recentAccess != null && lastAcesses.size() > 2) {
long elapsedMs = recentAccess.until(now, java.time.temporal.ChronoUnit.MILLIS);
lastAcesses.pollFirst();
Instant lastInstant = this.lastWarning.get();
long elapsedWarningMs = lastInstant == null ? Long.MAX_VALUE
: lastInstant.until(now, java.time.temporal.ChronoUnit.MILLIS);
if (elapsedMs < 100 && elapsedWarningMs > 1000) {
this.lastWarning.set(now);
new Exception("Zipfile was opened multiple times wihtin " + elapsedMs + "ms in same thread " //$NON-NLS-1$ //$NON-NLS-2$
+ Thread.currentThread() + ", consider caching: " + path) //$NON-NLS-1$
.printStackTrace();
}
}
}

public ZipFile getZipFile(IPath path, boolean checkInvalidArchiveCache) throws CoreException {
if (checkInvalidArchiveCache) {
isArchiveStateKnownToBeValid(path);
}
ZipCache zipCache;
ZipFile zipFile;
if ((zipCache = this.zipFiles.get()) != null
&& (zipFile = zipCache.getCache(path)) != null) {
if ((zipCache = this.zipFiles.get()) != null && (zipFile = zipCache.getCache(path)) != null) {
return zipFile;
}
File localFile = getLocalFile(path);

try {
if (ZIP_ACCESS_WARNING) {
traceZipAccessWarning(path);
}
if (ZIP_ACCESS_VERBOSE) {
trace("(" + Thread.currentThread() + ") [JavaModelManager.getZipFile(IPath)] Creating ZipFile on " + localFile ); //$NON-NLS-1$ //$NON-NLS-2$
}
Expand Down Expand Up @@ -5727,4 +5769,49 @@ public ClasspathAccessRule getAccessRuleForProblemId(char [] filePattern, int pr
private ClasspathAccessRule getFromCache(ClasspathAccessRule rule) {
return DeduplicationUtil.internObject(rule);
}

private static ThreadLocal<Boolean> readOnly = ThreadLocal.withInitial(() -> Boolean.FALSE);

public static void assertModelModifiable() {
if (readOnly.get().booleanValue()) {
throw new IllegalStateException("Its not allow to modify JavaModel during ReadOnly action."); //$NON-NLS-1$
}
}

public static <T, E extends Exception> T callReadOnly(JavaCallable<T, E> callable) throws E {
if (readOnly.get().booleanValue()) {
// nested
return callable.call();
} else {
try {
readOnly.set(Boolean.TRUE);
return JavaModelManager.getJavaModelManager().callReadOnlyUnchecked(callable);
} finally {
readOnly.set(Boolean.FALSE);
}
}
}

private <T, E extends Exception> T callReadOnlyUnchecked(JavaCallable<T, E> callable) throws E {
boolean hadTemporaryCache = hasTemporaryCache();
try {
getTemporaryCache();

return cacheZipFiles(callable);
} finally {
if (!hadTemporaryCache) {
resetTemporaryCache();
}
}
}

public static <T, E extends Exception> T cacheZipFiles(JavaCallable<T, E> callable) throws E {
Object instance = new Object();
try {
getJavaModelManager().cacheZipFiles(instance);
return callable.call();
} finally {
getJavaModelManager().flushZipFiles(instance);
}
}
}
Expand Up @@ -104,7 +104,8 @@ public SearchableEnvironment(JavaProject project, org.eclipse.jdt.core.ICompilat
|| !JavaCore.IGNORE.equals(project.getOption(JavaCore.COMPILER_PB_DISCOURAGED_REFERENCE, true));
this.workingCopies = workingCopies;
this.nameLookup = project.newNameLookup(workingCopies, excludeTestCode);
boolean java9plus = CompilerOptions.versionToJdkLevel(project.getOption(JavaCore.COMPILER_COMPLIANCE, true)) >= ClassFileConstants.JDK9;
boolean java9plus = JavaCore.callReadOnly(() -> CompilerOptions
.versionToJdkLevel(project.getOption(JavaCore.COMPILER_COMPLIANCE, true)) >= ClassFileConstants.JDK9);
if (java9plus) {
this.knownModuleLocations = new HashMap<>();

Expand Down

0 comments on commit 9fb036c

Please sign in to comment.