RFR: JDK-8046565: Platform Logger API and Service (original) (raw)
Mandy Chung mandy.chung at oracle.com
Tue Oct 20 07:10:38 UTC 2015
- Previous message: RFR: JDK-8046565: Platform Logger API and Service
- Next message: RFR: JDK-8046565: Platform Logger API and Service
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On Oct 19, 2015, at 11:18 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
webrev: http://cr.openjdk.java.net/~dfuchs/8046565/proto.02/webrev/index.html
Looks good in general. Some minor comments:
Good to see limited privileges being used that limits the permissions when invoking logger finder which is good.
LoggerFinderLoader.java
110 (PrivilegedAction<ServiceLoader>)
- can this use diamond operation (PrivilegedAction<>)?
System.java 1547 return LazyLoggers.getLogger(Objects.requireNonNull(name), caller);
Objects.requireNonNull(name) - already called. It can just do getLogger(name, caller).
RuntimePermission.java 360 * to JDK classes.
s/JDK/system/
LazyLoggers.java
sun/management/ManagementFactoryHelper.java
172 public interface LoggingMXBean 173 extends PlatformLoggingMXBean, java.util.logging.LoggingMXBean { 174 }
LazyLoggers.java 68 this(Objects.requireNonNull(loggerSupplier), 69 (Void)null);
130 this(Objects.requireNonNull(name), Objects.requireNonNull(factories), 131 Objects.requireNonNull(caller), null);
I would expect Objects.requireNonNull on the constructor initializes all the fields. But the above lines checks non-null before calling this(….). Does the other constructor allow null parameters? Why not moving Objects.requireNonNull to the other constructor?
365 new BiFunction<String, Class<?>, Logger>() {
- it can use diamond operator
BootstrapLogger.DetectBackend looks like have some duplication as LoggerFinderLoader.
- does it make sense to merge them?
923 boolean cleared;
- should that be volatile field?
AbstractLoggerWrapper.java
212 if (sourceClass==null && sourceMethod==null) { 234 if (sourceClass==null && sourceMethod==null) {
Missing space around “==“ and also in a few other lines.
java.util.logging.LogManager.demandLoggerFor
- I think this method is intended for the LoggerFinder provider to use. Perhaps we should add a note in the javadoc to say that.
SimpleConsoleLogger.skipLoggingFrame and another place is finding the caller information. JEP 259 may be useful for stack walking with filtering.
I skimmed through the tests and nothing pops up. There are multiple copies of AccessSystemLogger that can be shared as I comment last time.
Mandy
- Previous message: RFR: JDK-8046565: Platform Logger API and Service
- Next message: RFR: JDK-8046565: Platform Logger API and Service
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]