#1002293
- Created on2014-10-10 12:02
- Reported by
- StatusFixed
- PriorityNormal (3)
- Assigned to
- Milestone
- Plan Level-
- Due Date-
Potential memory leaks
I noticed a few times that my IDE capped at ~2G of heap (~15 projects open in the workspace). I took a heap dump and noticed a few old PC hanging around, usually maintained by links from annotations:
- implicit conversion annotations
- override markers
- BrowserInput for scaladoc hovers
Except for the last one, nothing is new here. All of these are long-lived objects, especially in editors that don't get any typing. Most likely the corresponding PCs have been shutdown by the watchdog, but their instance can't be reclaimed because of these links. In this case, the watchdog actually increases memory consumption (at least, if it shuts down a compiler when there are open editors for that project -- I'm using that setting).
The proper fix would be to redesign these markers. They should not hold on to a compiler symbol, but instead keep just the position where they occur. If the user clicks on one of them, it can relatively quickly recompute the symbol based on the position.
- implicit conversion annotations
- override markers
- BrowserInput for scaladoc hovers
Except for the last one, nothing is new here. All of these are long-lived objects, especially in editors that don't get any typing. Most likely the corresponding PCs have been shutdown by the watchdog, but their instance can't be reclaimed because of these links. In this case, the watchdog actually increases memory consumption (at least, if it shuts down a compiler when there are open editors for that project -- I'm using that setting).
The proper fix would be to redesign these markers. They should not hold on to a compiler symbol, but instead keep just the position where they occur. If the user clicks on one of them, it can relatively quickly recompute the symbol based on the position.
Leave a comment
I have to restart Scala-IDE several times at day at work because of these leaks. As Iulian already suggested, one of the worst offenders in this area seems to be
My first step would be removing the lazily computed
Please let me know if you have any further suggestions/remarks/ideas!
ImplicitConversionAnnotation as illustrated in the screenshots from MAT above. The heap dumps referenced in these screenshots can be found here: https://spideroak.com/browse/share/lolukukassus/scala-ide/scala-ide/heap-dumps/.My first step would be removing the lazily computed
sourceLink from ImplicitConversionAnnotation (see https://github.com/scala-ide/scala-ide/blob/ed1d6d3825759f6b4b1b2394a934263e53c48abe/org.scala-ide.sdt.core/src/org/scalaide/ui/internal/editor/decorators/implicits/ImplicitConversionsOrArgsAnnotation.scala#L19) and replace it with an eager value. If this should turn out to cause a performance regression, I could still experiment with a solution that explicitly nulls out the function after is has been used...Please let me know if you have any further suggestions/remarks/ideas!
on 2015-02-27 12:21 *
By Iulian Dragos
I'd definitely check the commit message for that lazy value. I'd assume that was eager before and made lazy for performance reasons. Looking up sources can take a long time, especially if they come from attached sources or other projects. It's exactly the same mechanism as for hyperlinking, and I'm sure we all had moments when we had to wait for a second or so until the target is resolved. We probably don't want that latency added for each implicit in a source file.
I'd suggest instead to store the position where the annotation occurs and make sure no links remain to anything coming from the compiler "cake" (trees, symbols or types) are.
I'd suggest instead to store the position where the annotation occurs and make sure no links remain to anything coming from the compiler "cake" (trees, symbols or types) are.
Thanks for your response... this value was indeed made lazy on purpose in the past - by you ;-). Still, I do not see how we could get rid of the compiler "cake" here (see https://github.com/scala-ide/scala-ide/blob/becda5894cb27806be34b97be56e0592b1c4e09e/org.scala-ide.sdt.core/src/org/scalaide/ui/internal/editor/decorators/implicits/ImplicitHighlightingPresenter.scala#L68)
except for doing things eagerly. Note that the Scaladocs for "mkHyperlink" even state "This is an exit point from the compiler cake." explicitly.
() => compiler.mkHyperlink(t.symbol, name = "Open Implicit", region, scu.scalaProject.javaProject)except for doing things eagerly. Note that the Scaladocs for "mkHyperlink" even state "This is an exit point from the compiler cake." explicitly.
We are leaking
Using MAT and the debugger, I found out that one of the problems is that the reconciler is installed twice for every editor:
Thus I removed the second call here: https://github.com/mlangc/scala-ide/commit/bb17efc4274bd9ab00d54dae498a2fab52139ba1
Note that I'm not yet done with my analysis - I fear that there is much more to come :-/.
ScalaSourceFileEditors. You can easily see this if you follow these steps:- Start the IDE
- Open a random Scala file
- Close the file and all projects
- Acquire a heap dump and open it with MAT
- Open the dominator tree and search for "ScalaSourceFileEditor"
Using MAT and the debugger, I found out that one of the problems is that the reconciler is installed twice for every editor:
- One time in org.eclipse.jface.text.source.SourceViewer.configure(SourceViewerConfiguration)
- A second time in https://github.com/scala-ide/scala-ide/blob/d0d2051472c862dda9f16d6292c18e0d94ccca32/org.scala-ide.sdt.core/src/org/scalaide/ui/internal/editor/ScalaSourceViewerConfiguration.scala#L217
Thus I removed the second call here: https://github.com/mlangc/scala-ide/commit/bb17efc4274bd9ab00d54dae498a2fab52139ba1
Note that I'm not yet done with my analysis - I fear that there is much more to come :-/.
on 2015-03-03 12:05 *
By Iulian Dragos
Great analysis, @mlangc!
Regarding `mkHyperlink`... yes, it is an exit point. But we're not calling that method... we create a thunk that calls that method, so the closure captures and holds on to the compiler. If we made that eager, yes.. it would be outside the cake. But it would be slow.
The only idea I have is to store only the position and redo some of the work to regain the symbol (and then compute the hyperlink) only when the user wants to navigate the hyperlink
Regarding `mkHyperlink`... yes, it is an exit point. But we're not calling that method... we create a thunk that calls that method, so the closure captures and holds on to the compiler. If we made that eager, yes.. it would be outside the cake. But it would be slow.
The only idea I have is to store only the position and redo some of the work to regain the symbol (and then compute the hyperlink) only when the user wants to navigate the hyperlink
Yea, I did some micro-benchmarks that suggest that calling
I've been using https://github.com/mlangc/scala-ide/tree/fix-memory-leaks-1002293 for two days now at work, and my experiences so far are quite good: The IDE still leaks memory, but not as bad as before. It is my intention to keep working on this until I can keep the IDE open for a whole working day with
Last but not least: Don't expect any substantial updates until the end of next week, as I'm about to take a short holiday.
mkHyperlink lazily results in substantial performance improvements; on the other side I did not yet notice any slowdown in practice - but that might have to do with the fact that I'm working on SSDs only.I've been using https://github.com/mlangc/scala-ide/tree/fix-memory-leaks-1002293 for two days now at work, and my experiences so far are quite good: The IDE still leaks memory, but not as bad as before. It is my intention to keep working on this until I can keep the IDE open for a whole working day with
-Xmx1500M. I'm going to look into what to do with mkHyperlink as soon as I'm at this point. Maybe it turns out that holding on to the compiler cake in ImplicitConversionAnnotation is not a problem if these annotations themselves are garbage collected properly.Last but not least: Don't expect any substantial updates until the end of next week, as I'm about to take a short holiday.
on 2015-03-13 23:54 *
By Iulian Dragos
Unfortunately, that's not a good solution. As I mentioned before, that is a potentially very expensive operation, so `mkHyperlink` can't be called during highlighting. It might block the UI, and that's a big user experience no-no. To reiterate, `mkHyperlink` needs to locate sources, load parse and partially type-check the target and locate the definition. That's a lot of work that might need to be done for a lot of implicit applications (a sourcefile may have 10s of such calls, since almost all collections need an implicit).
My advice is to make the whole thing even more lazy, and keep only a position instead of a tree or symbol.
My advice is to make the whole thing even more lazy, and keep only a position instead of a tree or symbol.
Don't leak `ScalaSourceFileEditors`
Calling "install" twice and "uninstall" only once on the reconciler causes listeners not being
removed properly and thereby leaking `ScalaSourceFileEditors`.
See #1002293 for additional information.
Branch: master
Commit: scala-ide:949e47da07
Calling "install" twice and "uninstall" only once on the reconciler causes listeners not being
removed properly and thereby leaking `ScalaSourceFileEditors`.
See #1002293 for additional information.
Branch: master
Commit: scala-ide:949e47da07
on 2016-04-06 11:50 *
By Iulian Dragos
Override Indicators now don’t hold on to compiler symbols.
This should remove one source of memory leaks. Also, only create one
override indicator per definition, towards the closest override.
Fix #1002293
Branch: master
Commit: scala-ide:42287d7f82
This should remove one source of memory leaks. Also, only create one
override indicator per definition, towards the closest override.
Fix #1002293
Branch: master
Commit: scala-ide:42287d7f82