Skip to content

Commit

Permalink
Bug 441726 - JDT performance regression due to bug 410207
Browse files Browse the repository at this point in the history 
Add a cache in JavaModelManager that maps a jar's IPath to its Java
language level, and persist the cache across Eclipse sessions. This
cache makes JarPackageFragmentRoot creation much faster. Items in the
cache are invalidated whenever the DeltaProcessor sees that a jar's
timestamp has changed.

FullSourceWorkspaceModelTests.testGetAllPackageFragmentRoots took 123ms
to execute before this patch, and only 29ms with this patch applied.
The same test took 30ms in Kepler with the patch for bug 411423 applied.

Change-Id: Ic97dff022f9501a25315c35fae42df79b01486cc
Signed-off-by: Terry Parker <[email protected]>
  • Loading branch information
@tparker @jarthana
tparker authored and jarthana committed Aug 18, 2014
1 parent ce05740 commit 3d73760
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 13 deletions.
9 changes: 7 additions & 2 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/DeltaProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Terry Parker <[email protected]> - DeltaProcessor exhibits O(N^2) behavior, see https://bugs.eclipse.org/bugs/show_bug.cgi?id=354332
* Terry Parker <[email protected]> - DeltaProcessor misses state changes in archive files, see https://bugs.eclipse.org/bugs/show_bug.cgi?id=357425
* Terry Parker <[email protected]> - [performance] Low hit rates in JavaModel caches - https://bugs.eclipse.org/421165
* Terry Parker <[email protected]> - Bug 441726 - JDT performance regression due to bug 410207
*******************************************************************************/
package org.eclipse.jdt.internal.core;

