Hi Alex,
Thanks for the review comments,
Yes, we can use the PainterMultiResolutionCachedImage.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.
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 9th 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. HYPERLINK
"http://cr.openjdk.java.net/%7Earapte/8163261/webrev.02/"http://cr.openjdk.java.net/~arapte/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/
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@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 : HYPERLINK
"http://cr.openjdk.java.net/%7Earapte/8163261/webrev.02/"http://cr.openjdk.java.net/~arapte/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;
HYPERLINK "mailto:swing-dev@openjdk.java.net"swing-dev@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; HYPERLINK
"mailto:swing-dev@openjdk.java.net"swing-dev@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: HYPERLINK
"http://cr.openjdk.java.net/%7Earapte/8163261/webrev.01/"http://cr.openjdk.java.net/~arapte/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; HYPERLINK
"mailto:swing-dev@openjdk.java.net"swing-dev@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: HYPERLINK
"http://cr.openjdk.java.net/%7Earapte/8163261/webrev.00/"http://cr.openjdk.java.net/~arapte/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
