(XS) java.logging Level.java minor cleanups (original) (raw)

Bernd Eckenfels ecki at zusammenkunft.net
Thu Aug 23 19:49:39 UTC 2018


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 pa = - () -> customLevel.getClass().getClassLoader(); - PrivilegedAction pn = customLevel.getClass()::getName; - final String name = AccessController.doPrivileged(pn); + PrivilegedAction pa = customLevel.getClass()::getClassLoader; final ClassLoader cl = AccessController.doPrivileged(pa); CUSTOMLEVELCLV.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 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



More information about the core-libs-dev mailing list