(XS) java.logging Level.java minor cleanups

[Daniel Fuchs]

Hi Bernd,

It's in:

https://bugs.openjdk.java.net/browse/JDK-8209987
http://hg.openjdk.java.net/jdk/jdk/rev/5f40be158613

best regards,

-- daniel

On 23/08/2018 20:49, Bernd Eckenfels wrote:
> Yes, Seeburger AG OCA should be on file (b.eckenfels at seeburger.de)
> 
> Thanks for following up
> 
> Did you had any opinion on the synthetic accessors as well?
> Gruß
> Bernd
> 
> -- 
> https://Bernd.eckenfels.net
> ------------------------------------------------------------------------
> *Von:* Daniel Fuchs <daniel.fuchs at oracle.com>
> *Gesendet:* Donnerstag, August 23, 2018 11:20 AM
> *An:* Bernd Eckenfels; core-libs-dev at openjdk.java.net
> *Betreff:* Re: (XS) java.logging Level.java minor cleanups
> Hi Bernd,
> 
> Good finding! I agree with the proposed cleanup.
> I am always a bit uneasy of touching these classes
> as they have a propensity to come back and bite you
> from behind when you're not looking ;-)
> 
> I have applied your patch and sent it to our test system
> for more confidence and everything went well.
> 
> I'll happily sponsor your patch.
> You did sign the OCA - right?
> 
> MfG,
> 
> -- daniel
> 
> On 11/08/2018 20:31, Bernd Eckenfels wrote:
>  > Hello,
>  >
>  > while investigating a Memory leak fix from IBM (IJ05380 is in 
> 8.0.5.20 but not in 8u yet) I noticed that the (fixed) code 
> registerWithClassLoader(Level) in the master queries a unused class name 
> (pn) and it can use the method reference instead of Lambda. See patch 
> below.
>  >
>  > There seems to be also a opportunity to use computeIfAbsent in a 
> unrelated method add(Level), not sure if this is wanted, what do you think?
>  >
>  >
>  > index dbc27124d33..401c6dc73e0 100644
>  > --- a/src/java.logging/share/classes/java/util/logging/Level.java
>  > +++ b/src/java.logging/share/classes/java/util/logging/Level.java
>  > @@ -610,10 +610,7 @@ public class Level implements 
> java.io.Serializable {
>  > }
>  >
>  > private static void registerWithClassLoader(Level customLevel) {
>  > - PrivilegedAction<ClassLoader> pa =
>  > - () -> customLevel.getClass().getClassLoader();
>  > - PrivilegedAction<String> pn = customLevel.getClass()::getName;
>  > - final String name = AccessController.doPrivileged(pn);
>  > + PrivilegedAction<ClassLoader> pa = 
> customLevel.getClass()::getClassLoader;
>  > final ClassLoader cl = AccessController.doPrivileged(pa);
>  > CUSTOM_LEVEL_CLV.computeIfAbsent(cl, (c, v) -> new ArrayList<>())
>  > .add(customLevel);
>  > @@ -624,19 +621,10 @@ public class Level implements 
> java.io.Serializable {
>  > // the mirroredLevel object is always added to the list
>  > // before the custom Level instance
>  > KnownLevel o = new KnownLevel(l);
>  > - List<KnownLevel> list = nameToLevels.get(l.name);
>  > - if (list == null) {
>  > - list = new ArrayList<>();
>  > - nameToLevels.put(l.name, list);
>  > - }
>  > - list.add(o);
>  > -
>  > - list = intToLevels.get(l.value);
>  > - if (list == null) {
>  > - list = new ArrayList<>();
>  > - intToLevels.put(l.value, list);
>  > - }
>  > - list.add(o);
>  > + nameToLevels.computeIfAbsent(l.name, (k) -> new ArrayList<>())
>  > + .add(o);
>  > + intToLevels.computeIfAbsent(l.value, (k) -> new ArrayList<>())
>  > + .add(o);
>  >
>  > // keep the custom level reachable from its class loader
>  > // This will ensure that custom level values are not GC'ed
>  >
>  >
>  > I am happy if anyone can sponsor this change.
>  >
>  > BTW: I get a synthetic Accessor warning on l.name/value, is this what 
> nestmates will resolve – is it worth adjusting the modifiers for those 
> fields anyway?
>  >
>  > Gruss
>  > Bernd
>  >
>