Performance: Add public API for Batch Reads in UI - closes eclipse-jd…

…t#1614

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.

eclipse-jdt#1614

org.eclipse.jdt.core/.options

@@ -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
 

org.eclipse.jdt.core/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jdt.core; singleton:=true
-Bundle-Version: 3.36.100.qualifier
+Bundle-Version: 3.37.0.qualifier
 Bundle-Activator: org.eclipse.jdt.core.JavaCore
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin

org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/ASTParser.java

@@ -1116,6 +1116,9 @@ public IBinding[] createBindings(IJavaElement[] elements, IProgressMonitor monit
 	}
 
 	private ASTNode internalCreateAST(IProgressMonitor monitor) {
+		return JavaCore.callReadOnly(() -> internalCreateASTCached(monitor));
+	}
+	private ASTNode internalCreateASTCached(IProgressMonitor monitor) {
 		boolean needToResolveBindings = (this.bits & CompilationUnitResolver.RESOLVE_BINDING) != 0;
 		switch(this.astKind) {
 			case K_CLASS_BODY_DECLARATIONS :

org.eclipse.jdt.core/model/org/eclipse/jdt/core/JavaCore.java

@@ -5963,6 +5963,9 @@ public static void run(IWorkspaceRunnable action, IProgressMonitor monitor) thro
 	 * @since 3.0
 	 */
 	public static void run(IWorkspaceRunnable action, ISchedulingRule rule, IProgressMonitor monitor) throws CoreException {
+		if (readOnly.get().booleanValue()) {
+			throw new IllegalStateException("Its not allow to modify JavaModel during ReadOnly action."); //$NON-NLS-1$
+		}
 		IWorkspace workspace = ResourcesPlugin.getWorkspace();
 		if (workspace.isTreeLocked()) {
 			new BatchOperation(action).run(monitor);
@@ -5971,6 +5974,77 @@ 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;
+	}
+
+	private static ThreadLocal<Boolean> readOnly = ThreadLocal.withInitial(() -> Boolean.FALSE);
+
+	/**
+	 * 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 (caching
+	 * things 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 Exception that is thrown by the argument.
+	 * @since 3.37
+	 */
+	public static <T, E extends Exception> T callReadOnly(JavaCallable<T, E> callable) throws E {
+		boolean wasReadonly = readOnly.get().booleanValue();
+		try {
+			readOnly.set(Boolean.TRUE);
+			return JavaModelManager.getJavaModelManager().callReadOnly(callable);
+		} finally {
+			if (!wasReadonly) {
+				readOnly.set(Boolean.FALSE);
+			}
+		}
+	}
+
+	/**
+	 * 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 Exception that is thrown by the argument.
+	 * @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.

org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaModelManager.java

@@ -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;
@@ -125,6 +128,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;
@@ -376,6 +380,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$
@@ -1652,6 +1657,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;
 
@@ -1956,6 +1962,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);
 
@@ -2946,19 +2953,50 @@ 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<>();
+
 	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) {
+				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();
+					}
+				}
+			}
 			if (ZIP_ACCESS_VERBOSE) {
 				trace("(" + Thread.currentThread() + ") [JavaModelManager.getZipFile(IPath)] Creating ZipFile on " + localFile ); //$NON-NLS-1$ //$NON-NLS-2$
 			}
@@ -5736,4 +5774,23 @@ public ClasspathAccessRule getAccessRuleForProblemId(char [] filePattern, int pr
 	private ClasspathAccessRule getFromCache(ClasspathAccessRule rule) {
 		return DeduplicationUtil.internObject(rule);
 	}
+
+	public <T, E extends Exception> T callReadOnly(JavaCallable<T, E> callable) throws E {
+		boolean hadTemporaryCache = hasTemporaryCache();
+		try {
+			getTemporaryCache();
+
+			Object instance = new Object();
+			try {
+				cacheZipFiles(instance);
+				return callable.call();
+			} finally {
+				flushZipFiles(instance);
+			}
+		} finally {
+			if (!hadTemporaryCache) {
+				resetTemporaryCache();
+			}
+		}
+	}
 }

org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java

@@ -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<>();
 

org.eclipse.jdt.core/pom.xml

@@ -17,7 +17,7 @@
     <version>4.31.0-SNAPSHOT</version>
   </parent>
   <artifactId>org.eclipse.jdt.core</artifactId>
-  <version>3.36.100-SNAPSHOT</version>
+  <version>3.37.0-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
 
   <properties>