You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by Emilian Bold <em...@gmail.com> on 2017/07/19 00:16:14 UTC

Replacing ClassFinder with ServiceLoader

Hello,

I noticed ClassFinder while tweaking ActionRouter and I believe it should
be replace with a proper service declaration and loading.

I'm a fan of the Lookup API (see [1] [2]) which is a small standalone JAR
used a lot in NetBeans.

The standard ServiceLoader [3] would also be a better replacement.

Some remarks:

* ClassFinder is almost always called with JMeterUtils.getSearchPaths().
This must be expected to be the class path actually.
* many calls of ClassFinder do some filtering, excluding some
implementations. This would be simple by just not registering those
implementations as a service.

With the ServiceLoader, ActionRouter would load commands with something
like:

> ServiceLoader<Command> commands = ServiceLoader.load(Command.class);

An each command will have to be registered in META-INF/services (for Java
8) or in the module declaration (for Java 9).

In NetBeans we have an annotation @ServiceProvider [4] which is simpler and
behind the scenes the build system generates at build time the
META-INF/services registration file.

Note that service registration and loading would work the same for 3rd
party JARs.

The only downside is we have to think about older external JARs that expect
the current behaviour. We could use some flag (in MANIFEST perhaps?) to
differentiate between them.

1.
http://bits.netbeans.org/8.0/javadoc/org-openide-util-lookup/org/openide/util/Lookup.html
2.
http://bits.netbeans.org/8.0/javadoc/org-openide-util-lookup/org/openide/util/lookup/doc-files/lookup-api.html
3. http://download.java.net/java/jdk9/docs/api/java/util/ServiceLoader.html
4.
http://bits.netbeans.org/8.0/javadoc/org-openide-util-lookup/org/openide/util/lookup/ServiceProvider.html

--emi

Re: Replacing ClassFinder with ServiceLoader

Posted by Emilian Bold <em...@gmail.com>.
BTW: here is an example code with ServiceLoader in the JMeter codebase
https://github.com/emilianbold/jmeter/commit/4bc26448e8c301b2af11f6629da93081abcb9935

What I'm trying is separate the hard dependency on NewDriver which
lives in the separate launcher JAR.

I'm introducing for that two new services (well, singletons,
actually): Places and ClassPath.

They are registered in META-INF/services.

And the launcher has the implementations of these services which
provide a bridge towards NewDriver.


--emi


