Skip to content

Commit

Permalink
[performance] share JRT entries across projects #2884
Browse files Browse the repository at this point in the history 
JarPackageFragmentRootInfo.rawPackageInfo hold a list of all files in
the JRT but each projects gets its own JarPackageFragmentRootInfo.
Calculate that files independent of the instance and reuse if for all
projects. To safely share the result across threads an unmodifiable
datastructure is used.

* use record instead of String[2] for classes vs resources
* reduce rawtypes, unchecked conversions
* avoid char[] to String conversions

tested by JavaProjectTests, AttachedJavadocTests

#2884
  • Loading branch information
@EcljpseB0T @jukzi
EcljpseB0T authored and jukzi committed Sep 17, 2024
1 parent 1a1f512 commit a431ff8
Show file tree
Hide file tree
Showing 10 changed files with 300 additions and 133 deletions.
48 changes: 48 additions & 0 deletions ...dt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/DeduplicationUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*******************************************************************************/
package org.eclipse.jdt.core.tests.compiler;

import java.util.Arrays;
import java.util.List;
import java.util.function.Supplier;

Expand All @@ -38,6 +39,7 @@ public void testDeduplication() {
assertDeduplicationString("aNeverUsedBefore".toCharArray());
assertDeduplicationCharArray("aNeverUsedBefore");
assertDeduplicationObject(() -> List.of("aNeverUsedBefore"));
assertDeduplicationList(() -> Arrays.asList("1","2","3"));

assertDeduplicationString("bNeverUsedBefore".toCharArray());
assertDeduplicationCharArray("bNeverUsedBefore");
Expand Down Expand Up @@ -133,6 +135,52 @@ private void assertDeduplicationObject(Supplier<Object> supplier) {
}
}

private void assertDeduplicationList(Supplier<List<String>> supplier) {
{ // weak
List<String> a = supplier.get();
List<String> b = supplier.get();
assertNotSame(a, b);
assertEquals(a, b);
List<String> expected = DeduplicationUtil.intern(b);
for (int i = 0; i < a.size(); i++) {
assertSame(b.get(i), expected.get(i));
}
b = null;
expected = null;

forceGc();

// Now "b" is not referenced anymore, can be garbage collected
// and DeduplicationUtil is supposed to release weak reference to it
// so after trying to intern "a" we will get "a" and not previously set "b"
List<String> actual = DeduplicationUtil.intern(a);

// It is impossible to rely on GC to run immediately, so loop few times
for (int i = 0; i < 42; i++) {
if(actual != a) {
forceGc();
actual = DeduplicationUtil.intern(a);
} else {
break;
}
}
for (int i = 0; i < a.size(); i++) {
assertSame(a.get(i), actual.get(i));
}
}
{ // strong
List<String> a = supplier.get();
List<String> b = supplier.get();
assertNotSame(a, b);
assertEquals(a, b);
List<String> actual = DeduplicationUtil.intern(a);
List<String> expected = DeduplicationUtil.intern(b);

// since "a" is still referenced, we should get it
assertSame(expected, actual);
}
}

private void forceGc() {
System.gc();
System.runFinalization();
Expand Down
54 changes: 51 additions & 3 deletions org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/JavaProjectTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.io.FileOutputStream;
import java.io.FileWriter;
import java.io.IOException;
import java.util.Arrays;
import java.util.Comparator;
import java.util.Hashtable;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
Expand Down Expand Up @@ -1686,21 +1688,67 @@ public void testPackageFragmentRootNonJavaResources9() throws Exception {
// the bug occurred only if META-INF/MANIFEST.MF was before META-INF in the ZIP file
// Altered the test for 534624. Usage of Zip file system for traversal no longer sees two different entries, but just the file.
zip.putNextEntry(new ZipEntry("META-INF/MANIFEST.MF"));

// a java class:
zip.putNextEntry(new ZipEntry("p1/p2/p3/java.class"));

// some more resources:
zip.putNextEntry(new ZipEntry("legalPackageName/MANIFEST2.MF"));
zip.putNextEntry(new ZipEntry("r1/r2/r3/resource.png"));
zip.putNextEntry(new ZipEntry("p/x.y/Test.txt"));
zip.putNextEntry(new ZipEntry("p1/-invalid-/p3/fake.class"));

/// just another resource:
zip.putNextEntry(new ZipEntry("invalid-/legalPackageName2/-MANIFEST.MF2"));
}
createJavaProject("P", new String[0], new String[] {getExternalResourcePath("lib.jar")}, "");
waitForManualRefresh();
IPackageFragmentRoot root = getPackageFragmentRoot("P", getExternalResourcePath("lib.jar"));
Object[] resources = root.getNonJavaResources();
assertResourceTreeEquals(
"unexpected non java resources",
"META-INF\n" +
" MANIFEST.MF",
resources);
"""
META-INF
MANIFEST.MF
invalid-
legalPackageName2
-MANIFEST.MF2"""
,resources);
IJavaElement[] children= root.getChildren();
Arrays.sort(children, Comparator.comparing(IJavaElement::getElementName));
String expected = """
<default> [in lib.jar]
legalPackageName [in lib.jar]
p [in lib.jar]
p1 [in lib.jar]
p1.p2 [in lib.jar]
p1.p2.p3 [in lib.jar]
r1 [in lib.jar]
r1.r2 [in lib.jar]
r1.r2.r3 [in lib.jar]""";
expected = expected.replace("lib.jar", root.getPath().toOSString());
assertElementsEqual("Unexpected package fragment roots", expected, children);

