Skip to content

Commit

Permalink
refactor JavaModel.getTarget()
Browse files Browse the repository at this point in the history 
to take heavy weight arguments.

As a preparation for performance fix:
For ClasspathEntry of type CPE_LIBRARY JavaModel.isExternalFile() is
costly to compute and should be cached inside ClasspathEntry

#303
  • Loading branch information
@EcljpseB0T @jukzi
EcljpseB0T authored and jukzi committed Feb 13, 2024
1 parent 8398f6c commit 631bdcc
Show file tree
Hide file tree
Showing 13 changed files with 39 additions and 37 deletions.
27 changes: 14 additions & 13 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ClasspathEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1575,7 +1575,7 @@ public static IAccessRule[] getAccessRules(IPath[] accessibleFiles, IPath[] nonA
@Override
public String toString() {
StringBuilder buffer = new StringBuilder();
Object target = JavaModel.getTarget(getPath(), true);
Object target = JavaModel.getTarget(this, true);
if (target instanceof File)
buffer.append(getPath().toOSString());
else
Expand Down Expand Up @@ -1998,7 +1998,7 @@ public static IJavaModelStatus validateClasspath(IJavaProject javaProject, IClas
break;

case IClasspathEntry.CPE_LIBRARY:
Object target = JavaModel.getTarget(path, false/*don't check resource existence*/);
Object target = JavaModel.getTarget(resolvedEntry, false/*don't check resource existence*/);
hasLibFolder |= target instanceof IContainer;
if ((index = Util.indexOfMatchingPath(path, outputLocations, outputCount)) != -1){
allowNestingInOutputLocations[index] = true;
Expand Down Expand Up @@ -2028,15 +2028,15 @@ public static IJavaModelStatus validateClasspath(IJavaProject javaProject, IClas

// allow nesting source entries in each other as long as the outer entry excludes the inner one
if (kind == IClasspathEntry.CPE_SOURCE
|| (kind == IClasspathEntry.CPE_LIBRARY && (JavaModel.getTarget(entryPath, false/*don't check existence*/) instanceof IContainer))) {
|| (kind == IClasspathEntry.CPE_LIBRARY && (JavaModel.getTarget(entry, false/*don't check existence*/) instanceof IContainer))) {
for (IClasspathEntry otherEntry : classpath) {
if (otherEntry == null) continue;
int otherKind = otherEntry.getEntryKind();
IPath otherPath = otherEntry.getPath();
if (entry != otherEntry
&& (otherKind == IClasspathEntry.CPE_SOURCE
|| (otherKind == IClasspathEntry.CPE_LIBRARY
&& (JavaModel.getTarget(otherPath, false/*don't check existence*/) instanceof IContainer)))) {
&& (JavaModel.getTarget(otherEntry, false/*don't check existence*/) instanceof IContainer)))) {
IPath otherPath = otherEntry.getPath();
char[][] inclusionPatterns, exclusionPatterns;
if (otherPath.isPrefixOf(entryPath)
&& !otherPath.equals(entryPath)
Expand Down Expand Up @@ -2214,9 +2214,9 @@ public static IJavaModelStatus validateClasspathEntry(IJavaProject project, ICla
return status;
}

private static IJavaModelStatus validateClasspathEntry(IJavaProject project, IClasspathEntry entry, IClasspathContainer entryContainer, boolean checkSourceAttachment, boolean referredByContainer){
private static IJavaModelStatus validateClasspathEntry(IJavaProject project, final IClasspathEntry entry, IClasspathContainer entryContainer, boolean checkSourceAttachment, boolean referredByContainer){

IPath path = entry.getPath();
final IPath path = entry.getPath();

// Build some common strings for status message
String projectName = project.getElementName();
Expand Down Expand Up @@ -2301,19 +2301,20 @@ private static IJavaModelStatus validateClasspathEntry(IJavaProject project, ICl
// variable entry check
case IClasspathEntry.CPE_VARIABLE :
if (path.segmentCount() >= 1){
IClasspathEntry resolved;
try {
entry = JavaCore.getResolvedClasspathEntry(entry);
resolved = JavaCore.getResolvedClasspathEntry(entry);
} catch (AssertionFailedException e) {
// Catch the assertion failure and throw java model exception instead
// see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=55992
return new JavaModelStatus(IJavaModelStatusConstants.INVALID_PATH, e.getMessage());
}
if (entry == null){
if (resolved == null){
return new JavaModelStatus(IJavaModelStatusConstants.CP_VARIABLE_PATH_UNBOUND, project, path);
}

// get validation status
IJavaModelStatus status = validateClasspathEntry(project, entry, null, checkSourceAttachment, false/*not referred by container*/);
IJavaModelStatus status = validateClasspathEntry(project, resolved, null, checkSourceAttachment, false/*not referred by container*/);
if (!status.isOK()) return status;

// return deprecation status if any
Expand All @@ -2329,7 +2330,7 @@ private static IJavaModelStatus validateClasspathEntry(IJavaProject project, ICl

// library entry check
case IClasspathEntry.CPE_LIBRARY :
path = ClasspathEntry.resolveDotDot(project.getProject().getLocation(), path);
IPath resolvedPath = ClasspathEntry.resolveDotDot(project.getProject().getLocation(), path);

// do not validate entries from Class-Path: in manifest
// (these entries are considered optional since the user cannot act on them)
Expand All @@ -2343,7 +2344,7 @@ private static IJavaModelStatus validateClasspathEntry(IJavaProject project, ICl
containerInfo = Messages.bind(Messages.classpath_containerInfo, new String[] {entryContainer.getDescription()});
}
}
IJavaModelStatus status = validateLibraryEntry(path, project, containerInfo, checkSourceAttachment ? entry.getSourceAttachmentPath() : null, entryPathMsg, ((ClasspathEntry) entry).isOptional());
IJavaModelStatus status = validateLibraryEntry(resolvedPath, project, containerInfo, checkSourceAttachment ? entry.getSourceAttachmentPath() : null, entryPathMsg, ((ClasspathEntry) entry).isOptional());
if (!status.isOK())
return status;
break;
Expand Down Expand Up @@ -2394,7 +2395,7 @@ private static IJavaModelStatus validateClasspathEntry(IJavaProject project, ICl
}
if (path.isAbsolute() && !path.isEmpty()) {
IPath projectPath= project.getProject().getFullPath();
if (!projectPath.isPrefixOf(path) || JavaModel.getTarget(path, true) == null){
if (!projectPath.isPrefixOf(path) || JavaModel.getTarget(entry, true) == null){
return new JavaModelStatus(IJavaModelStatusConstants.INVALID_CLASSPATH, Messages.bind(Messages.classpath_unboundSourceFolder, new String[] {entryPathMsg, projectName}));
}
} else {
Expand Down
4 changes: 2 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 @@ -1016,7 +1016,7 @@ private boolean createExternalArchiveDelta(Set<IJavaElement> refreshedElements,
boolean deltaContainsModifiedJar = false;
for (int j = 0; j < entries.length; j++){
if (entries[j].getEntryKind() == IClasspathEntry.CPE_LIBRARY) {
IPath entryPath = entries[j].getPath();
final IPath entryPath = entries[j].getPath();

if (!archivePathsToRefresh.contains(entryPath)) continue; // not supposed to be refreshed

Expand All @@ -1027,7 +1027,7 @@ private boolean createExternalArchiveDelta(Set<IJavaElement> refreshedElements,
this.manager.clearExternalFileState(entryPath);

// compute shared status
Object targetLibrary = JavaModel.getTarget(entryPath, true);
Object targetLibrary = JavaModel.getTarget(entries[j], true);

if (targetLibrary == null){ // missing JAR
if (this.state.getExternalLibTimeStamps().remove(entryPath) != null /* file was known*/
Expand Down
4 changes: 1 addition & 3 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JarEntryFile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.eclipse.core.resources.IStorage;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.jdt.core.IJarEntryResource;
import org.eclipse.jdt.core.IJavaModelStatusConstants;
import org.eclipse.jdt.core.IPackageFragmentRoot;
Expand Down Expand Up @@ -53,8 +52,7 @@ public InputStream getContents() throws CoreException {
IPackageFragmentRoot root = getPackageFragmentRoot();
if (Util.isJrt(root.getPath().toOSString())) {
try {
IPath rootPath = root.getPath();
Object target = JavaModel.getTarget(rootPath, false);
Object target = JavaModel.getTarget(root, false);
if (target != null && target instanceof File) {
return JRTUtil.getContentFromJrt((File) target, getEntryName(), root.getElementName());
}
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -120,7 +120,7 @@ protected boolean computeChildren(OpenableElementInfo info, IResource underlying
// always create the default package
rawPackageInfo.put(CharOperation.NO_STRINGS, new ArrayList[] { EMPTY_LIST, EMPTY_LIST });

Object file = JavaModel.getTarget(getPath(), true);
Object file = JavaModel.getTarget(this, true);
long classLevel = Util.getJdkLevel(file);
String projectCompliance = this.getJavaProject().getOption(JavaCore.COMPILER_COMPLIANCE, true);
long projectLevel = CompilerOptions.versionToJdkLevel(projectCompliance);
Expand Down
9 changes: 9 additions & 0 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,15 @@ public static Object getTarget(IPath path, boolean checkResourceExistence) {
return target;
return getExternalTarget(path, checkResourceExistence);
}
/** Return same as calling {@link #getTarget(IPath, boolean)} for {@link IClasspathEntry#getPath()} */
public static Object getTarget(IClasspathEntry entry, boolean checkResourceExistence) {
return getTarget(entry.getPath(), checkResourceExistence);
}
/** Return same as calling {@link #getTarget(IPath, boolean)} for {@link IPackageFragmentRoot#getPath()} */
public static Object getTarget(IPackageFragmentRoot root, boolean checkResourceExistence) {
return getTarget(root.getPath(), checkResourceExistence);
}


/**
* Helper method - returns the {@link IResource} corresponding to the provided {@link IPath},
Expand Down
4 changes: 2 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 @@ -723,7 +723,7 @@ public void computePackageFragmentRoots(
case IClasspathEntry.CPE_SOURCE :

if (projectPath.isPrefixOf(entryPath)){
Object target = JavaModel.getTarget(entryPath, true/*check existency*/);
Object target = JavaModel.getTarget(resolvedEntry, true/*check existency*/);
if (target == null) return;

if (target instanceof IFolder || target instanceof IProject){
Expand All @@ -736,7 +736,7 @@ public void computePackageFragmentRoots(
case IClasspathEntry.CPE_LIBRARY :
if (referringEntry != null && !resolvedEntry.isExported())
return;
Object target = JavaModel.getTarget(entryPath, true/*check existency*/);
Object target = JavaModel.getTarget(resolvedEntry, true/*check existency*/);
if (target == null)
return;

Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ModuleUpdater.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public static Set<String> determineModulesOfProjectsWithNonEmptyClasspath(JavaPr
}
for (IClasspathEntry e1 : expandedClasspath) {
if (e1.getEntryKind() == IClasspathEntry.CPE_PROJECT) {
Object target = JavaModel.getTarget(e1.getPath(), true);
Object target = JavaModel.getTarget(e1, true);
if (target instanceof IProject) {
IProject prereqProject = (IProject) target;
if (JavaProject.hasJavaNature(prereqProject)) {
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SourceMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ private synchronized void computeAllRootPaths(IJavaElement typeOrModule) {
manager.closeZipFile(zip); // handle null case
}
} else {
Object target = JavaModel.getTarget(root.getPath(), true);
Object target = JavaModel.getTarget(root, true);
if (target instanceof IResource) {
IResource resource = (IResource) target;
if (resource instanceof IContainer) {
Expand Down
4 changes: 2 additions & 2 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/NameEnvironment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ private void computeClasspathLocations(
}
ClasspathEntry entry = (ClasspathEntry) classpathEntries[i];
IPath path = entry.getPath();
Object target = JavaModel.getTarget(path, true);
Object target = JavaModel.getTarget(entry, true);
IPath externalAnnotationPath = entry.getExternalAnnotationPath(javaProject.getProject(), true);
if (target == null) continue nextEntry;
boolean isOnModulePath = isOnModulePath(entry);
Expand Down Expand Up @@ -208,7 +208,7 @@ private void computeClasspathLocations(
IPath srcExtAnnotPath = (externalAnnotationPath != null)
? externalAnnotationPath
: prereqEntry.getExternalAnnotationPath(javaProject.getProject(), true);
Object prereqTarget = JavaModel.getTarget(prereqEntry.getPath(), true);
Object prereqTarget = JavaModel.getTarget(prereqEntry, true);
if (!(prereqTarget instanceof IContainer)) continue nextPrereqEntry;
if (srcExtAnnotPath == null) {
// search in other sources contributing to the same binary location (that other loc will be skipped below, due to seen.contains()):
Expand Down
7 changes: 3 additions & 4 deletions org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/HierarchyScope.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,11 @@ public HierarchyScope(IType type, WorkingCopyOwner owner) throws JavaModelExcept
// resource path
IPackageFragmentRoot root = (IPackageFragmentRoot)type.getPackageFragment().getParent();
if (root.isArchive()) {
IPath jarPath = root.getPath();
Object target = JavaModel.getTarget(jarPath, true);
Object target = JavaModel.getTarget(root, true);
String zipFileName;
if (target instanceof IFile) {
// internal jar
zipFileName = jarPath.toString();
zipFileName = root.getPath().toString();
} else if (target instanceof File) {
// external jar
zipFileName = ((File)target).getPath();
Expand Down Expand Up @@ -161,7 +160,7 @@ private void buildResourceVector() {
// type in a jar
JarPackageFragmentRoot jar = (JarPackageFragmentRoot) root;
IPath jarPath = jar.getPath();
Object target = JavaModel.getTarget(jarPath, true);
Object target = JavaModel.getTarget(jar, true);
String zipFileName;
if (target instanceof IFile) {
// internal jar
Expand Down
4 changes: 2 additions & 2 deletions org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/JavaSearchScope.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ void add(JavaProject javaProject, IPath pathToAdd, int includeMask, Set<JavaProj
if ((includeMask & APPLICATION_LIBRARIES) != 0) {
IPath path = entry.getPath();
if (pathToAdd == null || pathToAdd.equals(path)) {
Object target = JavaModel.getTarget(path, false/*don't check existence*/);
Object target = JavaModel.getTarget(entry, false/*don't check existence*/);
if (target instanceof IFolder) // case of an external folder
path = ((IFolder) target).getFullPath();
String pathToString = path.getDevice() == null ? path.toString() : path.toOSString();
Expand All @@ -183,7 +183,7 @@ void add(JavaProject javaProject, IPath pathToAdd, int includeMask, Set<JavaProj
}
IPath path = entry.getPath();
if (pathToAdd == null || pathToAdd.equals(path)) {
Object target = JavaModel.getTarget(path, false/*don't check existence*/);
Object target = JavaModel.getTarget(entry, false/*don't check existence*/);
if (target instanceof IFolder) // case of an external folder
path = ((IFolder) target).getFullPath();
String pathToString = path.getDevice() == null ? path.toString() : path.toOSString();
Expand Down
5 changes: 0 additions & 5 deletions org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/JavaWorkspaceScope.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import java.util.Map;
import java.util.Set;

import org.eclipse.core.resources.IFolder;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Path;
Expand Down Expand Up @@ -100,10 +99,6 @@ public IPath[] enclosingProjectsAndJars() {
for (int j = 0, eLength = entries.length; j < eLength; j++) {
IClasspathEntry entry = entries[j];
if (entry.getEntryKind() == IClasspathEntry.CPE_LIBRARY) {
IPath path = entry.getPath();
Object target = JavaModel.getTarget(path, false/*don't check existence*/);
if (target instanceof IFolder) // case of an external folder
path = ((IFolder) target).getFullPath();
paths.add(entry.getPath());
}
}
Expand Down
2 changes: 1 addition & 1 deletion ....core/search/org/eclipse/jdt/internal/core/search/matching/JavaSearchNameEnvironment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ private ClasspathLocation mapToClassPathLocation(JavaModelManager manager, Packa
ClasspathLocation.forJrtSystem(path.toOSString(), rawClasspathEntry.getAccessRuleSet(), null, compliance) :
ClasspathLocation.forLibrary(manager.getZipFile(path), rawClasspathEntry.getAccessRuleSet(), rawClasspathEntry.isModular(), compliance) ;
} else {
Object target = JavaModel.getTarget(path, true);
Object target = JavaModel.getTarget(root, true);
if (target != null) {
if (root.getKind() == IPackageFragmentRoot.K_SOURCE) {
cp = new ClasspathSourceDirectory((IContainer)target, root.fullExclusionPatternChars(), root.fullInclusionPatternChars());
Expand Down

0 comments on commit 631bdcc

Please sign in to comment.