Re: [OpenJDK 2D-Dev] RFR: 8257500: Drawing MultiResolutionImage with ImageObserver "leaks" memory

2021-02-26 Thread Sergey Bylokhov
On Fri, 26 Feb 2021 11:58:48 GMT, Alexey Ivanov  wrote:

> Can `MultiResolutionToolkitImageTest.java` not be run on other platforms?

The MultiResolutionToolkitImage is responsible to support the "@2x.png" files 
extension to make an image from a few image files, and currently this only 
supported on macOS.

-

PR: https://git.openjdk.java.net/jdk/pull/2711


Re: [OpenJDK 2D-Dev] RFR: 8257500: Drawing MultiResolutionImage with ImageObserver "leaks" memory

2021-02-26 Thread Alexey Ivanov
On Wed, 24 Feb 2021 18:57:25 GMT, Sergey Bylokhov  wrote:

> This bug was reported as a leak of the ImageObserver when the user draws a 
> multiresolution image.
> 
> The multiresolution image is an image that contains a few images inside, when 
> the app uses the observer to get notifications about image loading we create 
> a special internal observer and use it to track loading resolution-variants. 
> This internal observer forwards notification from the resolution variant to 
> the application observer.
> 
> The mapping from the application observer to the internal observer is done 
> via "SoftCache" which is a kind of HashMap that uses soft references to the 
> key and value.
> 
> Here the bug comes, the soft references are cleared only under memory 
> pressure, and unused objects may sit hours in memory before being cleaned. 
> Moreover, the internal observer was implemented using a strong reference to 
> the application observer(it is not obvious since the lambda is used). So the 
> key object refers to the application's observer cannot be clear fast. This 
> causes an even longer delay of the memory cleanup, which was considered by 
> the use as a "leak".
> 
> The fix changes the usage of SoftCache to the WeakHashMap, so the key(the 
> application observer) will be cleared when the application lost the reference 
> to it.

Can `MultiResolutionToolkitImageTest.java` not be run on other platforms?

-