IPackageFragment pf= getPackageFragment("P", getExternalResourcePath("lib.jar"), "legalPackageName");
assertResourceTreeEquals(
"unexpected non java resources",
"""
MANIFEST2.MF"""
, pf.getNonJavaResources());
IPackageFragment pf2= getPackageFragment("P", getExternalResourcePath("lib.jar"), "p1");
assertResourceTreeEquals(
"unexpected non java resources",
"""
-invalid-
p3
fake.class"""
, pf2.getNonJavaResources());
} finally {
deleteExternalResource("lib.jar");
deleteProject("P");
}
}

/**
* Test raw entry inference performance for package fragment root
*/
Expand Down
13 changes: 3 additions & 10 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JModPackageFragmentRoot.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@

import org.eclipse.core.runtime.IPath;
import org.eclipse.jdt.core.IClasspathAttribute;
import org.eclipse.jdt.core.compiler.CharOperation;
import org.eclipse.jdt.internal.core.builder.ClasspathJMod;
import org.eclipse.jdt.internal.core.util.HashtableOfArrayToObject;

/**
* A package fragment root that corresponds to a JMod file.
Expand Down Expand Up @@ -47,15 +45,10 @@ protected JModPackageFragmentRoot(IPath externalPath, JavaProject project, IClas
*/
@Override
public String getClassFilePath(String entryName) {
char[] name = CharOperation.append(ClasspathJMod.CLASSES_FOLDER, entryName.toCharArray());
return new String(name);
return ClasspathJMod.CLASSES_FOLDER + entryName;
}
@Override
protected void initRawPackageInfo(HashtableOfArrayToObject rawPackageInfo, String entryName, boolean isDirectory, String compliance) {
char[] name = entryName.toCharArray();
if (CharOperation.prefixEquals(ClasspathJMod.CLASSES_FOLDER, name)) {
name = CharOperation.subarray(name, ClasspathJMod.CLASSES_FOLDER.length, name.length);
}
super.initRawPackageInfo(rawPackageInfo, new String(name), isDirectory, compliance);
protected String getClassNameSubFolder() {
return ClasspathJMod.CLASSES_FOLDER;
}
}
53 changes: 25 additions & 28 deletions org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JarPackageFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
package org.eclipse.jdt.internal.core;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;

import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.IPath;
Expand All @@ -29,6 +31,7 @@
import org.eclipse.jdt.core.IJavaModelStatusConstants;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.internal.compiler.lookup.TypeConstants;
import org.eclipse.jdt.internal.core.JarPackageFragmentRootInfo.PackageContent;
import org.eclipse.jdt.internal.core.util.DeduplicationUtil;
import org.eclipse.jdt.internal.core.util.Util;

Expand All @@ -37,7 +40,6 @@
*
* @see org.eclipse.jdt.core.IPackageFragment
*/
@SuppressWarnings({ "rawtypes", "unchecked" })
class JarPackageFragment extends PackageFragment {
/**
* Constructs a package fragment that is contained within a jar or a zip.
Expand All @@ -52,16 +54,16 @@ protected JarPackageFragment(PackageFragmentRoot root, String[] names) {
protected boolean buildStructure(OpenableElementInfo info, IProgressMonitor pm, Map newElements, IResource underlyingResource) throws JavaModelException {
JarPackageFragmentRoot root = (JarPackageFragmentRoot) getParent();
JarPackageFragmentRootInfo parentInfo = (JarPackageFragmentRootInfo) root.getElementInfo();
ArrayList[] entries = (ArrayList[]) parentInfo.rawPackageInfo.get(this.names);
PackageContent entries = parentInfo.rawPackageInfo.get(Arrays.asList(this.names));
if (entries == null)
throw newNotPresentException();
JarPackageFragmentInfo fragInfo = (JarPackageFragmentInfo) info;

// compute children
fragInfo.setChildren(computeChildren(entries[0/*class files*/]));
fragInfo.setChildren(computeChildren(entries.javaClasses()));

// compute non-Java resources
fragInfo.setNonJavaResources(computeNonJavaResources(entries[1/*non Java resources*/]));
fragInfo.setNonJavaResources(computeNonJavaResources(entries.resources()));

