Skip to content

Commit 5780a4c

Browse files
mbienmbien
authored
Merge pull request apache#8437 from mbien/unused-pkgprivate-performance
Cache ClassIndex during UnusedDetector search
2 parents 62b988d + 9bacf23 commit 5780a4c

File tree

4 files changed

+56
-56
lines changed

4 files changed

+56
-56
lines changed

‎java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/SemanticHighlighterBase.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@
9191
*
9292
* @author Jan Lahoda
9393
*/
94-
public abstract class SemanticHighlighterBase extends JavaParserResultTask {
94+
public abstract class SemanticHighlighterBase extends JavaParserResultTask<Result> {
9595

9696
public static final String JAVA_INLINE_HINT_PARAMETER_NAME = "javaInlineHintParameterName"; //NOI18N
9797
public static final String JAVA_INLINE_HINT_CHAINED_TYPES = "javaInlineHintChainedTypes"; //NOI18N
@@ -197,7 +197,7 @@ protected boolean process(CompilationInfo info, final Document doc, Settings set
197197

198198
Map<Element, List<UnusedDescription>> element2Unused = UnusedDetector.findUnused(info, () -> cancel.get()) //XXX: unnecessarily ugly
199199
.stream()
200-
.collect(Collectors.groupingBy(ud -> ud.unusedElement));
200+
.collect(Collectors.groupingBy(ud -> ud.unusedElement()));
201201
for (Map.Entry<Element, List<Use>> entry : v.type2Uses.entrySet()) {
202202
if (cancel.get())
203203
return true;

‎java/java.editor.base/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetector.java‎

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -82,20 +82,10 @@
8282
*/
8383
public class UnusedDetector {
8484

85-
public static class UnusedDescription {
86-
public final Element unusedElement;
87-
public final TreePath unusedElementPath;
88-
public final boolean packagePrivate;
89-
public final UnusedReason reason;
90-
91-
public UnusedDescription(Element unusedElement, TreePath unusedElementPath, boolean packagePrivate, UnusedReason reason) {
92-
this.unusedElement = unusedElement;
93-
this.unusedElementPath = unusedElementPath;
94-
this.packagePrivate = packagePrivate;
95-
this.reason = reason;
96-
}
97-
98-
}
85+
public record UnusedDescription(Element unusedElement, TreePath unusedElementPath, boolean packagePrivate, UnusedReason reason) {}
86+
87+
private static final Object RESULTS_KEY = new Object();
88+
private static final Object CLASS_INDEX_KEY = new Object();
9989

10090
public enum UnusedReason {
10191
NOT_WRITTEN_READ("neither read or written to"),
@@ -111,7 +101,8 @@ private UnusedReason(String text) {
111101
}
112102

113103
public static List<UnusedDescription> findUnused(CompilationInfo info, Callable<Boolean> cancel) {
114-
List<UnusedDescription> cached = (List<UnusedDescription>) info.getCachedValue(UnusedDetector.class);
104+
@SuppressWarnings("unchecked")
105+
List<UnusedDescription> cached = (List<UnusedDescription>) info.getCachedValue(RESULTS_KEY);
115106
if (cached != null) {
116107
return cached;
117108
}
@@ -181,7 +172,7 @@ public static List<UnusedDescription> findUnused(CompilationInfo info, Callable<
181172
}
182173
}
183174

184-
info.putCachedValue(UnusedDetector.class, result, CompilationInfo.CacheClearPolicy.ON_CHANGE);
175+
info.putCachedValue(RESULTS_KEY, result, CompilationInfo.CacheClearPolicy.ON_CHANGE);
185176

186177
return result;
187178
}
@@ -361,28 +352,17 @@ public boolean isDependencies() {
361352
default:
362353
return true;
363354
}
364-
ElementHandle eh = ElementHandle.create(el);
365-
Project prj = FileOwnerQuery.getOwner(info.getFileObject());
366-
ClasspathInfo cpInfo;
367-
if (prj != null) {
368-
SourceGroup[] sourceGroups = ProjectUtils.getSources(prj).getSourceGroups(JavaProjectConstants.SOURCES_TYPE_JAVA);
369-
FileObject[] roots = new FileObject[sourceGroups.length];
370-
for (int i = 0; i < sourceGroups.length; i++) {
371-
SourceGroup sourceGroup = sourceGroups[i];
372-
roots[i] = sourceGroup.getRootFolder();
373-
}
374-
cpInfo = ClasspathInfo.create(ClassPath.EMPTY, ClassPath.EMPTY, ClassPathSupport.createClassPath(roots));
375-
} else {
376-
cpInfo = info.getClasspathInfo();
377-
}
378-
Set<FileObject> res = cpInfo.getClassIndex().getResources(ElementHandle.create(typeElement), searchKinds, scope);
355+
FileObject fileObject = info.getFileObject();
356+
ClassIndex classIndex = getCachedClassIndex(fileObject, info);
357+
Set<FileObject> res = classIndex.getResources(ElementHandle.create(typeElement), searchKinds, scope);
379358
if (res != null) {
359+
ElementHandle<Element> eh = ElementHandle.create(el);
380360
for (FileObject fo : res) {
381361
try {
382362
if (Boolean.TRUE.equals(cancel.call())) {
383363
return false;
384364
}
385-
if (fo != info.getFileObject()) {
365+
if (fo != fileObject) {
386366
JavaSource js = JavaSource.forFileObject(fo);
387367
if (js == null) {
388368
return false;
@@ -397,6 +377,7 @@ public Void scan(Tree tree, Element p) {
397377
Element element = cc.getTrees().getElement(new TreePath(getCurrentPath(), tree));
398378
if (element != null && eh.signatureEquals(element)) {
399379
found.set(true);
380+
return null;
400381
}
401382
super.scan(tree, p);
402383
}
@@ -417,6 +398,29 @@ public Void scan(Tree tree, Element p) {
417398
return false;
418399
}
419400

401+
// obtaining the ClassIndex can be expensive, this is usually called multiple times per search
402+
private static ClassIndex getCachedClassIndex(FileObject fileObject, CompilationInfo info) {
403+
ClassIndex classIndex = (ClassIndex) info.getCachedValue(CLASS_INDEX_KEY);
404+
if (classIndex == null) {
405+
ClasspathInfo cpInfo;
406+
Project prj = FileOwnerQuery.getOwner(fileObject);
407+
if (prj != null) {
408+
SourceGroup[] sourceGroups = ProjectUtils.getSources(prj).getSourceGroups(JavaProjectConstants.SOURCES_TYPE_JAVA);
409+
FileObject[] roots = new FileObject[sourceGroups.length];
410+
for (int i = 0; i < sourceGroups.length; i++) {
411+
SourceGroup sourceGroup = sourceGroups[i];
412+
roots[i] = sourceGroup.getRootFolder();
413+
}
414+
cpInfo = ClasspathInfo.create(ClassPath.EMPTY, ClassPath.EMPTY, ClassPathSupport.createClassPath(roots));
415+
} else {
416+
cpInfo = info.getClasspathInfo();
417+
}
418+
classIndex = cpInfo.getClassIndex();
419+
info.putCachedValue(CLASS_INDEX_KEY, classIndex, CompilationInfo.CacheClearPolicy.ON_CHANGE);
420+
}
421+
return classIndex;
422+
}
423+
420424
private static List<UsedDetector> collectUsedDetectors(CompilationInfo info) {
421425
List<UsedDetector> detectors = new ArrayList<>();
422426
for (UsedDetector.Factory factory : Lookup.getDefault().lookupAll(UsedDetector.Factory.class)) {
@@ -661,7 +665,7 @@ public Void visitLiteral(LiteralTree node, Void p) {
661665
@Override
662666
public Void visitMethodInvocation(MethodInvocationTree node, Void p) {
663667
Element invoked = info.getTrees().getElement(new TreePath(getCurrentPath(), node.getMethodSelect()));
664-
if (invoked != null && invoked.getEnclosingElement() == methodHandlesLookup && node.getArguments().size() > 0) {
668+
if (invoked != null && invoked.getEnclosingElement() == methodHandlesLookup && !node.getArguments().isEmpty()) {
665669
ExpressionTree clazz = node.getArguments().get(0);
666670
Element lookupType = null;
667671
if (clazz.getKind() == Kind.MEMBER_SELECT) {
@@ -678,16 +682,12 @@ public Void visitMethodInvocation(MethodInvocationTree node, Void p) {
678682
}
679683
}
680684
switch (invoked.getSimpleName().toString()) {
681-
case "findStatic": case "findVirtual": case "findSpecial":
685+
case "findStatic", "findVirtual", "findSpecial" ->
682686
type2LookedUpMethods.computeIfAbsent(lookupType, t -> new HashSet<>()).add(lookupName);
683-
break;
684-
case "findConstructor":
687+
case "findConstructor" ->
685688
type2LookedUpMethods.computeIfAbsent(lookupType, t -> new HashSet<>()).add("<init>");
686-
break;
687-
case "findGetter": case "findSetter": case "findStaticGetter":
688-
case "findStaticSetter": case "findStaticVarHandle": case "findVarHandle":
689+
case "findGetter", "findSetter", "findStaticGetter", "findStaticSetter", "findStaticVarHandle", "findVarHandle" ->
689690
type2LookedUpFields.computeIfAbsent(lookupType, t -> new HashSet<>()).add(lookupName);
690-
break;
691691
}
692692
}
693693
return super.visitMethodInvocation(node, p);

‎java/java.editor.base/test/unit/src/org/netbeans/modules/java/editor/base/semantic/UnusedDetectorTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ public void run(CompilationController parameter) {
593593

594594
Set<String> result = UnusedDetector.findUnused(parameter, () -> false)
595595
.stream()
596-
.map(ud -> parameter.getCompilationUnit().getLineMap().getLineNumber(parameter.getTrees().getSourcePositions().getStartPosition(ud.unusedElementPath.getCompilationUnit(), ud.unusedElementPath.getLeaf())) + ":" + ud.unusedElement.getSimpleName() + ":" + ud.reason.name())
596+
.map(ud -> parameter.getCompilationUnit().getLineMap().getLineNumber(parameter.getTrees().getSourcePositions().getStartPosition(ud.unusedElementPath().getCompilationUnit(), ud.unusedElementPath().getLeaf())) + ":" + ud.unusedElement().getSimpleName() + ":" + ud.reason().name())
597597
.collect(Collectors.toSet());
598598
assertEquals(new HashSet<>(Arrays.asList(expected)), result);
599599
} catch (IOException e) {

‎java/java.hints/src/org/netbeans/modules/java/hints/bugs/Unused.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ public static List<ErrorDescription> unused(HintContext ctx) {
6767
if (ctx.isCanceled()) {
6868
break;
6969
}
70-
if (ud.unusedElementPath.getLeaf() != ctx.getPath().getLeaf()) {
70+
if (ud.unusedElementPath().getLeaf() != ctx.getPath().getLeaf()) {
7171
continue;
7272
}
73-
if (!detectUnusedPackagePrivate && ud.packagePrivate) {
73+
if (!detectUnusedPackagePrivate && ud.packagePrivate()) {
7474
continue;
7575
}
7676
ErrorDescription err = convertUnused(ctx, ud);
@@ -98,34 +98,34 @@ public static List<ErrorDescription> unused(HintContext ctx) {
9898
})
9999
private static ErrorDescription convertUnused(HintContext ctx, UnusedDescription ud) {
100100
//TODO: switch expression candidate!
101-
String name = ud.unusedElement.getSimpleName().toString();
101+
String name = ud.unusedElement().getSimpleName().toString();
102102
String message;
103103
Fix fix = null;
104-
switch (ud.reason) {
104+
switch (ud.reason()) {
105105
case NOT_WRITTEN_READ: message = Bundle.ERR_NeitherReadOrWritten(name);
106-
fix = JavaFixUtilities.removeFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath);
106+
fix = JavaFixUtilities.removeFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath());
107107
break;
108108
case NOT_WRITTEN: message = Bundle.ERR_NotWritten(name);
109109
break;
110110
case NOT_READ: message = Bundle.ERR_NotRead(name);
111111
//unclear what can be done with unused binding variables currently (before "_"):
112-
if (ud.unusedElementPath.getParentPath().getLeaf().getKind() != Kind.BINDING_PATTERN) {
113-
fix = JavaFixUtilities.safelyRemoveFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath);
112+
if (ud.unusedElementPath().getParentPath().getLeaf().getKind() != Kind.BINDING_PATTERN) {
113+
fix = JavaFixUtilities.safelyRemoveFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath());
114114
}
115115
break;
116116
case NOT_USED:
117-
if (ud.unusedElement.getKind() == ElementKind.CONSTRUCTOR) {
117+
if (ud.unusedElement().getKind() == ElementKind.CONSTRUCTOR) {
118118
message = Bundle.ERR_NotUsedConstructor();
119-
fix = JavaFixUtilities.removeFromParent(ctx, Bundle.FIX_RemoveUsedConstructor(), ud.unusedElementPath);
119+
fix = JavaFixUtilities.removeFromParent(ctx, Bundle.FIX_RemoveUsedConstructor(), ud.unusedElementPath());
120120
} else {
121121
message = Bundle.ERR_NotUsed(name);
122-
fix = JavaFixUtilities.removeFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath);
122+
fix = JavaFixUtilities.removeFromParent(ctx, Bundle.FIX_RemoveUsedElement(name), ud.unusedElementPath());
123123
}
124124
break;
125125
default:
126-
throw new IllegalStateException("Unknown unused type: " + ud.reason);
126+
throw new IllegalStateException("Unknown unused type: " + ud.reason());
127127
}
128-
return fix != null ? ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath, message, fix)
129-
: ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath, message);
128+
return fix != null ? ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath(), message, fix)
129+
: ErrorDescriptionFactory.forName(ctx, ud.unusedElementPath(), message);
130130
}
131131
}

0 commit comments

Comments
 (0)