Marked as reviewed by aivanov (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2711


Re: [OpenJDK 2D-Dev] RFR: 8257500: Drawing MultiResolutionImage with ImageObserver "leaks" memory

2021-02-25 Thread Alexander Zvegintsev
On Wed, 24 Feb 2021 18:57:25 GMT, Sergey Bylokhov  wrote:

> This bug was reported as a leak of the ImageObserver when the user draws a 
> multiresolution image.
> 
> The multiresolution image is an image that contains a few images inside, when 
> the app uses the observer to get notifications about image loading we create 
> a special internal observer and use it to track loading resolution-variants. 
> This internal observer forwards notification from the resolution variant to 
> the application observer.
> 
> The mapping from the application observer to the internal observer is done 
> via "SoftCache" which is a kind of HashMap that uses soft references to the 
> key and value.
> 
> Here the bug comes, the soft references are cleared only under memory 
> pressure, and unused objects may sit hours in memory before being cleaned. 
> Moreover, the internal observer was implemented using a strong reference to 
> the application observer(it is not obvious since the lambda is used). So the 
> key object refers to the application's observer cannot be clear fast. This 
> causes an even longer delay of the memory cleanup, which was considered by 
> the use as a "leak".
> 
> The fix changes the usage of SoftCache to the WeakHashMap, so the key(the 
> application observer) will be cleared when the application lost the reference 
> to it.

Marked as reviewed by azvegint (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/2711


Re: [OpenJDK 2D-Dev] RFR: 8257500: Drawing MultiResolutionImage with ImageObserver "leaks" memory

2021-02-25 Thread Sergey Bylokhov
On Fri, 26 Feb 2021 00:52:51 GMT, Alexander Zvegintsev  
wrote:

>> This bug was reported as a leak of the ImageObserver when the user draws a 
>> multiresolution image.
>> 
>> The multiresolution image is an image that contains a few images inside, 
>> when the app uses the observer to get notifications about image loading we 
>> create a special internal observer and use it to track loading 
>> resolution-variants. This internal observer forwards notification from the 
>> resolution variant to the application observer.
>> 
>> The mapping from the application observer to the internal observer is done 
>> via "SoftCache" which is a kind of HashMap that uses soft references to the 
>> key and value.
>> 
>> Here the bug comes, the soft references are cleared only under memory 
>> pressure, and unused objects may sit hours in memory before being cleaned. 
>> Moreover, the internal observer was implemented using a strong reference to 
>> the application observer(it is not obvious since the lambda is used). So the 
>> key object refers to the application's observer cannot be clear fast. This 
>> causes an even longer delay of the memory cleanup, which was considered by 
>> the use as a "leak".
>> 
>> The fix changes the usage of SoftCache to the WeakHashMap, so the key(the 
>> application observer) will be cleared when the application lost the 
>> reference to it.
>
> src/java.desktop/share/classes/sun/awt/image/MultiResolutionToolkitImage.java 
> line 130:
> 
>> 128: 
>> 129: if (observer == null || image == null) {
>> 130: return false;
> 
> How about a case when `ObserverCache`'s class instance is not collected by 
> GC, but its `observerRef` or `imageRef`  is collected. Is it possible?
> If so this class instance will permanently return false on `imageUpdate()` 
> call.

observerRef points to the same object as a key in this WeakHashMap, so if 
observerRef will point to null means that the key is already collected->at some 
point(when any of the methods of this collection will be called) the value of 
this map(the ObserverCache object) will be collected as well.

-

PR: https://git.openjdk.java.net/jdk/pull/2711


Re: [OpenJDK 2D-Dev] RFR: 8257500: Drawing MultiResolutionImage with ImageObserver "leaks" memory

2021-02-25 Thread Alexander Zvegintsev
On Wed, 24 Feb 2021 18:57:25 GMT, Sergey Bylokhov  wrote:

> This bug was reported as a leak of the ImageObserver when the user draws a 
> multiresolution image.
> 
> The multiresolution image is an image that contains a few images inside, when 
> the app uses the observer to get notifications about image loading we create 
> a special internal observer and use it to track loading resolution-variants. 
> This internal observer forwards notification from the resolution variant to 
> the application observer.
> 
> The mapping from the application observer to the internal observer is done 
> via "SoftCache" which is a kind of HashMap that uses soft references to the 
> key and value.
> 
> Here the bug comes, the soft references are cleared only under memory 
> pressure, and unused objects may sit hours in memory before being cleaned. 
> Moreover, the internal observer was implemented using a strong reference to 
> the application observer(it is not obvious since the lambda is used). So the 
> key object refers to the application's observer cannot be clear fast. This 
> causes an even longer delay of the memory cleanup, which was considered by 
> the use as a "leak".
> 
> The fix changes the usage of SoftCache to the WeakHashMap, so the key(the 
> application observer) will be cleared when the application lost the reference 
> to it.

src/java.desktop/share/classes/sun/awt/image/MultiResolutionToolkitImage.java 
line 130:

> 128: 
> 129: if (observer == null || image == null) {
> 130: return false;

How about a case when `ObserverCache`'s class instance is not collected by GC, 
but its `observerRef` or `imageRef`  is collected. Is it possible?
If so this class instance will permanently return false on `imageUpdate()` call.

-

PR: https://git.openjdk.java.net/jdk/pull/2711


[OpenJDK 2D-Dev] RFR: 8257500: Drawing MultiResolutionImage with ImageObserver "leaks" memory

2021-02-24 Thread Sergey Bylokhov
This bug was reported as a leak of the ImageObserver when the user draws a 
multiresolution image.

The multiresolution image is an image that contains a few images inside, when 
the app uses the observer to get notifications about image loading we create a 
special internal observer and use it to track loading resolution-variants. This 
internal observer forwards notification from the resolution variant to the 
application observer.

The mapping from the application observer to the internal observer is done via 
"SoftCache" which is a kind of HashMap that uses soft references to the key and 
value.

Here the bug comes, the soft references are cleared only under memory pressure, 
and unused objects may sit hours in memory before being cleaned. Moreover, the 
internal observer was implemented using a strong reference to the application 
observer(it is not obvious since the lambda is used). So the key object refers 
to the application's observer cannot be clear fast. This causes an even longer 
delay of the memory cleanup, which was considered by the use as a "leak".

The fix changes the usage of SoftCache to the WeakHashMap, so the key(the 
application observer) will be cleared when the application lost the reference 
to it.

-

Commit messages:
 - Update ImageObserverLeak.java
 - Update MultiResolutionToolkitImage.java
 - cleanup
 - Update MultiResolutionToolkitImageTest.java
 - Initial version
 - temp version

Changes: https://git.openjdk.java.net/jdk/pull/2711/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2711=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8257500
  Stats: 125 lines in 3 files changed: 88 ins; 9 del; 28 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2711.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2711/head:pull/2711

PR: https://git.openjdk.java.net/jdk/pull/2711