RFR: JDK-8046565: Platform Logger API and Service
Mandy Chung
mandy.chung at oracle.com
Tue Oct 20 07:10:38 UTC 2015
> 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<DefaultLoggerFinder>>)
- 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.</td>
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
More information about the core-libs-dev
mailing list