Proposed signature:

@Target(ElementType.TYPE)
public @interface Service {
  Class[] value();
  int position() default Integer.MAX_VALUE;
  String[] supersedes() default {};
  String path() default "";
}

Created attachment 72144 [details]
Example registration for openide.filesystems

Created attachment 72145 [details]
Proposed implementation

Y01 I'd rather see the annotation next to Lookup then in module system. Maybe as org.openide.util.lookups.Service. It 
seems to me it is more lookup related than module system related.

Otherwise perfect, this is how our future should look like.

Looking forward to it as well. Couple of questions:

R01 When does annotation processor get called? During build?
R02 What about UI? I suppose we want to keep service nodes in Project, but adding a service via context menu should be
changed to add an annotation, right? Plus there should be context menu item "Register as service" on eligible classes. 
R03 (related to R01) Is it sufficient for Service Node to listen to META-INF/services as it currently does? In other
words when someone adds the annotation to a class, will META-INF/services be updated anytime soon?

Y01 - reasonable, will do.

R01 - yes, during the build. If using JDK 6+ to build, this would be automatic, but to support building on JDK 5 for now
we need to do some tricks to run the JDK 6 javac; see issue #147393.

R02 / R03 - as part of the change I am deleting the META-INF/services GUI entirely. It was never very stable or fast,
and is now superfluous when you can declare a service directly from source code with no special metadata files.

Re deleting UI: not sure about 'superfluous', following the same reasoning Navigator window is also superfluous as you
can't add a method there :).
Service UI has never been especially useful for adding a service, but IMHO its primary purpose is to show what services
are exported from the module and what services will be present in the whole application (you see, I am a fan of those
"overall" views :-)). With annotations this kind of information is even more scattered over the sources.

