[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: [PATCH] 8130458 : BooleanBinding binding/unbinding leaks
From:       Jesper Skov <jskov () zoftcorp ! dk>
Date:       2015-08-16 8:40:19
Message-ID: CA+_EZTmf2_22y2ww=P_YT9iBODmE597BQxCbxLW7Lr94j6Wm_w () mail ! gmail ! com
[Download RAW message or body]

Thanks for your reply. I didn't reply at the time, as I was away on
vacation.

I see you made patches for all types, which is super.

I'll look for something else to fix.

Cheers,
Jesper

On Fri, Jul 24, 2015 at 4:15 PM, Vadim Pakhnushev <
vadim.pakhnushev@oracle.com> wrote:

> Hi Jesper,
>
> First of all if you want to contribute to OpenJFX you need to sign OCA, as
> described here: http://openjdk.java.net/contribute/
>
> The change itself looks OK, but we might want to look at the other similar
> issues, for example, all objects returned from helper class Bindings will
> suffer from this.
> That is, in the test case from the 8130458 if you change the code in the
> loop to Bindings.not(p) the memory will also grow for the same reason.
>
> There is a mechanism for dealing with this kind of issues,
> ExpressionHelperBase.trim method, which works with WeakListeners.
> So I think it's worth to try to wrap BindingHelperObserver in the
> WeakListener in the Binding implementations (BooleanBinding) and see if it
> helps.
> trim implementation is suboptimal though so if for example a lot of
> listeners get collected, then there would be a lot of unnecessary array
> copying.
>
> Thanks,
> Vadim
>
>
> On 19.07.2015 11:20, Jesper Skov wrote:
>
>> Hi,
>>
>> I had a look quick look around in the bugs lists for something to try
>> fixing. I happened on this issue.
>>
>> I am aware that the other primitive binding classes should have similar
>> treatment.
>> But I wanted to get approval/hints before continuing.
>>
>> The change adds a new inner class that allows the wrapped binding to be
>> unbound when the parent is.
>>
>> There is no test (hard to test presence/absence of a leak).
>>
>> Thanks,
>> Jesper
>>
>
>
[prev in list] [next in list] [prev in thread] [next in thread]