Community
Participate
Working Groups
Currently, an image provider registered to a diagram type is not released when the corresponding diagram editor is closed. This is implemented for performance reason since it's expensive to cache the image data. But the downside of this implementation is memories for the images are not released when the editor is closed thus causing memory leaks. Since we cannot predict whether the user is likely to re-open the same editor again or not in the future, we should adhere to the good programming practice of releasing all the memories when a program exits.
Forum discussion can be found at http://www.eclipse.org/forums/index.php?t=tree&th=202971&start=0&S=71fe419cf8a542ec682b3ccd58198a86
Shenxue, i had a look at the issue. The ImageProvider only has a string to string map between image keys and path strings. It is the Graphiti UI Plugin's image registry that handles the images. Indeed they are kept until the SWT Display goes away. But also the JavaDoc of the Image Registry states the following: "Because of this, clients do not need to (indeed, must not attempt to) dispose of these images themselves." It is hard for us to know when to dispose the images centrally. An easy solution would be to offer a method "removeImageFromRegistry" in the Graphiti's ImageService which calls the remove method of ImageRegistry to dispose the image. This method can be invoked by clients of the framework when they know that their image is not in use anymore to free space. What do you think? Would this suffice to solve your use-case? Otherwise we would have to change the whole design and affect existing customers and cause API changes (have an image provider per editor and not centrally etc) Best, Tim
..also we should think about the drawback of having one image registry per editor: if we open several editors of the same type we allocate the same image several times.... Clearly, an advantage would be that all images are gone if all editors are closed but this can be an disadvantage, too: you cannot use the images somewhere else in the UI, since their lifecycle would be tied to the lifecycle of the editor.
(In reply to comment #2) > Shenxue, > > i had a look at the issue. The ImageProvider only has a string to string > map between image keys and path strings. It is the Graphiti UI Plugin's > image registry that handles the images. Indeed they are kept until the SWT > Display goes away. But also the JavaDoc of the Image Registry states the > following: > "Because of this, clients do not > need to (indeed, must not attempt to) dispose of these images themselves." > > It is hard for us to know when to dispose the images centrally. > > An easy solution would be to offer a method "removeImageFromRegistry" in the > Graphiti's ImageService > which calls the remove method of ImageRegistry to dispose the image. > This method can be invoked by clients of the framework when they know that > their image is not in use anymore to free space. > > What do you think? Would this suffice to solve your use-case? > > Otherwise we would have to change the whole design and affect existing > customers and cause API changes (have an image provider per editor and not > centrally etc) > > Best, Tim Yes adding removeImageFromRegistry method to the ImageService would solve my problem. When I dispose my editor, I can call that. Thanks!
Created attachment 198373 [details] mylyn/context/zip
I checked in the mentioned method. Can you verify that it works as desired?
Created attachment 198375 [details] mylyn/context/zip
Tim, After spending some time with Graphiti's ImageService and ImageProvider, it seemed to me there is one more missing piece in removing image caches when the diagram editor is closed. AbstractImageProvider class doesn't provide an API to remove image file paths from its own cache. When diagram editor is closed, the image file path caches in AbstractImageProvider aren't cleared either. Could you provider an API in AbstractImageProvider to remove image file path cache? That method can do two things: clear the image file path cache and call removeImageFromRegistry() on ImageService to remove the image data cache. Does that make sense? Thanks, Shenxue
Shenxue, i also had a look at that. But the current design does not allow us to remove the image file paths. Observe that the image file paths are initialized in the constructor. If we remove them, they will never come back ;-) We would have to change the whole image provider setup (API changes). Do you see severe consequences if we stick to the current setup? Best, Tim
(In reply to comment #9) > Shenxue, > > i also had a look at that. But the current design does not allow us > to remove the image file paths. Observe that the image file paths > are initialized in the constructor. If we remove them, they will > never come back ;-) > > We would have to change the whole image provider setup (API changes). Do you > see severe consequences if we stick to the current setup? > > Best, Tim I can work around this problem by checking whether the image file path has been registered before I add image file path to the cache. Thanks for looking into this.
Marked as part of Juno
Bookkeeping: Set target release
Part of Graphiti 0.9.0 (Eclipse Juno)