On Thu, Jul 20, 2017 at 10:45 PM, Emilian Bold <em...@gmail.com> wrote:
>>  - Another critical part is to make it as easy as today to register a
>>   plugin which is IMO one of the major reasons for JMeter success.
>
> Do you mean easy to register a plugin in the UI or easy to develop a plugin?
>
>>   - The backward compatibility with 3rd party plugins is a critical part.
>>   IMO, no change should be made before validating new proposal will work with
>>   most popular existing 3rd plugins.
>
> I can imagine a scenario where old plugin JARs are inspected with the
> current ClassFinder while for everything else we use ServiceLoader.
>
> The idea being that the old style is deprecated and will be removed in
> time. But we shouldn't use ClassFinder at all in JMeter internal code.
>
>> Can you clarify:
>> => "The only downside is we have to think about older external JARs that
>> expect the current behaviour. We could use some flag (in MANIFEST perhaps?)
>> to differentiate between them."
>
> It would help to know if a plugin uses the ServiceLoader or not. One
> trivial check would be to verify for the presence of
> META-INF/services/. Another would be to invent a manifest flag,
> perhaps "X-JMeter-API-level: 4" in the MANIFEST.MF
>
> Markings in the GUI and showing warnings about plugins that don't keep
> up with API changes would also be a good way to encourage them being
> up to update.
>
>> => "many calls of ClassFinder do some filtering, excluding some
>> implementations. This would be simple by just not registering those
>> implementations as a service."
>
> ClassFinder calls usually include the "String notContains" filter.
> Which is an odd way because callers shouldn't care what they get
> except in very specific situations. If a "service" is registered
> globally it will be there.
>
> I assume filtering happens because you need some implementations
> (like, a GUI Command) but there's no way to differentiate right now if
> that implementation is for global public consumption or not. Is should
> be generally up to the implementation class to declare itself a
> service and not for the calling class to filter (because then, every
> new subclass needs a decision about being filtered or not).
>
> Also note that the Lookup API handles this global / labeled separation
> with "named" lookups inside META-INF/namedservices/ [1]. This would
> allow one to write
> Lookups.forPath("GUI/Main/Menu").lookupAll(Command.class) and
> Lookups.forPath("GUI/Report/Menu").lookupAll(Command.class) and get
> the commands instances separately. But this is an advanced situation
> -- seems to me JMeter never uses the filtered classes in some other
> case, so it's just a way of 'hiding' them.
>
> 1. http://bits.netbeans.org/8.0/javadoc/org-openide-util-lookup/org/openide/util/lookup/Lookups.html#forPath(java.lang.String)
>
> --emi
>
>
> On Thu, Jul 20, 2017 at 10:20 PM, Philippe Mouawad
> <ph...@gmail.com> wrote:
>> Hello Emilian,
>> Thanks for your analysis , and new ideas !
>>
>> Can you clarify:
>> => "The only downside is we have to think about older external JARs that
>> expect the current behaviour. We could use some flag (in MANIFEST perhaps?)
>> to differentiate between them."
>>
>> => "many calls of ClassFinder do some filtering, excluding some
>> implementations. This would be simple by just not registering those
>> implementations as a service."
>>
>> My 2 cents with current (partial ?) understanding of your proposal:
>>
>>    - The backward compatibility with 3rd party plugins is a critical part.
>>    IMO, no change should be made before validating new proposal will work with
>>    most popular existing 3rd plugins.
>>    - Another critical part is to make it as easy as today to register a
>>    plugin which is IMO one of the major reasons for JMeter success.
>>
>>
>> Thanks
>>
>>
>> On Wed, Jul 19, 2017 at 2:16 AM, Emilian Bold <em...@gmail.com>
>> wrote:
>>
>>> Hello,
>>>
>>> I noticed ClassFinder while tweaking ActionRouter and I believe it should
>>> be replace with a proper service declaration and loading.
>>>
>>> I'm a fan of the Lookup API (see [1] [2]) which is a small standalone JAR
>>> used a lot in NetBeans.
>>>
>>> The standard ServiceLoader [3] would also be a better replacement.
>>>
>>> Some remarks:
>>>
>>> * ClassFinder is almost always called with JMeterUtils.getSearchPaths().
>>> This must be expected to be the class path actually.
>>> * many calls of ClassFinder do some filtering, excluding some
>>> implementations. This would be simple by just not registering those
>>> implementations as a service.
>>>
>>> With the ServiceLoader, ActionRouter would load commands with something
>>> like:
>>>
>>> > ServiceLoader<Command> commands = ServiceLoader.load(Command.class);
>>>
>>> An each command will have to be registered in META-INF/services (for Java
>>> 8) or in the module declaration (for Java 9).
>>>
>>> In NetBeans we have an annotation @ServiceProvider [4] which is simpler and
>>> behind the scenes the build system generates at build time the
>>> META-INF/services registration file.
>>>
>>> Note that service registration and loading would work the same for 3rd
>>> party JARs.
>>>
>>> The only downside is we have to think about older external JARs that expect
>>> the current behaviour. We could use some flag (in MANIFEST perhaps?) to
>>> differentiate between them.
>>>
>>> 1.
>>> http://bits.netbeans.org/8.0/javadoc/org-openide-util-
>>> lookup/org/openide/util/Lookup.html
>>> 2.
>>> http://bits.netbeans.org/8.0/javadoc/org-openide-util-
>>> lookup/org/openide/util/lookup/doc-files/lookup-api.html
>>> 3. http://download.java.net/java/jdk9/docs/api/java/util/
>>> ServiceLoader.html
>>> 4.
>>> http://bits.netbeans.org/8.0/javadoc/org-openide-util-
>>> lookup/org/openide/util/lookup/ServiceProvider.html
>>>
>>> --emi
>>>
>>
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.

