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

Performance issue on large project #1479

Merged
merged 1 commit into from
Jun 16, 2020
Merged

Conversation

snjeza
Copy link
Contributor

@snjeza snjeza commented Jun 12, 2020

Fixes #1469

Signed-off-by: Snjezana Peco snjezana.peco@redhat.com

@fbricon
Copy link
Contributor

fbricon commented Jun 13, 2020

Can you please give some details about this change? Why? How? How much improvement is expected?

@snjeza
Copy link
Contributor Author

snjeza commented Jun 13, 2020

Can you please give some details about this change? Why? How? How much improvement is expected?

Java LS locks whole workspace when opening, closing changing, validating a file.
The PR locks only used file(s).
We also avoid the workspace update when handling the open/close/change operations.

@fbricon
Copy link
Contributor

fbricon commented Jun 14, 2020

I tried loading the guava project with and without this fix, but wasn't able to spot any significant difference when invoking completion (as per #1469 (comment)).
The changes look pretty sane to me, but I'd really love to see a scenario which shows a positive impact of this change. @testforstephen @jdneo @Eskibear can you give it a try?

@snjeza
Copy link
Contributor Author

snjeza commented Jun 14, 2020

Additional features:

  • stop hover, codeAction and findLinks when it is cancelled.
  • don't reconcile a compilation unit within BaseDocumentLifeCycleHandler.performValidation(IProgressMonitor). It is reconciled later within publishDiagnostics(IProgressMonitor) - 2da94de#diff-49d07c54e1e695e6884afffabd2b7fafR181

@testforstephen
Copy link
Contributor

I tried the fix on spring-boot project (5k+ files, 100+ gradle projects), i also didn't observe an obvious improvement. It's still a little lagging on completion, especially becomes more bad when opening a larger file (e.g. 2k+ LOC). Seems worthwhile to try lazy resolving addtionalTextEdits.

@Eskibear has more experience on completion performance analysis, could you have a look at the fix?

@Eskibear
Copy link
Contributor

Eskibear commented Jun 15, 2020

I just tried guava which contains 20K+ java files, on my mac with 2.9G Dual-Core i5 + 8G RAM. No suspicious extension enabled.

Either with or without this PR, I don't see significant performance issue. Editing CharMatcher.java metioned by the issue owner, time cost for textDocument/completion was 500ms ~ 1500ms in both cases, obviously lagging but somehow usable for me.

Then I broke down the time cost in jProfiler, finding unit.codeComplete and getCompletionItems are costing most time, and the ratio is about 1:3, meaning that calculating textEdits for import statements is a big rock.

It's as expected and matches our previous investigation. And for the moment, I think that lazy resolving addtionalTextEdit should be a promising way to improve the performance.

@snjeza
Copy link
Contributor Author

snjeza commented Jun 15, 2020

Either with or without this PR, I don't see significant performance issue. Editing CharMatcher.java metioned by the issue owner, time cost for textDocument/completion was 500ms ~ 1500ms in both cases, obviously lagging but somehow usable for me.

The PR doesn't change textDocument/completion than codeAction, triggerValidation, hover, findLinks... The PR should enhance overall VS Code performance.

@snjeza snjeza changed the title [WIP] Performance issue on large project Performance issue on large project Jun 16, 2020
Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
@fbricon fbricon merged commit 9519b76 into eclipse-jdtls:master Jun 16, 2020
@fbricon
Copy link
Contributor

fbricon commented Jun 16, 2020

Thanks @snjeza!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance issue on large project
4 participants