ServiceLoader.load* take 2

[Paul Sandoz]

Hi Mandy,

Thanks for reviewing.

On Jun 21, 2012, at 3:01 AM, Mandy Chung wrote:

> Paul,
> 
> On 6/12/2012 8:03 AM, Paul Sandoz wrote:
>> the first part of which is implemented by the following webrev, which changes service instance creation so that it is scoped to the configuration rather than the class loader:
>> 
>>   http://cr.openjdk.java.net/~psandoz/jigsaw/global-services/webrev/
> 
> I believe you want to discuss and push the service loader changes separately.  I reviewed this first part which looks good overall.
> 
> I also wonder if the new ModuleServiceLoader could just be an inner class in ServiceLoader and I understand your thought behind and change Loader.findServices to package-private.  I'm fine if you prefer to keep it as is.
> 

OK.


> LoaderPool.java L119 - Is 'servceInterfaceName' a typo (missing letter 'i' in "service")
> 

Grrr... from day one the 'i' key on my MacBook Pro keyboard has been dodgy.


> ModuleServiceLoader.java L124: does javadoc parse '<S>' correctly?
> 

Yes, JavaDoc will display it in the "Type Parameters" section of the documented method. FWIW NetBeans auto-generated it for me :-)

--

I will send out another webrev today with an update for the two changes.


> classloader.sh test - looks like you can just extend hello.sh, many.sh, and possibly optional.sh to test the different ServiceLoader.load* methods that could also be launched in a new VM if appropriate.
> 

Yes, there is lots of duplication here that can be fixed, plus we also need to expand the tests to include such things as transitive dependencies and second level service consumer modules. Do you mind if we tackle that as a separate set of commits?

I am not too familiar with the test infrastructure and the reuse of such shall scripts (it seems like open season and we can do what we want as long as there is appropriate meta-data that jtreg understands). Any more advice on how to improve this would be greatly appreciated?

Paul.