RFR: 6543126: Level.known can leak memory

[Daniel Fuchs]

Hi Peter,

On 16/08/16 13:05, Peter Levart wrote:
> Hi Daniel,
>
> Passing Stream<Level> returning extractors to Stream::flatMap is one way
> of doing it. The other would be to make findByXXX methods return
> Optional<KnownLevel> and then use Optional::map method on the result to
> extract the needed property, so instead of:
>
>     Optional<Level> level = findByName(name, KnownLevel::referent);
>
> You could simply make findByName return Optional<KnownLevel> and then:
>
>     Optional<Level> level = findByName(name).map(Reference::get);
>
> Just plain null-when-absent-returning getters used with Optional::map
> would do.

Well - not exactly because when looking for levelObject the KnownLevel
could become stale after it's returned. What we want is loop until
we find a levelObject which is not null (that's why the previous
version had some while loops in Level.parse).

But maybe I'm misunderstanding what you are saying?

best regards,

-- daniel

>
>
> Regards, Peter
>
> On 08/16/2016 12:42 PM, Daniel Fuchs wrote:
>> Hi Mandy,
>>
>> I added an additional selector parameter to the find methods.
>> This made it possible to return Optional<Level> instead of
>> KnownLevel - and it does simply the parse() method.
>>
>> http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.02
>>
>> best regards,
>>
>> -- daniel
>>
>> On 11/08/16 20:12, Mandy Chung wrote:
>>>
>>>> On Aug 11, 2016, at 2:29 AM, Daniel Fuchs <daniel.fuchs at oracle.com>
>>>> wrote:
>>>>
>>>> On 10/08/16 17:21, Mandy Chung wrote:
>>>>>> On Jul 29, 2016, at 4:54 AM, Daniel Fuchs
>>>>>> <daniel.fuchs at oracle.com> wrote:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/
>>>>> This looks pretty good.
>>>>>
>>>>> Since KnownLevel is now a Reference, I suggest to change
>>>>> KnownLevel::findByName, findByValue and findByLocalizedLevelName to
>>>>> return Optional<Level> instead such that the parse method
>>>>> implementaiton could be simplified.
>>>>
>>>> We need to return KnownLevel because sometimes we need the
>>>> level object and sometimes the mirror.
>>>
>>> So either findByName(String name, boolean mirror) or two methods:
>>> findLevelByName and findMirroredLevelByName??
>>>
>>> Or seriously consider to remove KnownLevel class by introducing a new
>>> Level subclass with final Level.getName, Level.getLocalizedName,
>>> Level.getResourceBundleName methods??
>>>
>>> Mandy
>>>
>>
>