search

openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.21k stars 329 forks source link

Performance issue with JavaParser #2183

Closed BoykoAlex closed 11 months ago

BoykoAlex commented 2 years ago

Parsing a very large Java file (~160K lines of code) takes much longer than parsing the same file with JDT java parser.

The java file that takes long is: PlSqlParser.java from the attached project.

The issues originated here: https://github.com/spring-projects/sts4/issues/825

The code seems to be stuck mostly in:

ForkJoinPool.commonPool-worker-10  Runnable CPU usage on sample: 1s 1ms
  java.util.regex.Pattern$Start.match(Pattern.java:3608)
  java.util.regex.Matcher.search(Matcher.java:1728)
  java.util.regex.Matcher.find(Matcher.java:772)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.lambda$visitVariables$28(ReloadableJava17ParserVisitor.java:1387)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor$$Lambda$1028.0x000000080123ec18.get()
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitVariables(ReloadableJava17ParserVisitor.java:1394)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitVariable(ReloadableJava17ParserVisitor.java:1342)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitVariable(ReloadableJava17ParserVisitor.java:71)
  com.sun.tools.javac.tree.JCTree$JCVariableDecl.accept(JCTree.java:1045)
  com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:86)
  org.openrewrite.java.iso
[plsql.zip](https://github.com/openrewrite/rewrite/files/9479517/plsql.zip)
lated.ReloadableJava17ParserVisitor.convert(ReloadableJava17ParserVisitor.java:1478)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convert(ReloadableJava17ParserVisitor.java:1507)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convertAll(ReloadableJava17ParserVisitor.java:1544)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitMethod(ReloadableJava17ParserVisitor.java:950)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitMethod(ReloadableJava17ParserVisitor.java:71)
  com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:953)
  com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:86)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convert(ReloadableJava17ParserVisitor.java:1478)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convert(ReloadableJava17ParserVisitor.java:1507)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convertStatements(ReloadableJava17ParserVisitor.java:1616)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convertStatements(ReloadableJava17ParserVisitor.java:1598)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitClass(ReloadableJava17ParserVisitor.java:472)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitClass(ReloadableJava17ParserVisitor.java:71)
  com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:860)
  com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:86)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convert(ReloadableJava17ParserVisitor.java:1478)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convert(ReloadableJava17ParserVisitor.java:1507)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convertStatements(ReloadableJava17ParserVisitor.java:1616)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convertStatements(ReloadableJava17ParserVisitor.java:1598)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitClass(ReloadableJava17ParserVisitor.java:472)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitClass(ReloadableJava17ParserVisitor.java:71)
  com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:860)
  com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:86)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convert(ReloadableJava17ParserVisitor.java:1478)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.convertAll(ReloadableJava17ParserVisitor.java:1531)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitCompilationUnit(ReloadableJava17ParserVisitor.java:518)
  org.openrewrite.java.isolated.ReloadableJava17ParserVisitor.visitCompilationUnit(ReloadableJava17ParserVisitor.java:71)
  com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:614)
  com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:86)
  org.openrewrite.java.isolated.ReloadableJava17Parser.lambda$parseInputs$0(ReloadableJava17Parser.java:177)
  org.openrewrite.java.isolated.ReloadableJava17Parser$$Lambda$906.0x00000008011ef328.apply()
  java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
  java.util.Iterator.forEachRemaining(Iterator.java:133)
  java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1845)
  java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
  java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
  java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
  java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
  java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
  org.openrewrite.java.isolated.ReloadableJava17Parser.parseInputs(ReloadableJava17Parser.java:199)
  org.openrewrite.java.Java17Parser.parseInputs(Java17Parser.java:43)
  org.springframework.ide.vscode.commons.rewrite.java.ORAstUtils.parseInputs(ORAstUtils.java:233)
  org.springframework.ide.vscode.boot.java.rewrite.RewriteCompilationUnitCache.doParse(RewriteCompilationUnitCache.java:277)
  org.springframework.ide.vscode.boot.java.rewrite.RewriteCompilationUnitCache.lambda$7(RewriteCompilationUnitCache.java:234)
  org.springframework.ide.vscode.boot.java.rewrite.RewriteCompilationUnitCache$$Lambda$632.0x00000008010a4350.get()
  java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1768)
  java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1760)
  java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
  java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
  java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
  java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
  java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)
BoykoAlex commented 2 years ago

CC: @martinlippert

martinlippert commented 2 years ago

For some background: I experimented with the IDE support that we are working on to make sure we can deal with the various situations. From my experience, this parsing performance is quite important for the interactive experience in the IDE, so would be good to address this topic somehow. The large file seems a bit unrealistic for real use (for sure), but it helped me quite a bit to uncover those bottlenecks that also slow down the parsing experience for smaller files (but obviously on a smaller scale).

And my assumption is that improving this would also help making every other use case faster as well... :-)

BoykoAlex commented 2 years ago

The project having PlSqlParser.java - the java source with 160K lines of code plsql.zip

jkschneider commented 2 years ago

Thanks for the pointer @BoykoAlex . I think replacing this regex with iteration is totally feasible.

martinlippert commented 2 years ago

@jkschneider Another interesting thread dump that causes project reconciling to go a bit crazy on our end: https://github.com/spring-projects/sts4/issues/825#issuecomment-1259086573

timtebeek commented 11 months ago

@BoykoAlex Are you still seeing these performance issues? It's been a while with quite some changes to the parser since; two of them linked above. I'm looking to prune the backlog a bit, so trying to decide if we should close or revisit.

martinlippert commented 11 months ago

@timtebeek We moved away from using OpenRewrite for the performance critical parsing pieces (reconciling Java sources), so the parser performance is not a critical part for us anymore.

timtebeek commented 11 months ago

Good to know! Then I hope you don't mind me closing this one, given the large amount of changes since, and how you are now unlikely to notice even if there are further improvements.

martinlippert commented 11 months ago

@timtebeek But nevertheless always interested to hear about the latest improvements in this area!!! :-)

martinlippert commented 11 months ago

@timtebeek Oh, and not to forget that we continue to use OpenRewrite not only when running upgrade recipes from within the IDE, but also the quick fixes, so improvements in this space are still highly welcome and totally awesome, so please keep us in the loop here.

timtebeek commented 11 months ago

Will keep you posted! It's usually Knut who makes performance improvements everywhere he looks; it's just that we only have one Knut. :)