jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emilian Bold <emilian.b...@gmail.com>
Subject Re: Replacing ClassFinder with ServiceLoader
Date Thu, 20 Jul 2017 21:58:04 GMT
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 <emilian.bold@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
> <philippe.mouawad@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 <emilian.bold@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.

Mime
View raw message