[modules-dev] Updated review request for 6628143, Service Loader support

[Dave Bristor]

Hi folks,

I incorporated the feedback (thanks, Mandy!) and have updated the webrev:

6621843 Support service- and service-provider modules
webrev: http://webrev.invokedynamic.info/bristor/6621843-17/
bug report: http://bugs.sun.com/view_bug.do?bug_id=6621843

I'd like to get any feedback on these changes by 2/July, a week from today.

As far as I know, no one has looked much into the new ServiceProcessor.java 
class... it was my "learning experience" for annotation processing and there 
are possibly better ways to do things.

Here are my notes on the changes:

* loadInstalled and extension repository: Based on followup conversation with 
Stanley, I've changed RepositoryConfig to have a getLastRepository() method. 
loadInstalled() uses this instead of getSystemRepository().

* laziness: I was able to defer calls to isCompatible(), which is what creates
an instance of a service provider, into the ModulesIterator.hasNext().  The
old code was such that, upon invocation of hasNext(), each candidate service
provider's class was loaded.  Now, service provider classes will be loaded,
but not all of them: Each call to hasNext() will load only enough service
provider candidates to allow next() to return one of them.  So lets say there
are 5 candidates, and the second one has isCompatible() == false: The first
call to hasNext() loads a class, the 2nd call to hasNext() loads 2 classes
(the incompatible one, and then a compatible one), the 3rd call loads 1 class,
the 4th call loads one class, and the 5th call doesn't load any (none left!)
and returns false.

* doPrivileged: After investigation, conversation, and looking at the 
getClassLoader() source, we agreed that the original code was OK: no 
doPrivileged blocks are necessary, so those I added have been removed.

* printlns: I took them out.  They were in isCompatible().  Since that's now
being called from hasNext(), it is a compatible change to have the exceptions
being handled mirror the behavior of LazyIterator.next(), and invoke fail()
appropriately (which will throw ServiceConfigurationError).

* for loop in getCandidates: You suggested using a foreach, but that can't be
done, because of the removal from the iterator within the loop.  But...

* ... you also suggested using a Map<module name, module provider>, which may
do away with the need for remove() on the iterator.  I've done that, and think
it is much cleaner (thanks!)

Thanks,
	Dav