lahodaj opened a new pull request, #7707:
URL: https://github.com/apache/netbeans/pull/7707

   @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:
   ```
   public class NavigatorTest {
       public void test() {
           new NavigatorTest() {
           };
       }
   }
   ```
   
   the `LspElementUtils` will crash with `StackOverflowError`, the relevant 
part of the stacktrace is this:
   ```
        at 
org.netbeans.modules.java.source.ui.LspElementUtils.getAnonymousInnerClasses(LspElementUtils.java:398)
        at 
org.netbeans.modules.java.source.ui.LspElementUtils.element2StructureElement(LspElementUtils.java:119)
        at 
org.netbeans.modules.java.source.ui.LspElementUtils.element2StructureElement(LspElementUtils.java:111)
        at 
org.netbeans.modules.java.source.ui.LspElementUtils.element2StructureElement(LspElementUtils.java:129)
        at 
org.netbeans.modules.java.source.ui.LspElementUtils$1.visitNewClass(LspElementUtils.java:375)
        at 
org.netbeans.modules.java.source.ui.LspElementUtils$1.visitNewClass(LspElementUtils.java:365)
        at com.sun.tools.javac.tree.JCTree$JCNewClass.accept(JCTree.java:1934)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:92)
        at 
com.sun.source.util.TreeScanner.visitExpressionStatement(TreeScanner.java:504)
        at 
com.sun.tools.javac.tree.JCTree$JCExpressionStatement.accept(JCTree.java:1652)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:92)
        at com.sun.source.util.TreeScanner.scan(TreeScanner.java:110)
        at com.sun.source.util.TreeScanner.visitBlock(TreeScanner.java:271)
        at com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1145)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:92)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:95)
        at com.sun.source.util.TreeScanner.visitMethod(TreeScanner.java:223)
        at com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:989)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:66)
        at 
org.netbeans.modules.java.source.ui.LspElementUtils.getAnonymousInnerClasses(LspElementUtils.java:398)
   ```
   
   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. And `LspElementUtils` 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 for `Element`s, and does 
so using `ElementOpenAccessor.getInstance().getOpenInfo(ci.getClasspathInfo(), 
h, new AtomicBoolean());` - and this method, again, is not particularly fast. 
And for `Element`s that originate in the current compilation unit, this method 
is unnecessarily heavyweight. The proposal is to use a shortcut for `Element`s 
originating in the current compilation unit, bypassing search for the source 
file and the `Element` in that source file (as we know it is the current file 
anyway).
   
   Before this patch, the `documentSymbol` crashed for `Attr.java`, and took 
consistently over 1.5s on my (fairly fast) laptop for `JCTree` (which is full 
of declarations, for `getOpenInfo` was called a lot. With this patch, 
`documentSymbol` usually (although not always) takes under 100ms, sometimes 
even considerably faster.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to