JEP 264: Platform Logging API and Service (original) (raw)
Peter Levart peter.levart at gmail.com
Tue Oct 6 13:32:52 UTC 2015
- Previous message: JEP 264: Platform Logging API and Service
- Next message: JEP 264: Platform Logging API and Service
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On 10/05/2015 09:59 PM, Daniel Fuchs wrote:
Thanks Roger!
I have updated the specdiff online. http://cr.openjdk.java.net/~dfuchs/8046565/proto.01/specdiff-api/overview-summary.html
The only comment I haven't taken into account yet is this: > - the LoggerFinder constructor says "Only one instance will be created". > That applies to the normal static logger initialization > (getLoggerFinder). > But there might be other cases where the application or service > might create a LoggerFinder > for its own purposes, and the phrase is not accurate in that case. I was wondering whether I should try enforcing this actually, by throwing a ServiceConfigurationError or whatever if the LoggerFinder service is already loaded when the constructor is invoked. We had trouble in the past with LogManager - because the spec said there should be only one instance, but the implementation did not enforce it. It may be a bit different with LoggerFinder - as this is an abstract class devoid of instance state (the only method with a body is getLocalizedLogger and that's stateless) - so there may not be as much harm as with LogManager. There is probably no good way of implementing such enforcement though - so it would be a best effort :-(
Hi Daniel,
Scala has singleton objects. Java hasn't. Your statement intrigued me to think whether it would be possible to enforce a one-instance-per-concrete-class rule in Java using just API. Here's a prototype that I think does that. Can you think of a corner case that fools it?
/**
An abstract base class for subclasses that can only have one instance
per concrete subclass. Subclasses must define a public no-args constructor
which must never be called directly from code (it throws
UnsupportedOperationException), but via the factory method:
{@link #getInstance(Class)}. */ public abstract class ClassObject {
/**
- Lazily constructs and returns a singleton instance per given
concrete * {@code ClassObject} subclass or throws exception. * Subclasses must define a public no-argument constructor. Multiple * invocations of this method with same {@code clazz} parameter either return * the same instance or throw the same exception. The result of this method * is therefore stable for given parameter. * * @param clazz the Class representing concrete {@code ClassObject} subclass * @param the type of the {@code ClassObject} subclass * @return a singleton instance for given {@code clazz} * @throws InstantiationException see {@link Constructor#newInstance} * @throws IllegalAccessException see {@link Constructor#newInstance} * @throws InvocationTargetException see {@link Constructor#newInstance} */ public static T getInstance(Class clazz) throws InstantiationException, IllegalAccessException, InvocationTargetException { return clazz.cast(factoryCV.get(clazz).get()); }
/**
* ClassObject constructor allows construction of a single instance of
* {@code ClassObject} subclass per concrete subclass.
*/
public ClassObject() {
Factory factory = factoryCV.get(getClass());
synchronized (factory) {
if (!factory.inConstruction) {
throw new UnsupportedOperationException(
"Direct construction of ClassObject instances is
not supported." + " Please use ClassObject.getInstance(Class) instead."); } if (factory.constructed != null) { throw new IllegalStateException( "Attempt to instantiate a second " + getClass().getName() + " instance."); } factory.constructed = this; } }
/**
* A ClassValue cache of Factory instances per class
*/
private static final ClassValue<Factory> factoryCV = new
ClassValue() { @Override protected Factory computeValue(Class<?> clazz) { return new Factory(clazz.asSubclass(ClassObject.class)); } };
/**
* A Factory responsible for constructing and caching a singleton
instance * of specified class. */ private static final class Factory { // the class of instance to construct private final Class<? extends ClassObject> clazz; // published instance or Throwable private volatile Object instance; // temporarily set to true while constructing and checked in ClassObject constructor boolean inConstruction; // the just being constructed instance or Throwable, set in ClassObject constructor Object constructed;
Factory(Class<? extends ClassObject> clazz) {
this.clazz = clazz;
}
ClassObject get() throws InstantiationException,
IllegalAccessException, InvocationTargetException { Object obj = instance; if (obj == null) { synchronized (this) { obj = instance; if (obj == null) { if (constructed == null) { try { Constructor<? extends ClassObject> constructor = clazz.getDeclaredConstructor(); inConstruction = true; constructor.newInstance(); } catch (Throwable t) { // override with construction exception if thrown constructed = t; } finally { inConstruction = false; } } instance = obj = constructed; assert obj != null; } } } if (obj instanceof ClassObject) { return (ClassObject) obj; } Factory.sneakyThrow((Throwable) obj); // not reached return null; }
@SuppressWarnings("unchecked")
private static <T extends Throwable> void sneakyThrow(Throwable
t) throws T { throw (T) t; } } }
I don't think it is very important to enforce such rule for services provided via ServiceLoader since similar effect can be achieved by treating the published abstract type only as a factory for a real service which can then be initialized and cached as a singleton object in the implementation itself. But that can not be enforced on the system level. If the abstract type has state and you would want to initialize it only once, ClassObject might be an answer. If ClassObject abstract class was part of JDK, ServiceLoader would have to special-case it's subclasses and invoke ClassObject.getInstance(Class) instead of the no-arg constructor to obtain the instance.
Regards, Peter
best regards, -- daniel
On 05/10/15 16:06, Roger Riggs wrote: Hi Daniel,
This looks good, a few comments... j.l.System: - the behaviors described by the @implNote in getLogger(name) and @implSpec in getLogger(name, resourceBundle) seem like they should be consistent for the two methods. System.Logger: - log(level, throwable, Supplier) - to be more consistent, the Suppler argument should be before the Throwable argument. Then in all cases the msg/string is before the Throwable. System.Logger.Level - TRACE might be used for more than just method entry and exit, perhaps the description can be more general: "usually used for diagnostic information." LoggerFinder: - should the CONTROLPERMISSION field be renamed to "LOGGERFINDERPERMISSION"? - the LoggerFinder constructor says "Only one instance will be created". That applies to the normal static logger initialization (getLoggerFinder). But there might be other cases where the application or service might create a LoggerFinder for its own purposes, and the phrase is not accurate in that case.
Editorial: - The @since tags can be set to "9". - "JVM" -> "Java runtime"; JVM would specifically refer to the virtual machine implementation. (j.u.System.LoggerFinder) - "used by the JDK" can be omitted. (j.u.System.LoggerFinder) When used in the context of the JDK documentation itself it seems out of place and is unnecessary. - the word 'actually' can/should be omitted from descriptions. (j.l.System) Thanks, Roger On 10/5/2015 6:54 AM, Daniel Fuchs wrote: New JEP Candidate: http://openjdk.java.net/jeps/264 Hi I have uploaded an initial prototype specdiff: http://cr.openjdk.java.net/~dfuchs/8046565/proto.01/specdiff-api/overview-summary.html
LoggerFinder methods that take a Class<?> caller argument will take a java.lang.reflect.Module callerModule instead when that is available in jdk9/dev. comments welcome, best regards, -- daniel
- Previous message: JEP 264: Platform Logging API and Service
- Next message: JEP 264: Platform Logging API and Service
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]