newElements.put(this, fragInfo);
return true;
Expand All @@ -70,13 +72,13 @@ protected boolean buildStructure(OpenableElementInfo info, IProgressMonitor pm,
* Compute the children of this package fragment. Children of jar package fragments
* can only be IClassFile (representing .class files).
*/
private IJavaElement[] computeChildren(ArrayList namesWithoutExtension) {
private IJavaElement[] computeChildren(List<String> namesWithoutExtension) {
int size = namesWithoutExtension.size();
if (size == 0)
return NO_ELEMENTS;
IJavaElement[] children = new IJavaElement[size];
for (int i = 0; i < size; i++) {
String nameWithoutExtension = (String) namesWithoutExtension.get(i);
String nameWithoutExtension = namesWithoutExtension.get(i);
if (TypeConstants.MODULE_INFO_NAME_STRING.equals(nameWithoutExtension))
children[i] = new ModularClassFile(this);
else
Expand All @@ -87,15 +89,14 @@ private IJavaElement[] computeChildren(ArrayList namesWithoutExtension) {
/**
* Compute all the non-java resources according to the given entry names.
*/
private Object[] computeNonJavaResources(ArrayList entryNames) {
int length = entryNames.size();
if (length == 0)
private Object[] computeNonJavaResources(List<String> entryNames) {
if (entryNames.isEmpty()) {
return JavaElementInfo.NO_NON_JAVA_RESOURCES;
HashMap jarEntries = new HashMap(); // map from IPath to IJarEntryResource
HashMap childrenMap = new HashMap(); // map from IPath to ArrayList<IJarEntryResource>
ArrayList topJarEntries = new ArrayList();
for (int i = 0; i < length; i++) {
String resName = (String) entryNames.get(i);
}
HashMap<IPath, IJarEntryResource> jarEntries = new HashMap<>();
HashMap<IPath, ArrayList<IPath>> childrenMap = new HashMap<>();
ArrayList<IJarEntryResource> topJarEntries = new ArrayList<>();
for (String resName: entryNames) {
// consider that a .java file is not a non-java resource (see bug 12246 Packages view shows .class and .java files when JAR has source)
if (!Util.isJavaLikeFileName(resName)) {
IPath filePath = new Path(resName);
Expand All @@ -112,11 +113,11 @@ private Object[] computeNonJavaResources(ArrayList entryNames) {
} else {
IPath parentPath = childPath.removeLastSegments(1);
while (parentPath.segmentCount() > 0) {
ArrayList parentChildren = (ArrayList) childrenMap.get(parentPath);
ArrayList<IPath> parentChildren = childrenMap.get(parentPath);
if (parentChildren == null) {
Object dir = new JarEntryDirectory(parentPath.lastSegment());
JarEntryDirectory dir = new JarEntryDirectory(parentPath.lastSegment());
jarEntries.put(parentPath, dir);
childrenMap.put(parentPath, parentChildren = new ArrayList());
childrenMap.put(parentPath, parentChildren = new ArrayList<>());
parentChildren.add(childPath);
if (parentPath.segmentCount() == 1) {
topJarEntries.add(dir);
Expand All @@ -132,11 +133,9 @@ private Object[] computeNonJavaResources(ArrayList entryNames) {
}
}
}
Iterator entries = childrenMap.entrySet().iterator();
while (entries.hasNext()) {
Map.Entry entry = (Map.Entry) entries.next();
IPath entryPath = (IPath) entry.getKey();
ArrayList entryValue = (ArrayList) entry.getValue();
for (Entry<IPath, ArrayList<IPath>> entry: childrenMap.entrySet()) {
IPath entryPath = entry.getKey();
ArrayList<IPath> entryValue = entry.getValue();
JarEntryDirectory jarEntryDirectory = (JarEntryDirectory) jarEntries.get(entryPath);
int size = entryValue.size();
IJarEntryResource[] children = new IJarEntryResource[size];
Expand All @@ -150,7 +149,7 @@ private Object[] computeNonJavaResources(ArrayList entryNames) {
jarEntryDirectory.setParent(this);
}
}
return topJarEntries.toArray(new Object[topJarEntries.size()]);
return topJarEntries.toArray(Object[]::new);
}
/**
* Returns true if this fragment contains at least one java resource.
Expand Down Expand Up @@ -179,10 +178,8 @@ protected JarPackageFragmentInfo createElementInfo() {
*/
@Override
public IClassFile[] getAllClassFiles() throws JavaModelException {
ArrayList list = getChildrenOfType(CLASS_FILE);
IClassFile[] array= new IClassFile[list.size()];
list.toArray(array);
return array;
ArrayList<?> list = getChildrenOfType(CLASS_FILE);
return list.toArray(IClassFile[]::new);
}
/**
* A jar package fragment never contains compilation units.
Expand Down
Loading

0 comments on commit a431ff8

Please sign in to comment.