Fixing LspElementUtils with anonymous classes extending an enclosing class, and speeding up the StructureElement computation. #7707
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@jtulach reported the code completion inside VS Code is sometimes slow for him. I was thinking of that, and experimenting, and I think at least part of the problem is that tasks that should be background tasks are not properly cancellable in the VS Code (Java) server.
But, while experimenting with sources like:
https://github.com/openjdk/jdk/blob/master/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
and:
https://github.com/openjdk/jdk/blob/master/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java
I've realized there are significant problems with the
documentSymbol
computation. Namely:a) for code like:
the
LspElementUtils
will crash withStackOverflowError
, the relevant part of the stacktrace is this:The issues is that while computing
getAnonymousInnerClasses
, the code will not look at the anonymous class, but at its super class, looking for the super class' members. If this superclass is the enclosing class, it will get back to the same anonymous class. It is not clear to me why the code does what it does - the proposal in this patch is to simply look at the real members of the anonymous class, and show those in the structure. I may be missing something, but I don't see an end-user reason to look at the super-class members.b) the code calls
Trees.getPath(Element)
a lot. While this method is not terribly slow, it is not fast either - it will scan the appropriate AST looking for the declaration of the given Element. AndLspElementUtils
is calling this method a lot, which slows down the process. It may be possible to speed up the method, but this patch proposes to 1) avoid calling the method if we already have the answer somewhere else; 2) for the typical case of getting enclosed element for a type, use an optimized implementation that will simply find the correct member's AST node directly.c)
LspElementUtils
need to compute position info forElement
s, and does so usingElementOpenAccessor.getInstance().getOpenInfo(ci.getClasspathInfo(), h, new AtomicBoolean());
- and this method, again, is not particularly fast. And forElement
s that originate in the current compilation unit, this method is unnecessarily heavyweight. The proposal is to use a shortcut forElement
s originating in the current compilation unit, bypassing search for the source file and theElement
in that source file (as we know it is the current file anyway).Before this patch, the
documentSymbol
crashed forAttr.java
, and took consistently over 1.5s on my (fairly fast) laptop forJCTree
(which is full of declarations, forgetOpenInfo
was called a lot. With this patch,documentSymbol
usually (although not always) takes under 100ms, sometimes even considerably faster.