Community
Participate
Working Groups
Right now the way we handle colors is inefficient and leaky. Need to manage them better -- possibly a gradient solution and be sure to dispose of everything when finished.
Just wanted to mention that the current color management is not actually leaky per se. The color management scheme uses the same strategy as GMF Color Registry. There is an upper limit to the number of colors that will be created appropriate for the range of colors in the diagram itself. Colors are *not* created anew if one already exists. The FigureManager The question is whether this upper limit is too high, and if so whether colors should be disposed upon editor closing. My feeling is no, as these are likely to be reused. We I am going to do some re-factoring to separate out the color management aspect from the figure handler, but I'm not planning to make any other change as of yet. Instead, I'm thinking of putting a monitor for the number of colors in place. Then we can observe the registry in actually usage and make sure the number of colors doesn't get out of control. I've used a very similar approach in a number of other circumstances and haven't had any issues.
Resolving. I've set the monitor limit to 1,000 and we'll see how we do with it. My guess is that it will be a little low. It might be nice to share the registry with SWT and/or GMF though on the other hand it might be better to isolate it in the case where the plugin is unloaded. Note that worst case is quite bad -- we could theoretically get ~256 * n colors, where n is the number of unique colors that the user has employed for edges and it could be even worse in the case where the background is different colors but in practice I just don't see that happening. For gray scale the upper limit is obviously 256 but in observation seems around 140 or so. We need these colors to get the nice fade effect. We'll keep an eye on it and see if it has the potential to create real world issues.
FigureManager.colorCache appears to be unused now. Please remove the modal warning dialog in GradiatedColorRegistry and log a message instead. The current implementation is still problematic since colors are never released even when all editors are closed. Is it feasible to add a hook that disposes no longer used colors on editor close?
Reopening to discusson comment#3.
(In reply to comment #3) > The current implementation is still problematic since colors are never released > even when all editors are closed. Is it feasible to add a hook that disposes no > longer used colors on editor close? I was thinking about that...it's not something that GMF does, though it arguably should as well. I can think of arguments in both directions, here are a couple against it: 1. In the common case where users are editing diagrams with gray scale components, you're going to have a lot of allocations and disposals as editors are opened and closed. 2. It's going to create more coupling. The registry would have to be aware of every potential usage of it, but clients are free to use it in whatever context they want. So if we were going to do it right, we'd have to come up with some kind of mechanism where the only way to get access to the color registry is to register yourself as a view or editor first. Then we have to disallow that on removal, etc.. 3. So it's a double edged sword, because if you screw something up in the API, or if a view or editor is removed and there is still some path to the figures and colors somehow (which might happen given the complexities of some GMF implementations), then you have the potential to create duplicate colors. The advantage of the current setup is that you could have a lot of colors but no matter what you don't have to worry about having multiples, i.e. a true leak. I did some very informal testing and it looked like a diagram with a few colors added maybe a couple of MBs, difficult to say given gc, etc.. should really do a memory probe I guess. Does anyone have any idea about what the memory and system resources are for each color allocation across platforms? My bias is to say that as long as there isn't an actual leak, and you're not trying to actually create 256^3 colors :) that modern systems should be able to handle this easily, but that may of course be wrong. One thought I just had was to treat all of the "ghosting" entities as gray-scale. I'm not sure that that would improve or detract from the interface, but I'd like to wait and see how this works with different diagram types and what the memory costs are before making a decision like that. One side note... I noticed that GMF doesn't unregister the colors at all, but after a fair amount of machination, the SWT color registry does on display dispose. What's the point of that, I wonder? Is it just being really anal, or is there some concern that Eclipse itself won't release system resources on exit? I guess if one were using multiple displays this might come into play in some uncommon scenarios.
The handling of resources is system specific. I don't have a good sense of common thresholds but the number of handles is limited and we have seen numerous problems resulting out of this. The Eclipse forms UI is a good example how to handle that efficiently. If you use a shared FormColors instance gradients the registry uses reference counting to dispose resources when they are no longer needed. Do we have enough information about the life-cycle of the elements that we set the colors on to implement a similar solution? I'm not saying that we need to do this now but I would at least like to have a bug open with some background information.
(In reply to comment #6) > The handling of resources is system specific. I don't have a good sense of > common thresholds but the number of handles is limited and we have seen numerous > problems resulting out of this. The Eclipse forms UI is a good example how to > handle that efficiently. If you use a shared FormColors instance gradients the > registry uses reference counting to dispose resources when they are no longer > needed. Thanks for the reference. It looks like in a lot of cases they just create a new one for each usage, which might be a simpler approach. > > Do we have enough information about the life-cycle of the elements that we set > the colors on to implement a similar solution? Yes, I think we do. They should share life-cycle with their host IWorkbenchPart. The wrinkle is that the colors will be the same or similar across many editors, but again it might be an ok strategy to still just keep them as direct members of the workbench part and dispose of them there. That would of course be inline with the typical approach. > I'm not saying that we need to do > this now but I would at least like to have a bug open with some background > information. Agreed. I don't think we should close this one until we're pretty certain that it's not going to be problematic. And I hear you that we don't want to be leaving resources open if at all possible after the user has closed all of the relevant tools. Another option would be to reference count the editors as a whole and simply clear the cache when all relevant editors are removed. The problem with these more global approaches is that we would not be able to determine which editors are relevant without dealing with the UIBridges i some way. Just to say it's not trivial..
In reviewing other bugs, I realized something that will impact this discussion. The gradiated colors are used for certain edge features only, not nodes. For nodes we use an alpha technique that unfortunately didn't work in those special edge (pun intended) cases. So that cuts down the potential number of colors quite a bit. Also note that in almost all cases the background color will be white, which again dramatically cuts down the number of cases.
OK, I'm going to leave this open, but move it out from under the initial release parade.
Mylyn has been restructured, and our issue tracking has moved to GitHub [1]. We are closing ~14K Bugzilla issues to give the new team a fresh start. If you feel that this issue is still relevant, please create a new one on GitHub. [1] https://github.com/orgs/eclipse-mylyn