Re: Replacing ClassFinder with ServiceLoader

Posted by Emilian Bold <em...@gmail.com>.
>  - Another critical part is to make it as easy as today to register a
>   plugin which is IMO one of the major reasons for JMeter success.

Do you mean easy to register a plugin in the UI or easy to develop a plugin?

>   - The backward compatibility with 3rd party plugins is a critical part.
>   IMO, no change should be made before validating new proposal will work with
>   most popular existing 3rd plugins.

I can imagine a scenario where old plugin JARs are inspected with the
current ClassFinder while for everything else we use ServiceLoader.

The idea being that the old style is deprecated and will be removed in
time. But we shouldn't use ClassFinder at all in JMeter internal code.

> Can you clarify:
> => "The only downside is we have to think about older external JARs that
> expect the current behaviour. We could use some flag (in MANIFEST perhaps?)
> to differentiate between them."

It would help to know if a plugin uses the ServiceLoader or not. One
trivial check would be to verify for the presence of
META-INF/services/. Another would be to invent a manifest flag,
perhaps "X-JMeter-API-level: 4" in the MANIFEST.MF

Markings in the GUI and showing warnings about plugins that don't keep
up with API changes would also be a good way to encourage them being
up to update.

> => "many calls of ClassFinder do some filtering, excluding some
> implementations. This would be simple by just not registering those
> implementations as a service."

ClassFinder calls usually include the "String notContains" filter.
Which is an odd way because callers shouldn't care what they get
except in very specific situations. If a "service" is registered
globally it will be there.

I assume filtering happens because you need some implementations
(like, a GUI Command) but there's no way to differentiate right now if
that implementation is for global public consumption or not. Is should
be generally up to the implementation class to declare itself a
service and not for the calling class to filter (because then, every
new subclass needs a decision about being filtered or not).

Also note that the Lookup API handles this global / labeled separation
with "named" lookups inside META-INF/namedservices/ [1]. This would
allow one to write
Lookups.forPath("GUI/Main/Menu").lookupAll(Command.class) and
Lookups.forPath("GUI/Report/Menu").lookupAll(Command.class) and get
the commands instances separately. But this is an advanced situation
-- seems to me JMeter never uses the filtered classes in some other
case, so it's just a way of 'hiding' them.

1. http://bits.netbeans.org/8.0/javadoc/org-openide-util-lookup/org/openide/util/lookup/Lookups.html#forPath(java.lang.String)

--emi


On Thu, Jul 20, 2017 at 10:20 PM, Philippe Mouawad
<ph...@gmail.com> wrote:
> Hello Emilian,
> Thanks for your analysis , and new ideas !
>
> Can you clarify:
> => "The only downside is we have to think about older external JARs that
> expect the current behaviour. We could use some flag (in MANIFEST perhaps?)
> to differentiate between them."
>
> => "many calls of ClassFinder do some filtering, excluding some
> implementations. This would be simple by just not registering those
> implementations as a service."
>
> My 2 cents with current (partial ?) understanding of your proposal:
>
>    - The backward compatibility with 3rd party plugins is a critical part.
>    IMO, no change should be made before validating new proposal will work with
>    most popular existing 3rd plugins.
>    - Another critical part is to make it as easy as today to register a
>    plugin which is IMO one of the major reasons for JMeter success.
>
>
> Thanks
>
>
> On Wed, Jul 19, 2017 at 2:16 AM, Emilian Bold <em...@gmail.com>
> wrote:
>
>> Hello,
>>
>> I noticed ClassFinder while tweaking ActionRouter and I believe it should
>> be replace with a proper service declaration and loading.
>>
>> I'm a fan of the Lookup API (see [1] [2]) which is a small standalone JAR
>> used a lot in NetBeans.
>>
>> The standard ServiceLoader [3] would also be a better replacement.
>>
>> Some remarks:
>>
>> * ClassFinder is almost always called with JMeterUtils.getSearchPaths().
>> This must be expected to be the class path actually.
>> * many calls of ClassFinder do some filtering, excluding some
>> implementations. This would be simple by just not registering those
>> implementations as a service.
>>
>> With the ServiceLoader, ActionRouter would load commands with something
>> like:
>>
>> > ServiceLoader<Command> commands = ServiceLoader.load(Command.class);
>>
>> An each command will have to be registered in META-INF/services (for Java
>> 8) or in the module declaration (for Java 9).
>>
>> In NetBeans we have an annotation @ServiceProvider [4] which is simpler and
>> behind the scenes the build system generates at build time the
>> META-INF/services registration file.
>>
>> Note that service registration and loading would work the same for 3rd
>> party JARs.
>>
>> The only downside is we have to think about older external JARs that expect
>> the current behaviour. We could use some flag (in MANIFEST perhaps?) to
>> differentiate between them.
>>
>> 1.
>> http://bits.netbeans.org/8.0/javadoc/org-openide-util-
>> lookup/org/openide/util/Lookup.html
>> 2.
>> http://bits.netbeans.org/8.0/javadoc/org-openide-util-
>> lookup/org/openide/util/lookup/doc-files/lookup-api.html
>> 3. http://download.java.net/java/jdk9/docs/api/java/util/
>> ServiceLoader.html
>> 4.
>> http://bits.netbeans.org/8.0/javadoc/org-openide-util-
>> lookup/org/openide/util/lookup/ServiceProvider.html
>>
>> --emi
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: Replacing ClassFinder with ServiceLoader

