Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ASTProvider to getAST for source action handlers #1533

Merged
merged 3 commits into from
Aug 27, 2020

Conversation

testforstephen
Copy link
Contributor

Signed-off-by: Jinbo Wang jinbwan@microsoft.com

ASTProvider will cache the AST node for the active editor, should use it whenever possible.

Below is the profiling comparison on editing a Java file with 200 LOC.

  • Before:
    The source action handlers will parse a new AST every time in the source action handler. Most of time of getSourceActionCommands is on parsing AST node. And the time ratio between getSourceActionCommands and getCodeActionCommands is about 64%.
    image

  • After:
    Use ASTProvider to get AST node, and the time ratio reduces to 40%.
    image

Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
@fbricon
Copy link
Contributor

fbricon commented Aug 26, 2020

test this please

@fbricon
Copy link
Contributor

fbricon commented Aug 26, 2020

There are 2 failures on Jenkins with this PR:
org.eclipse.jdt.ls.core.internal.handlers.GenerateToStringHandlerTest.testGenerateToString_customizedSettings
org.eclipse.jdt.ls.core.internal.handlers.GenerateToStringHandlerTest.testGenerateToString

I can reproduce them on my linux box and my mac

Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
@fbricon fbricon merged commit 45363e4 into eclipse-jdtls:master Aug 27, 2020
@fbricon
Copy link
Contributor

fbricon commented Aug 27, 2020

Thanks @testforstephen!

@fbricon fbricon added this to the End August 2020 milestone Aug 27, 2020
@testforstephen testforstephen deleted the jinbo_astprovider branch August 27, 2020 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants