OutOfMemoryError when opening eclipse.jdt.ls with Javac bits #755
Comments
|
I've appended a couple of commits that might significantly improve performance (saving some useless processing here and there, and also improving synchronization to not have same work repeated uselessly across multiple threads). Can you please give it a new try and report whether those have led to improvements regarding this issue? |
|
sure, will try and let's see if it happens again. |
|
Still observe OOM when refreshing the test explorer. I have two questions regarding the implementation:
|
We do not need to cancel unless the content of the file or the project configuration has changed.
There is no need to reparse the AST when moving the cursor. Can't it be cached so it's just reused?
When parsing only (without JavacBindingResolver being used), we should be able to just drop the whole Java context before returning the DOM. I think it should already be happening for this case as IIRC there is no reference left from AST to Context. |
Here are 3 suspect leaks.
For problem 1, the language server could respond client operation in parallel and parsing AST in parallel, so that caching lots of Javac models in memory. Since we will create a new Context for each AST parse operation, that seems to rebuild the type system (Javac models) again and cause more memory than ECJ. Another project I got the OOM is https://github.com/eclipse-sirius/sirius-web, this project is a multi-module maven projects and contain many dependencies as well. Caching the Javac type models seems memory expensive for such complex project. Maybe it's worth a look into how to reuse the Javac type models for the AST parsing job in the same project. |
|
Would it help to change CodeActionHandler to synchronize public static CompilationUnit getASTRoot(ICompilationUnit unit, IProgressMonitor monitor) {
if (unit == null) {
return null;
}
synchronized (unit) {
return CoreASTProvider.getInstance().getAST(unit, CoreASTProvider.WAIT_YES, monitor);
}
}? That should prevent from loading multiple times the AST for the same unit. |
Now that I think of it, I think there is an issue I'm aware of related to this. The shared ast mechanism ( So with JDT-LS (and likely Eclipse), if you open a file, type a few things (reconciling triggers caching), switch to another file and only move the cursor/trigger quick-fix/assist/code actions (no editing/saving), then the source file in the active editor is re-parsed every single time to get the AST. The previous file's AST remains cached. For ECJ, parsing is a pretty fast operation, and @datho7561 mentioned he had no reason to believe it'd be slow even for javac, but if you're re-parsing the AST every time and using each copy that might cause memory issues if the references aren't properly disposed. |
|
It seems like "com.microsoft.java.test.plugin.util.TestSearchUtils.parseToAst(TestSearchUtils.java:639)" is involved in this task. It might be that object retaining more ASTs than necessary. I think you should be able to avoid creating an AST here: it's somehow already present; you can either better reuse existing AST, or use the Java IType which should contain the necessary information (methods and annotations). |
It uses The question is why retaining more ASTs is so expensive when comparing Javac with ECJ. Since there is an ecosystem building tools based on JDT language server, it's difficult to expect downstream projects to optimize for this issue. If we can find a way to address this at the infrastructure level, it would be far more beneficial. |
|
Would you be able to debug with Javac vs with ECJ and inspect the number of AST instances that are still referenced in both cases? |
|
I have various ideas of potential causes. However, I still didn't manage to reproduce the issue in Eclipse IDE. Do you think you could write a test case for JDT-LS that reproduces the OOM programatically? |
|
A quick question: What size of project do you try to reproduce the issue? |
|
And one thing I forgot to mention is that vscode-java adds |
|
I have multiple modules of JDT Core, JDT LS, m2e, LSP4E, sprint-petclinic
open together in my workspace. The main difference seems to be that in
Eclipse IDE with JDT-UI, we don't extensively use codeminings and don't use
the CodeLensHandler at all.
I sometimes put a breakpoint on creation of new AST, and then ask the
debugger for "instance count", but even after extensive usage; I don't see
this number explode, it remains usually about 2 * number of open files, and
when I close files, the "instance count" decreases.
|
|
I cannot reproduce this issue. I installed latest https://github.com/fbricon/vscode-java/releases/tag/javac-prototype in my local VSCode, then I open sirius-web project, open a random Java file ( CodingRulesTests.java ), verify I get expected edition features, then open the Testing perspective to get the test explorer and I didn't face any OOM. |
|
I'd say it's more stable than the test I ran 3 weeks ago, and the issue is now more difficult to reproduce than before. However, I still can reproduce it under stress testing. Typically, when an Out of Memory (OOM) error occurs, the language server appears to be busy and becomes unresponsive. To mock this, I open a bunch of Java files in editor, and keep clicking code navigation in different files without waiting for the previous operation to complete before starting the next one. Meanwhile, I perform other tasks like refreshing the Test Explorer view. Once the language server gets overwhelmed, it shows a spinning Java status icon with the message The sample project I used for testing is https://github.com/mgarin/weblaf. Before I test it, I will wait for the project is fully imported into VS Code (You will see a Here is the heap dump. (The mock test starts at 2:10 pm, and OOM occurs at about 2:20 pm) |
|
In #847 , I tried to dispose the filemanager upon analysis, but this doesn't work because navigating the symbols sometimes need a working filemanager. |
|
I have implemented a custom file manager that is capable of caching the jars so they get loaded only once: #885 @testforstephen Can you please give it a fresh try to vscode-java and report whether it improves things? |
@mickaelistria You can try the following:
#909 improves it a little bit. |
|
Does this still occur? Various fixes took place to improve reuse of jar files and could have fixed it. |
The heap dump file is large, cannot share it directly. Here is the suspect report from Eclipse Memory Analyzer.
OutOfMemory.mp4
The text was updated successfully, but these errors were encountered: