<Swing Dev> Review Request for 8163261: regression on Linux: java/awt/LightweightDispatcher/LWDispatcherMemoryLeakTest.java
Alexandr Scherbatiy
alexandr.scherbatiy at oracle.com
Mon Sep 5 10:09:46 UTC 2016
On 9/2/2016 7:18 AM, Ambarish Rapte wrote:
>
> Hi Alex,
>
> Thanks for the review comments,
>
> Yes, we can use thePainterMultiResolutionCachedImage.class object as a
> key in the PainterMultiResolutionCachedImage.getResolutionVariant()
> method.
>
> But there is below concern,
>
> 1.If PainterMultiResolutionCachedImage.class object is used as key in
> /Map<Object, ImageCache> cacheMap;/
>
> 2.There will be only one key value pair entry in the /cacheMap/.
>
The CachedPainter.paint0() method uses getClass() as a key for the
cacheMap. At least there should be more than 1 key (getClass() +
PainterMultiResolutionCachedImage.class).
> 3.The /ImageCache/ value created for the
> /PainterMultiResolutionCachedImage class/ is only of size 1.
>
> 4.Hence there will be only one image present at a time in the /cacheMap/.
>
> 5.Each time a new image is added, previous one will get deleted.
> In method: /ImageCache::getEntry() => if (entries.size() >=
> maxCount) { entries.removeLast(); }/
>
> 6.This will result repeated deletion and creation of images.
>
> This requires increase in the size of /ImageCache /associated
> with/PainterMultiResolutionCachedImage.class /key to a certain
> justified value.
>
> For example, we can set the value to 8, so that at a time 8 images can
> be stored in the /ImageCache/. So for to add 9^th entry, last image
> would be removed.
>
> I am not sure what this value can be. I think this value can be number
> of components.
>
> SwinSet2 demo creates 17+ such images.
>
> After deciding a good value, this change can be done, a lesser value
> may add to processing by re-creating images.
>
> Currently we have 2 approaches:
>
> Approach1: /PainterMultiResolutionCachedImage /object as key,
> with/hashCode() & equals() /method/.
> /http://cr.openjdk.java.net/~arapte/8163261/webrev.02/
> <http://cr.openjdk.java.net/%7Earapte/8163261/webrev.02/>
>
> Approach2: /PainterMultiResolutionCachedImage.class /object//as key,
> with a good justified size of/ImageCache.
> /http://cr.openjdk.java.net/~arapte/8163261/webrev.03/
> <http://cr.openjdk.java.net/%7Earapte/8163261/webrev.03/>
>
Could the image cache size be increased only for the
PainterMultiResolutionCachedImage.class key in the approach 2 because
other keys may require only one image?
Thanks,
Alexandr.
>
> //
>
> //
>
> BOTH the approach would result in creating same number of creating
> images, in a normal scenario.
>
> however depending of GC/OOM this can change, as we have soft
> references to images.
>
> I observed over a long run of SwingSet2, Approach2 creates less number
> of images, but Approach1 recreates images.
>
> Another difference would be,
>
> Approach1 will add <key, value> pair entries to /Map<Object,
> ImageCache> cacheMap/.
>
> &
>
> Approach2 will add only ONE <key, value> pair entry to /Map<Object,
> ImageCache> cacheMap;/ But add entries to associated /ImageCache/.
>
> I think, approach 2 can be used with a justified size of ImageCache.
> In webrev.03 it is 32.
>
> Please provide, your comments.
>
> Regards,
>
> Ambarish
>
> *From:*Alexandr Scherbatiy
> *Sent:* Monday, August 29, 2016 6:09 PM
> *To:* Ambarish Rapte; Sergey Bylokhov; Rajeev Chamyal;
> swing-dev at openjdk.java.net
> *Subject:* Re: <Swing Dev> Review Request for 8163261: regression on
> Linux: java/awt/LightweightDispatcher/LWDispatcherMemoryLeakTest.java
>
>
> CachedPainter.paint0() uses the object returned by getClass() as a key
> for the cache.
> Is it possible to use PainterMultiResolutionCachedImage.class object
> as a key in the
> PainterMultiResolutionCachedImage.getResolutionVariant() method?
>
> Thanks,
> Alexandr.
>
> On 8/26/2016 1:34 PM, Ambarish Rapte wrote:
>
> Hi Alex & All,
>
> Please review the updated webrev :
> http://cr.openjdk.java.net/~arapte/8163261/webrev.02/
> <http://cr.openjdk.java.net/%7Earapte/8163261/webrev.02/>
>
> Changes: (Copying from the previous email)
>
> I have included the change you suggested, to set component
> reference null after painting the component.
>
> In addition there is change in PainterMultiResolutionCachedImage
> to add /hashCode()/ & /equals()/ methods.
>
> Object of /PainterMultiResolutionCachedImage/ is used as key in
> the /Map<Object, ImageCache> cacheMap./
>
> /hashCode()/ & /equals()/ would avoid multiple entries of similar
> image objects in the /cacheMap/ & help reduce the size of /cacheMap./
>
> //
>
> Verified this change by executing the /SwinSet2/ demo, all
> components get painted correctly.
>
> Also a small change in test, added check on panel reference and
> modified error message.
>
> Regards,
>
> Ambarish
>
> *From:*Ambarish Rapte
> *Sent:* Friday, August 26, 2016 12:34 PM
> *To:* Ambarish Rapte; Alexander Scherbatiy; Sergey Bylokhov;
> Rajeev Chamyal; swing-dev at openjdk.java.net
> <mailto:swing-dev at openjdk.java.net>
> *Subject:* RE: <Swing Dev> Review Request for 8163261: regression
> on Linux:
> java/awt/LightweightDispatcher/LWDispatcherMemoryLeakTest.java
>
> Hi All,
>
> Please hold the review for this.
>
> There are some merge conflicts with latest code so I shall update
> the webrev again for review.
>
> Regards,
>
> Ambarish
>
> *From:*Ambarish Rapte
> *Sent:* Thursday, August 25, 2016 10:19 PM
> *To:* Alexander Scherbatiy; Sergey Bylokhov; Rajeev Chamyal;
> swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
> *Subject:* Re: <Swing Dev> Review Request for 8163261: regression
> on Linux:
> java/awt/LightweightDispatcher/LWDispatcherMemoryLeakTest.java
>
> Hi Alex,
>
> Thanks for the review comments,
>
> Please review the updated webrev:
> http://cr.openjdk.java.net/~arapte/8163261/webrev.01/
> <http://cr.openjdk.java.net/%7Earapte/8163261/webrev.01/>
>
> I have included the change you suggested, to set component
> reference null after painting the component.
>
> In addition there is change in PainterMultiResolutionCachedImage
> to add /hashCode()/ & /equals()/ methods.
>
> Object of /PainterMultiResolutionCachedImage/ is used as key in
> the /Map<Object, ImageCache> cacheMap./
>
> /hashCode()/ & /equals()/ would avoid multiple entries of similar
> image objects in the /cacheMap/ & help reduce the size of /cacheMap./
>
> //
>
> Verified this change by executing the /SwinSet2/ demo, all
> components get painted correctly.
>
> Also a small change in test, added check on panel reference and
> modified error message.
>
> Regards,
>
> Ambarish
>
> *From:*Alexander Scherbatiy
> *Sent:* Monday, August 22, 2016 5:47 PM
> *To:* Ambarish Rapte; Sergey Bylokhov; Rajeev Chamyal;
> swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
> *Subject:* Re: <Swing Dev> Review Request for 8163261: regression
> on Linux:
> java/awt/LightweightDispatcher/LWDispatcherMemoryLeakTest.java
>
>
> On 17/08/16 15:49, Alexandr Scherbatiy wrote:
>
> On 8/15/2016 12:43 PM, Ambarish Rapte wrote:
>
> Hi,
>
> Please review fix for JDK9,
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8163261
>
> Webrev:
> http://cr.openjdk.java.net/~arapte/8163261/webrev.00/
> <http://cr.openjdk.java.net/%7Earapte/8163261/webrev.00/>
>
> Issue:
>
> Reference to JButton was not getting collected by GC.
>
> Cause:
>
> A strong reference to objects was held by
> PainterMultiResolutionCachedImage.
>
> And the image reference was held by HashMap.
>
> Fix:
>
> Changing the HashMap to WeakHashMap. Entry to
> WeakHashMap gets removed after the object has no other strong
> reference.
>
> May be using the soft reference would be better in this case.
> It could be expensive to recreate a cache with images every time
> GC removed them.
>
>
> There is the code which sets a component to the
> PainterMultiResolutionCachedImage:
> CachedPainter.paint0(...)
> -------
> if (image instanceof PainterMultiResolutionCachedImage) {
> ((PainterMultiResolutionCachedImage)
> image).setParams(c, args);
> }
>
> // Render to the passed in Graphics
> paintImage(c, g, x, y, w, h, image, args);
> -------
>
> May be it is possible to clean up the component and args from the
> PainterMultiResolutionCachedImage after the image is drawn?
>
>
> Thanks,
> Alexandr.
>
> Regards,
>
> Ambarish
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160905/a8ce70e1/attachment.html>
More information about the swing-dev
mailing list