Expand Down Expand Up @@ -988,8 +989,9 @@ private boolean createExternalArchiveDelta(HashSet refreshedElements, IProgressM
if (this.state.getExternalLibTimeStamps().remove(entryPath) != null /* file was known*/
&& this.state.roots.get(entryPath) != null /* and it was on the classpath*/) {
externalArchivesStatus.put(entryPath, EXTERNAL_JAR_REMOVED);
// the jar was physically removed: remove the index
// the jar was physically removed: remove the index and from the language level cache
this.manager.indexManager.removeIndex(entryPath);
this.manager.removeFromLanguageLevelsCache(entryPath);
}

} else if (targetLibrary instanceof File){ // external JAR
Expand All @@ -1004,12 +1006,15 @@ private boolean createExternalArchiveDelta(HashSet refreshedElements, IProgressM
if (newTimeStamp == 0){ // file doesn't exist
externalArchivesStatus.put(entryPath, EXTERNAL_JAR_REMOVED);
this.state.getExternalLibTimeStamps().remove(entryPath);
// remove the index
// remove the index and from the language level cache
this.manager.indexManager.removeIndex(entryPath);
this.manager.removeFromLanguageLevelsCache(entryPath);

} else if (oldTimestamp.longValue() != newTimeStamp){
externalArchivesStatus.put(entryPath, EXTERNAL_JAR_CHANGED);
this.state.getExternalLibTimeStamps().put(entryPath, new Long(newTimeStamp));
// remove from the language level cache so it can be recalculated
this.manager.removeFromLanguageLevelsCache(entryPath);
// first remove the index so that it is forced to be re-indexed
this.manager.indexManager.removeIndex(entryPath);
// then index the jar
Expand Down
15 changes: 12 additions & 3 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JarPackageFragmentRoot.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
* Terry Parker <[email protected]> - Bug 441726 - JDT performance regression due to bug 410207
*******************************************************************************/
package org.eclipse.jdt.internal.core;

Expand Down Expand Up @@ -59,12 +60,20 @@ public class JarPackageFragmentRoot extends PackageFragmentRoot {
* does not have an associated <code>IResource</code>.
*/
protected JarPackageFragmentRoot(IPath externalJarPath, JavaProject project) {
this(null, project, Util.getJdkLevel(JavaModel.getTarget(externalJarPath, true)));
}

/**
* Constructs a package fragment root which is the root of the Java package directory hierarchy
* based on a JAR file that is not contained in a <code>IJavaProject</code> and
* does not have an associated <code>IResource</code>.
*/
public JarPackageFragmentRoot(IPath externalJarPath, JavaProject project, long complianceLevel) {
super(null, project);
this.jarPath = externalJarPath;
Object file = JavaModel.getTarget(getPath(), true);
long level = Util.getJdkLevel(file);
this.complianceLevel = CompilerOptions.versionFromJdkLevel(level);
this.complianceLevel = CompilerOptions.versionFromJdkLevel(complianceLevel);
}

/**
* Constructs a package fragment root which is the root of the Java package directory hierarchy
* based on a JAR file.
Expand Down
111 changes: 105 additions & 6 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaModelManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* Terry Parker <[email protected]> - DeltaProcessor misses state changes in archive files, see https://bugs.eclipse.org/bugs/show_bug.cgi?id=357425
* Thirumala Reddy Mutchukota <[email protected]> - Contribution to bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=411423
* Terry Parker <[email protected]> - [performance] Low hit rates in JavaModel caches - https://bugs.eclipse.org/421165
* Terry Parker <[email protected]> - Bug 441726 - JDT performance regression due to bug 410207
*******************************************************************************/
package org.eclipse.jdt.internal.core;

Expand Down Expand Up @@ -93,6 +94,7 @@ public class JavaModelManager implements ISaveParticipant, IContentTypeChangeLis
private static final String INVALID_ARCHIVES_CACHE = "invalidArchivesCache"; //$NON-NLS-1$
private static final String EXTERNAL_FILES_CACHE = "externalFilesCache"; //$NON-NLS-1$
private static final String ASSUMED_EXTERNAL_FILES_CACHE = "assumedExternalFilesCache"; //$NON-NLS-1$
private static final String JAR_LANGUAGE_LEVEL_CACHE = "jarLanguageLevelCache"; //$NON-NLS-1$

/**
* Define a zip cache object.
Expand Down Expand Up @@ -1462,6 +1464,11 @@ public String toString() {
*/
private Set assumedExternalFiles;

/*
* A map of jar IPaths to the language level of each jar.
*/
private Map<IPath, Long> jarLanguageLevels;

/**
* Update the classpath variable cache
*/
Expand Down Expand Up @@ -1598,6 +1605,7 @@ private JavaModelManager() {
this.invalidArchives = loadClasspathListCache(INVALID_ARCHIVES_CACHE);
this.externalFiles = loadClasspathListCache(EXTERNAL_FILES_CACHE);
this.assumedExternalFiles = loadClasspathListCache(ASSUMED_EXTERNAL_FILES_CACHE);
this.jarLanguageLevels = loadJarLanguageLevelsCache();
String includeContainerReferencedLib = System.getProperty(RESOLVE_REFERENCED_LIBRARIES_FOR_CONTAINERS);
this.resolveReferencedLibrariesForContainers = TRUE.equalsIgnoreCase(includeContainerReferencedLib);
}
Expand Down Expand Up @@ -1640,6 +1648,38 @@ public void addExternalFile(IPath path) {
}
}

/**
* Returns a language level for a jar or {@code null} if the jar's language level isn't cached.
*/
public Long getJarLanguageLevel(IPath path) {
if (this.jarLanguageLevels != null) {
return this.jarLanguageLevels.get(path);
}
return null;
}

/**
* Adds a jar to the language levels cache.
*/
public void addToJarLanguageLevelsCache(IPath path, Long languageLevel) {
// unlikely to be null
if (this.jarLanguageLevels == null) {
this.jarLanguageLevels = Collections.synchronizedMap(new HashMap());
}
if (this.jarLanguageLevels != null) {
this.jarLanguageLevels.put(path, languageLevel);
}
}

/**
* Removes a jar from the language levels cache.
*/
public void removeFromLanguageLevelsCache(IPath path) {
if (this.jarLanguageLevels != null) {
this.jarLanguageLevels.remove(path);
}
}

/**
* Starts caching ZipFiles.
* Ignores if there are already clients.
Expand Down Expand Up @@ -3186,7 +3226,7 @@ public void setClasspathBeingResolved(IJavaProject project, boolean classpathIsR

private Set loadClasspathListCache(String cacheName) {
Set pathCache = new HashSet();
File cacheFile = getClasspathListFile(cacheName);
File cacheFile = getClasspathCacheFile(cacheName);
DataInputStream in = null;
try {
in = new DataInputStream(new BufferedInputStream(new FileInputStream(cacheFile)));
Expand All @@ -3209,11 +3249,67 @@ private Set loadClasspathListCache(String cacheName) {
}
return Collections.synchronizedSet(pathCache);
}

private File getClasspathListFile(String fileName) {
return JavaCore.getPlugin().getStateLocation().append(fileName).toFile();

private Map<IPath, Long> loadJarLanguageLevelsCache() {
Map<IPath, Long> pathCache = new HashMap<IPath, Long>();
File cacheFile = getClasspathCacheFile(JAR_LANGUAGE_LEVEL_CACHE);
DataInputStream in = null;
try {
in = new DataInputStream(new BufferedInputStream(new FileInputStream(cacheFile)));
int size = in.readInt();
while (size-- > 0) {
IPath path = Path.fromPortableString(in.readUTF());
long complianceLevel = in.readLong();
pathCache.put(path, complianceLevel);
}
} catch (IOException e) {
if (cacheFile.exists())
Util.log(e, "Unable to read JavaModelManager " + JAR_LANGUAGE_LEVEL_CACHE + " file"); //$NON-NLS-1$ //$NON-NLS-2$
} finally {
if (in != null) {
try {
in.close();
} catch (IOException e) {
// nothing we can do: ignore
}
}
}
return Collections.synchronizedMap(pathCache);
}


private void saveJarLanguageLevelsCache() throws CoreException {
File file = getClasspathCacheFile(JAR_LANGUAGE_LEVEL_CACHE);
DataOutputStream out = null;
try {
out = new DataOutputStream(new BufferedOutputStream(new FileOutputStream(file)));
synchronized (this.jarLanguageLevels) {
out.writeInt(this.jarLanguageLevels.size());
for (Map.Entry<IPath, Long> entry : this.jarLanguageLevels.entrySet()) {
IPath path = entry.getKey();
Long level = entry.getValue();
out.writeUTF(path.toPortableString());
out.writeLong(level);
}
}
} catch (IOException e) {
IStatus status = new Status(IStatus.ERROR, JavaCore.PLUGIN_ID, IStatus.ERROR, "Problems while saving non-chaining jar cache", e); //$NON-NLS-1$
throw new CoreException(status);
} finally {
if (out != null) {
try {
out.close();
} catch (IOException e) {
// Attempt to remove the invalid file.
file.delete();
}
}
}
}

private File getClasspathCacheFile(String fileName) {
return JavaCore.getPlugin().getStateLocation().append(fileName).toFile();
}

private Set getNonChainingJarsCache() throws CoreException {
// Even if there is one entry in the cache, just return it. It may not be
// the complete cache, but avoid going through all the projects to populate the cache.
Expand Down Expand Up @@ -3994,6 +4090,8 @@ public void resetClasspathListCache() {
this.externalFiles.clear();
if (this.assumedExternalFiles != null)
this.assumedExternalFiles.clear();
if (this.jarLanguageLevels != null)
this.jarLanguageLevels.clear();
}

/*
Expand Down Expand Up @@ -4068,7 +4166,7 @@ private void saveBuiltState(PerProjectInfo info) throws CoreException {
}

private void saveClasspathListCache(String cacheName) throws CoreException {
File file = getClasspathListFile(cacheName);
File file = getClasspathCacheFile(cacheName);
DataOutputStream out = null;
try {
out = new DataOutputStream(new BufferedOutputStream(new FileOutputStream(file)));
Expand Down Expand Up @@ -4344,6 +4442,7 @@ public void saving(ISaveContext context) throws CoreException {
saveClasspathListCache(INVALID_ARCHIVES_CACHE);
saveClasspathListCache(EXTERNAL_FILES_CACHE);
saveClasspathListCache(ASSUMED_EXTERNAL_FILES_CACHE);
saveJarLanguageLevelsCache();

// will need delta since this save (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=38658)
context.needDelta();
Expand Down
14 changes: 12 additions & 2 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaProject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Stephan Herrmann <[email protected]> - Contributions for
* Bug 320618 - inconsistent initialization of classpath container backed by external class folder
* Bug 346010 - [model] strange initialization dependency in OptionTests
* Terry Parker <[email protected]> - Bug 441726 - JDT performance regression due to bug 410207
*******************************************************************************/
package org.eclipse.jdt.internal.core;

Expand Down Expand Up @@ -616,7 +617,7 @@ public void computePackageFragmentRoots(
} else if (target instanceof File) {
// external target
if (JavaModel.isFile(target)) {
root = new JarPackageFragmentRoot(entryPath, this);
root = getJarPackageFragmentRoot(entryPath);
} else if (((File) target).isDirectory()) {
root = new ExternalPackageFragmentRoot(entryPath, this);
}
Expand Down Expand Up @@ -1833,7 +1834,16 @@ public IPackageFragmentRoot getPackageFragmentRoot0(IPath externalLibraryPath) {
IFolder linkedFolder = JavaModelManager.getExternalManager().getFolder(externalLibraryPath);
if (linkedFolder != null)
return new ExternalPackageFragmentRoot(linkedFolder, externalLibraryPath, this);
return new JarPackageFragmentRoot(externalLibraryPath, this);
return getJarPackageFragmentRoot(externalLibraryPath);
}

private JarPackageFragmentRoot getJarPackageFragmentRoot(IPath externalLibraryPath) {
Long languageLevel = JavaModelManager.getJavaModelManager().getJarLanguageLevel(externalLibraryPath);
if (languageLevel == null) {
languageLevel = Util.getJdkLevel(JavaModel.getTarget(externalLibraryPath, true));
JavaModelManager.getJavaModelManager().addToJarLanguageLevelsCache(externalLibraryPath, languageLevel);
}
return new JarPackageFragmentRoot(externalLibraryPath, this, languageLevel);
}

/**
Expand Down
15 changes: 15 additions & 0 deletions ...ipse.jdt.core/search/org/eclipse/jdt/internal/core/search/indexing/AddJarFileToIndex.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
* Terry Parker <[email protected]> - Bug 441726 - JDT performance regression due to bug 410207
*******************************************************************************/
package org.eclipse.jdt.internal.core.search.indexing;

Expand All @@ -27,6 +28,8 @@
import org.eclipse.jdt.core.search.SearchEngine;
import org.eclipse.jdt.core.search.SearchParticipant;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileReader;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFormatException;
import org.eclipse.jdt.internal.compiler.parser.Scanner;
import org.eclipse.jdt.internal.compiler.parser.TerminalTokens;
import org.eclipse.jdt.internal.compiler.util.SimpleLookupTable;
Expand Down Expand Up @@ -217,6 +220,7 @@ public boolean execute(IProgressMonitor progressMonitor) {
if ((indexLocation = index.getIndexLocation()) != null) {
indexPath = new Path(indexLocation.getCanonicalFilePath());
}
Long languageLevel = JavaModelManager.getJavaModelManager().getJarLanguageLevel(zipFilePath);
for (Enumeration e = zip.entries(); e.hasMoreElements();) {
if (this.isCancelled) {
if (JobManager.VERBOSE)
Expand All @@ -233,6 +237,17 @@ public boolean execute(IProgressMonitor progressMonitor) {
final byte[] classFileBytes = org.eclipse.jdt.internal.compiler.util.Util.getZipEntryByteContent(ze, zip);
JavaSearchDocument entryDocument = new JavaSearchDocument(ze, zipFilePath, classFileBytes, participant);
this.manager.indexDocument(entryDocument, participant, index, indexPath);
// Update the language level cache value for this jar, since we've already done the expensive IO.
if (languageLevel == null) {
try {
languageLevel = new ClassFileReader(classFileBytes, null).getVersion();
JavaModelManager.getJavaModelManager().addToJarLanguageLevelsCache(zipFilePath, languageLevel);
} catch (ClassFormatException e1) {
// If we have trouble reading a class, stop trying to find this jar's language level.
// Set the local languageLevel variable to a dummy value but don't add it to the cache.
languageLevel = Long.MIN_VALUE;
}
}
}
}
if(this.forceIndexUpdate) {
Expand Down

0 comments on commit 3d73760

Please sign in to comment.