Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[performance] optimized ClasspathDirectory #3433

Merged
merged 2 commits into from
Dec 10, 2024
Merged

[performance] optimized ClasspathDirectory #3433

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a0e053
[performance] optimized ClasspathDirectory
EcljpseB0T Dec 10, 2024
9c059ca
reset ClasspathDirectory.packageSecondaryTypes #3433
EcljpseB0T Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 39 additions & 40 deletions ...t.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/ClasspathDirectory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,18 @@
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.HashSet;
import java.util.Hashtable;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import java.util.stream.Stream;
import org.eclipse.jdt.core.compiler.CharOperation;
import org.eclipse.jdt.internal.compiler.CompilationResult;
import org.eclipse.jdt.internal.compiler.DefaultErrorHandlingPolicies;
import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
import org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileReader;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFormatException;
import org.eclipse.jdt.internal.compiler.classfmt.ExternalAnnotationProvider;
Expand All @@ -51,18 +52,17 @@
import org.eclipse.jdt.internal.compiler.problem.ProblemReporter;
import org.eclipse.jdt.internal.compiler.util.Util;

@SuppressWarnings({"rawtypes", "unchecked"})
public class ClasspathDirectory extends ClasspathLocation {

private Hashtable directoryCache;
private final String[] missingPackageHolder = new String[1];
private final int mode; // ability to only consider one kind of files (source vs. binaries), by default use both
private final String encoding; // only useful if referenced in the source path
private Hashtable<String, Hashtable<String, String>> packageSecondaryTypes = null;
Map options;
private final Map<String, String[]> directoryCache = new ConcurrentHashMap<>();
private final String[] missingPackageHolder = new String[1];
private final int mode; // ability to only consider one kind of files (source vs. binaries), by default use both
private final String encoding; // only useful if referenced in the source path
private final Map<String, Map<String, String>> packageSecondaryTypes = new ConcurrentHashMap<>();
private final Map<String, String> options;

ClasspathDirectory(File directory, String encoding, int mode,
AccessRuleSet accessRuleSet, String destinationPath, Map options) {
AccessRuleSet accessRuleSet, String destinationPath, Map<String, String> options) {
super(accessRuleSet, destinationPath);
this.mode = mode;
this.options = options;
Expand All @@ -74,43 +74,47 @@ public class ClasspathDirectory extends ClasspathLocation {
}
if (!this.path.endsWith(File.separator))
this.path += File.separator;
this.directoryCache = new Hashtable(11);
this.encoding = encoding;
}
String[] directoryList(String qualifiedPackageName) {
if (File.separatorChar != '/' && qualifiedPackageName.indexOf('/') != -1) {
qualifiedPackageName = qualifiedPackageName.replace('/', File.separatorChar);

private String[] directoryList(String qualifiedPackageName) {
String[] cached = this.directoryCache.computeIfAbsent(qualifiedPackageName, this::computeDirectoryList);
if (cached == this.missingPackageHolder) {
return null; // package exists in another classpath directory or jar
}
String[] dirList = (String[]) this.directoryCache.get(qualifiedPackageName);
if (dirList == this.missingPackageHolder) return null; // package exists in another classpath directory or jar
if (dirList != null) return dirList;
return cached;
}

File dir = new File(this.path + qualifiedPackageName);
notFound : if (dir.isDirectory()) {
private String[] computeDirectoryList(String qualifiedPackageName) {
String qualifiedPackagePath = qualifiedPackageName.replace('/', File.separatorChar);
File dir = new File(this.path + qualifiedPackagePath);
String[] dirList = dir.list();
notFound: if (dirList != null) { // if isDirectory
// must protect against a case insensitive File call
// walk the qualifiedPackageName backwards looking for an uppercase character before the '/'
int index = qualifiedPackageName.length();
int last = qualifiedPackageName.lastIndexOf(File.separatorChar);
while (--index > last && !ScannerHelper.isUpperCase(qualifiedPackageName.charAt(index))){/*empty*/}
int index = qualifiedPackagePath.length();
int last = qualifiedPackagePath.lastIndexOf(File.separatorChar);
while (--index > last && !ScannerHelper.isUpperCase(qualifiedPackagePath.charAt(index))) {
/* empty */}
if (index > last) {
if (last == -1) {
if (!doesFileExist(qualifiedPackageName, Util.EMPTY_STRING))
if (!doesFileExist(qualifiedPackagePath, Util.EMPTY_STRING))
break notFound;
} else {
String packageName = qualifiedPackageName.substring(last + 1);
String parentPackage = qualifiedPackageName.substring(0, last);
String packageName = qualifiedPackagePath.substring(last + 1);
String parentPackage = qualifiedPackagePath.substring(0, last);
if (!doesFileExist(packageName, parentPackage))
break notFound;
}
}
if ((dirList = dir.list()) == null)
if (dirList.length == 0) {
dirList = CharOperation.NO_STRINGS;
this.directoryCache.put(qualifiedPackageName, dirList);
}
return dirList;
}
this.directoryCache.put(qualifiedPackageName, this.missingPackageHolder);
return null;
return this.missingPackageHolder;
}

boolean doesFileExist(String fileName, String qualifiedPackageName) {
String[] dirList = directoryList(qualifiedPackageName);
if (dirList == null) return false; // most common case
Expand All @@ -121,7 +125,7 @@ boolean doesFileExist(String fileName, String qualifiedPackageName) {
return false;
}
@Override
public List fetchLinkedJars(FileSystem.ClasspathSectionProblemReporter problemReporter) {
public List<Classpath> fetchLinkedJars(FileSystem.ClasspathSectionProblemReporter problemReporter) {
return null;
}
private NameEnvironmentAnswer findClassInternal(char[] typeName, String qualifiedPackageName, String qualifiedBinaryFileName, boolean asBinaryOnly) {
Expand Down Expand Up @@ -202,10 +206,10 @@ public NameEnvironmentAnswer findClass(char[] typeName, String qualifiedPackageN
/**
* Add all the secondary types in the package
*/
private Hashtable<String, String> getSecondaryTypes(String qualifiedPackageName) {
Hashtable<String, String> packageEntry = new Hashtable<>();
private Map<String, String> getSecondaryTypes(String qualifiedPackageName) {
Map<String, String> packageEntry = new ConcurrentHashMap<>();

String[] dirList = (String[]) this.directoryCache.get(qualifiedPackageName);
String[] dirList = this.directoryCache.get(qualifiedPackageName);
if (dirList == this.missingPackageHolder // package exists in another classpath directory or jar
|| dirList == null)
return packageEntry;
Expand Down Expand Up @@ -241,13 +245,7 @@ private Hashtable<String, String> getSecondaryTypes(String qualifiedPackageName)
return packageEntry;
}
private NameEnvironmentAnswer findSourceSecondaryType(String typeName, String qualifiedPackageName, String qualifiedBinaryFileName) {

if (this.packageSecondaryTypes == null) this.packageSecondaryTypes = new Hashtable<>();
Hashtable<String, String> packageEntry = this.packageSecondaryTypes.get(qualifiedPackageName);
if (packageEntry == null) {
packageEntry = getSecondaryTypes(qualifiedPackageName);
this.packageSecondaryTypes.put(qualifiedPackageName, packageEntry);
}
Map<String, String> packageEntry = this.packageSecondaryTypes.computeIfAbsent(qualifiedPackageName, this::getSecondaryTypes);
String fileName = packageEntry.get(typeName);
return fileName != null ? new NameEnvironmentAnswer(new CompilationUnit(null,
fileName, this.encoding, this.destinationPath),
Expand Down Expand Up @@ -360,7 +358,8 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
@Override
public void reset() {
super.reset();
this.directoryCache = new Hashtable(11);
this.directoryCache.clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why teh secondaryTypes are not cleared?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wondered about that. i decided to only make a performance change here keeping logic as is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpalat : any clue? packageSecondaryTypes field was added in c39467a

Copy link
Contributor

@mpalat mpalat Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpalat : any clue? packageSecondaryTypes field was added in c39467a

I believe its a bug - @jukzi could you please add code to clear this also in reset alongwith this PR. If not I can create another PR to address this. please let me know

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

this.packageSecondaryTypes.clear();
}
@Override
public String toString() {
Expand Down
Loading