Posted by Philippe Mouawad <ph...@gmail.com>.
Hello Emilian,
Thanks for your analysis , and new ideas !

Can you clarify:
=> "The only downside is we have to think about older external JARs that
expect the current behaviour. We could use some flag (in MANIFEST perhaps?)
to differentiate between them."

=> "many calls of ClassFinder do some filtering, excluding some
implementations. This would be simple by just not registering those
implementations as a service."

My 2 cents with current (partial ?) understanding of your proposal:

   - The backward compatibility with 3rd party plugins is a critical part.
   IMO, no change should be made before validating new proposal will work with
   most popular existing 3rd plugins.
   - Another critical part is to make it as easy as today to register a
   plugin which is IMO one of the major reasons for JMeter success.


Thanks


On Wed, Jul 19, 2017 at 2:16 AM, Emilian Bold <em...@gmail.com>
wrote:

> Hello,
>
> I noticed ClassFinder while tweaking ActionRouter and I believe it should
> be replace with a proper service declaration and loading.
>
> I'm a fan of the Lookup API (see [1] [2]) which is a small standalone JAR
> used a lot in NetBeans.
>
> The standard ServiceLoader [3] would also be a better replacement.
>
> Some remarks:
>
> * ClassFinder is almost always called with JMeterUtils.getSearchPaths().
> This must be expected to be the class path actually.
> * many calls of ClassFinder do some filtering, excluding some
> implementations. This would be simple by just not registering those
> implementations as a service.
>
> With the ServiceLoader, ActionRouter would load commands with something
> like:
>
> > ServiceLoader<Command> commands = ServiceLoader.load(Command.class);
>
> An each command will have to be registered in META-INF/services (for Java
> 8) or in the module declaration (for Java 9).
>
> In NetBeans we have an annotation @ServiceProvider [4] which is simpler and
> behind the scenes the build system generates at build time the
> META-INF/services registration file.
>
> Note that service registration and loading would work the same for 3rd
> party JARs.
>
> The only downside is we have to think about older external JARs that expect
> the current behaviour. We could use some flag (in MANIFEST perhaps?) to
> differentiate between them.
>
> 1.
> http://bits.netbeans.org/8.0/javadoc/org-openide-util-
> lookup/org/openide/util/Lookup.html
> 2.
> http://bits.netbeans.org/8.0/javadoc/org-openide-util-
> lookup/org/openide/util/lookup/doc-files/lookup-api.html
> 3. http://download.java.net/java/jdk9/docs/api/java/util/
> ServiceLoader.html
> 4.
> http://bits.netbeans.org/8.0/javadoc/org-openide-util-
> lookup/org/openide/util/lookup/ServiceProvider.html
>
> --emi
>



-- 
Cordialement.
Philippe Mouawad.