[OpenJDK 2D-Dev] RFR: 8175984 : ICC_Profile has un-needed, not-empty finalize method

[Sergey Bylokhov]

On 10/11/19 3:43 pm, Philip Race wrote:
> On 10/11/19, 2:52 PM, Sergey Bylokhov wrote:
>> Hi, Phil.
>>
>> It looks like after the fix we will skip the call "unregisterDeferral" and
>> the ProfileActivator which was registered by the
>> "ProfileDeferralMgr.registerDeferral" will not be removed --> memory leak?
> 
> I discussed that below in my initial email. It won't in practice leak anything.

Initially missed that, +1.

> 
>  > I had started to look at adding Disposer code to do the other thing that
>  >the finalize() method does which is remove a deferred activation registration
>  >but decided this was pointless.
> 
>  >That is used only for the pre-defined instances held in static variables of the class
>  >and those live for the life of the VM so will never be freed ...
> 
> -phil.
>>
>> BTW do we have an option to mark this "finalize" as forRemoval=true or just delete it?(example JDK-8159009/JDK-8212198)
>>
>> On 10/11/19 2:01 pm, Philip Race wrote:
>>> Bug: ICC_Profile has un-needed, not-empty finalize method
>>> Webrev: http://cr.openjdk.java.net/~prr/8175984/
>>>
>>> This is a native memory utilisation problem due to delay in collecting and
>>> freeing ICC_Profiles and their associated classes and data.
>>>
>>> The color management engine is native code and consumes the stream of data
>>> representing the ICC profile and constructs internal native data structures.
>>> The delay is because the garbage collector doesn't know about the native memory
>>> consumption and from a Java perspective there is no need to initiate garbage collection.
>>>
>>> But it is worse than that because even collection when does happen it is delayed in
>>> freeing this memory.
>>>
>>> The native data is referenced internally via a Profile object which is registered with the 2D disposer
>>> so that the data will be freed after the disposer's  reference to it is enqueued and cleared.
>>>
>>> The disposer needing to run does add some delay but there's a further delay.
>>>
>>> ICC_Profile has  non-empty finalize which is there to free this native data.
>>>
>>> This used to matter with the old closed source CMS *only* because no one bothered
>>> to implement 2D disposer support for it.
>>>
>>> But the call in finalize() is a no-op for LCMS
>>>
>>> So currently the ICC_Profile has to be finalized and only then can the GC initiate
>>> clearing the weak refs and freeing the native data.
>>>
>>> Emptying the finalize() [*] method helps make the reclamation more prompt.
>>> It is still possible to see large amounts of native memory consumed but it is
>>> much better than before.
>>>
>>> [*] For those who don't know, the VM treats an empty finalize() method as non-existent
>>> so the object does not need to be finalized.
>>>
>>> I had started to look at adding Disposer code to do the other thing that
>>> the finalize() method does which is remove a deferred activation registration
>>> but decided this was pointless.
>>>
>>> That is used only for the pre-defined instances held in static variables of the class
>>> and those live for the life of the VM so will never be freed ...
>>>
>>> There is more clean up that could be done in the lower classes - is freeProfile needed any more ?
>>> Do we actually want to keep the deferral mechanism ?
>>>
>>> Can we do something to provide a way to more promptly free the data, perhaps via new API ?
>>>
>>> But I decided to keep it to a minimum for safe and trivial backporting and leave the rest
>>> to another day.
>>>
>>> -phil.
>>>
>>>
>>>
>>>
>>
>>


-- 
Best regards, Sergey.