Showing services exported from a single module is I think not very useful. (Note that they are already displayed as
informational notes during the build.) Showing all services present in an application from a suite logical view might be
useful, though I am not sure of the exact use case. Remember you can find all registrations of a particular interface in
open module projects just using Find Usages; apisupport.refactoring could if desired add a provider which additionally
reports M-I/s/* registrations present in the binary platform corresponding to open module(s). And of course you can Find
Usages on @Service itself if you really wanted to.

In any case the previous GUI code is no longer worth much: the parts concerned with edit operations are obsolete, and
the display aspect would no longer work anyway because it would not see services defined in module projects. The same
considerations will also affect the XML layer node eventually, once most layer entries are generated from annotations.

There is also the general problem with overview nodes that it is very hard to make them both efficient and correct. For
correctness you need to listen for changes on a lot of different data sources, and this can quickly lead to leaks and
"event storms". Overview functionality which is produced as a static report upon request (e.g. Find Usages) does not
suffer from these problems - it takes however long it takes (and may be cancellable), and is accurate at the time it was
created.

The proposed implementation is not safe in the context of incremental compiles.   For how to do that see
https://hickory.dev.java.net/nonav/apidocs/index.html?net/java/dev/hickory/incremental/package-summary.html. (Note that
the javadoc for the (*sole public) class in that package provides a sample implementation of @ServiceProvider.) There
are also a couple of other things about that example which are worth heeding - don't generate the output till processing
is complete (think what would happen if a different processor in the first round generated a class annotated with this
proposed annotation) and don't do anything if there are errors raised.

@Service is probably not the correct name. (see http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6354623 for a related
earlier problem). According to http://java.sun.com/javase/6/docs/api/index.html?java/util/ServiceLoader.html this
proposed annotation identifies the "ServiceProvider". The "Service" is the abstract class or interface that is being
implemented so the member which specifies that should be called "service" (rather than value). It should rather be like this

@interface ServiceProvider {
    Class<?> service();
}

(note using "value" as a member name is probably a bad idea unless it is the only member)

from here on I'll use my suggested naming.

As an alternative to specifying the service with an annotation member each of the "Services" in neatbeans could be
annotated with a new marker annotation @Service then the processor for @ServiceProvider could look at the supertypes of
the target and determine the Service(s) automatically which saves the programmer from specifying them explicitly since
they are usually pretty obvious from inspection of the declared supertypes.

I wonder whether the META-INF/services portion is of such general value that it would be more valuable as part of the
JDK rather than hidden away inside NB? 

Are there any existing examples where a single class is a Provider for more than one Service? Can the complexity of
having multiple services specified (Class[] value() ) be justified? And if so, what if the position or supersedes 
values are different for the multiple services? Its actually all a bit ugly. The convention for dealing with possible
plurality is to introduce a collection annotation, for example

@interface ServiceProviders {
    ServiceProvider[] value();
}

would provide a cleaner mechanism if a class is a ServiceProvider for more than one service allowing each to have a
different position etc.

The 'String path() default "";' member is unrelated to META-INF/services and I can't see how 'String[] supersedes()
default {};' can be implemented for META-INF/services. Maybe these are two different things and maybe there should be
two different annotations, one for {@link Lookups#forPath}and one for java.util/ServiceLoader / {@link
Lookup#getDefault} to separate the concerns. Mixing them is somewhat confusing.

The javadoc for 'int position() default Integer.MAX_VALUE;' (at least for the META-INF/services case) should be a bit
more explicit about the ordering being only relative to other service providers for the same service in the same module.
(that is it only controls the ordering in one META-INF/services file. Nothing in the META-INF/services mechanism
provides for specifying any absolute ordering between different jar files, they are searched in classpath order. 

Bruce, thank you for the detailed review! I will try to address your points in turn.


First of all, yes it would be great for an annotation like this to be defined in the Java platform, but we cannot wait
for years for a new JDK incorporating such an API to be released (and then years more for NB to be able to assume that
all its users are using at least that version of the Java platform). Anyway NB's Lookups.metaInfServices defines
extensions to the ServiceLoader behavior (e.g. positions) which would not be supported by a Java platform annotation.


I agree that @ServiceProvider is likely a better name. Regarding the use of the special attribute name "value", you may
be right that this should not be used in case there are other optional attributes (as indeed there are), though I will
point out that at least javax.annotation.Generated violates this convention already. "service" is a reasonable name for
the attribute.


I like your suggestion of @ServiceProviders({@ServiceProvider(...), @ServiceProvider(...)}) for dealing with multiple
registrations from a single class. To answer your question, yes there are cases where this would be used, e.g.
org.netbeans.modules.java.project.UnitTestForSourceQueryImpl is registered as both a
org.netbeans.spi.java.queries.MultipleRootsUnitTestForSourceQueryImplementation and a (deprecated)
org.netbeans.spi.java.queries.UnitTestForSourceQueryImplementation.


The suggestion of marking services with @Service and then inferring the service name in a @ServiceProvider by inspection
of its supertypes has come up before. For now I think this is not a good idea, for these reasons:

1. It would interact poorly with @ServiceProviders, especially if you wanted separate positions for each supertype.

2. It would gratuitously prevent you from registering a provider for a service which has long been defined but which has
not yet been marked with @Service.

3. It would violate the general principle that the behavior of a class should not be modified merely by implementing a
new interface.

4. To someone reading source code (a much more important audience than the person writing it!) "@ServiceProvider" does
not say what is really happening (you need to think for a moment about what interface is involved), whereas
"@ServiceProvider(service=Something.class)" says it very clearly. This is especially important when the provider does
not directly declare that it implements the interface but goes through an intermediate base class.

5. Having an explicit declaration of the service in the source code makes it easier for tools (other than the AP) to
generate summaries of service providers, and lets Find Usages jump to all providers of a given service.

#1-#3 could be mitigated by retaining a "service" attribute but making it optional when exactly one supertype is a
declared @Service. I consider #4 the most relevant point: too much "magic" makes code harder to follow, and in this case
it is not making the code significantly shorter anyway.


To the question of incremental compilation:

The proposed implementation already does handle incremental compilation, though not for 269-generated sources (which we
anyway do not use in NB yet): it deals with multiple types being registered under one service within the same job, and
also with a M-I/s/* file already present in the class output area from a previous job (which will be appended to,
similar entries being replaced). I learned to do this kind of trick independently in sezpoz.dev.java.net but I am glad
to see there is a generic infrastructure for it now available (though for reasons of dependency minimization we probably
cannot use it as such in NB). I will try to modify the processor to abort writing if there is an error condition, and to
defer writing until the last round in order to accommodate annotated generated sources.

See issue #149136 for further discussion on the topic of processor lifecycle. I find the 269 API to be maddening in that
it is close to impossible for multiple processors to cooperate on writing the same class output file (thus the
WeakHashMap<ProcessingEnvironment,...> hack in that implementation). In the case of @ServiceProvider this is not such a
big deal because it can be expected (hoped?) that there is one and only one processor generating M-I/s/* files.

Another technical question relates to incremental compilation: 269 does not state clearly whether it is legal for a
processor to overwrite a class output resource file which existed before the job started (e.g. from a previous run). It
is specific that the AP cannot write to the file twice in a job, but also says that inputs are considered to have been
generated by a primordial round. In fact JDK 6+ javac permits this whereas APT does not. The processor in this issue (as
well as the processor base class in #149136) absolutely requires this to work, as otherwise it would be impossible to
support routine incremental compilation. (Note that if you delete an annotated source file and do an incremental build,
the annotation will remain - but then again, so will the class file. As is usual, after deleting things you need to
remember to do a clean build of the module. I cannot think of any way to fix this.)


The "path" attribute just modifies the registration to be in META-INF/namedservices/$path/ rather than
META-INF/services/ and can be used with our Lookups.forPath. Other than the existence of the path, the behavior of the
service provider registration is identical, so I think using the same annotation is appropriate.

The "supersedes" attribute is orthogonal; again this implements a NB-specific extension to the service provider
mechanism. One module can hide another's provider.

The "position" attribute does indeed specify a global ordering across modules. Again this relies on a NB-specific extension.

*** Issue 141678 has been marked as a duplicate of this issue. ***

Merged in core-main #78103b644dfc.

I am in the process of updating all main & contrib M-I/s/* registrations to use @ServiceProvider. This is not trivial as
there are a lot of mistakes and special cases in our source base, hidden in the middle of hundreds of normal correct
registrations.

Created attachment 73147 [details]
Script to update registrations

Created attachment 73160 [details]
Updated script with several fixes

Using annotation in core-main #78da8804b135.

Also in contrib #0b9b6af17703.

Integrated into 'main-golden', will be available in build *200811041401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/d822d0d40c42
User: Jesse Glick <jglick@netbeans.org>
Log: #150447: @Service annotation.

What is about performance? 

The manifest-files an the layer.xml files are well known files in the JARs.

For annotations you need to scan all the classes to detect the ServiceProvider-annotation. The boot performance of
modules is a very critical section. With annotations we need(?) a additional register cache or we can wait a long time
for booting netbeans.

Hibernate lives with the same problem. Many JARs results in a very poor boot startup. In Hibernate you can exclude jars
from scanning JPA annotations. 

josh.

Josh,

No runtime preformance difference at all.  The annotations are processed at COMPILE TIME by an annotation processor
which generates the META-INF/services/service.xxx file.


Bruce

Bruce, thank you for the clarification. josh.