[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [equinox-dev] OSGiServiceSupplier
|
I did think about using an Optional instead because then you could do a map with a function. However from a performance point of view this will result in additional garbage being created on the fly (I don't think the JIT will optimise all of them away at the moment)
Passing in a function to run on the service is one idea but in the case of debug options they are typically used to assign true/false values to the booleans in the class (in some cases, multiple). If it took functions then it would have difficulty in returning many of them: so you'd either have to pass in multiple steps in the function and return an array or call it multiple times (potentially regetting the service each time). Or you would pass in a function to assign multiple variables in the enclosing class and have the same synthesised accessors to set the values of those fields, as if they were being set in an inner class.
That said there doesn't have to be an either/or - the same class could implement a different method to allow a lambda to be passed through. Or a different implementation could be used that has similar ideas but with the passthrough.
The API was inspired by existing use cases in code that I'm going through. In those cases a lambda form probably wouldn't be useful.
Alex
Sent from my iPhat 6
> On 25 Sep 2015, at 23:17, Tom Schindl <tom.schindl@xxxxxxxxxxxxxxx> wrote:
>
> And with direct calling the method
>
>> package org.eclipse.fx.core;
>>
>> import java.util.function.BiFunction;
>> import java.util.function.Function;
>>
>> import org.eclipse.osgi.service.debug.DebugOptions;
>> import org.osgi.framework.BundleContext;
>> import org.osgi.framework.FrameworkUtil;
>> import org.osgi.framework.ServiceReference;
>>
>> public class OSGiServiceWrapper<S> {
>> private BundleContext ctx;
>> private BundleContext context;
>> private Class<S> serviceType;
>>
>> public OSGiServiceWrapper(Class<?> requestor, Class<S> serviceType) {
>> this.context = FrameworkUtil.getBundle(requestor).getBundleContext();
>> this.serviceType = serviceType;
>> }
>>
>> public static <S> OSGiServiceWrapper<S> create(Class<?> requestor, Class<S> serviceType) {
>> return new OSGiServiceWrapper<>(requestor, serviceType);
>> }
>>
>>
>> public <V,R> Function<V,R> createFunction(BiFunction<V, S, R> f) {
>> return (v) -> {
>> ServiceReference<S> reference = this.context.getServiceReference(this.serviceType);
>> if( reference != null ) {
>> S service = this.context.getService(reference);
>> try {
>> return f.apply(v,service);
>> } finally {
>> this.context.ungetService(reference);
>> }
>>
>> }
>> return (R)null;
>> };
>> }
>>
>> public <V,R> R call(V v, BiFunction<V, S, R> f) {
>> ServiceReference<S> reference = this.context.getServiceReference(this.serviceType);
>> if( reference != null ) {
>> S service = this.context.getService(reference);
>> try {
>> return f.apply(v,service);
>> } finally {
>> this.context.ungetService(reference);
>> }
>>
>> }
>> return (R)null;
>> }
>>
>> public static void main(String[] args) {
>> OSGiServiceWrapper<DebugOptions> options = OSGiServiceWrapper.create(OSGiServiceWrapper.class, DebugOptions.class);
>> Function<String, Boolean> f = options.createFunction( (o,s) -> {
>> return s.getBooleanOption(o, true);
>> } );
>> f.apply("Test");
>>
>>
>> options.call("Test", (o,s) -> {
>> return s.getBooleanOption(o, true);
>> });
>> }
>> }
>
> Tom
>
>> On 26.09.15 00:12, Tom Schindl wrote:
>> Alex,
>>
>> What about this:
>>
>>> public class OSGiServiceWrapper<S> {
>>> private BundleContext ctx;
>>> private BundleContext context;
>>> private Class<S> serviceType;
>>>
>>> public OSGiServiceWrapper(Class<?> requestor, Class<S> serviceType) {
>>> this.context = FrameworkUtil.getBundle(requestor).getBundleContext();
>>> this.serviceType = serviceType;
>>> }
>>>
>>> public static <S> OSGiServiceWrapper<S> create(Class<?> requestor, Class<S> serviceType) {
>>> return new OSGiServiceWrapper<>(requestor, serviceType);
>>> }
>>>
>>>
>>> public <V,R> Function<V,R> createFunction(BiFunction<V, S, R> f) {
>>> return (v) -> {
>>> ServiceReference<S> reference = this.context.getServiceReference(this.serviceType);
>>> if( reference != null ) {
>>> S service = this.context.getService(reference);
>>> try {
>>> return f.apply(v,service);
>>> } finally {
>>> this.context.ungetService(reference);
>>> }
>>>
>>> }
>>> return (R)null;
>>> };
>>> }
>>>
>>> public static void main(String[] args) {
>>> OSGiServiceWrapper<DebugOptions> options = OSGiServiceWrapper.create(OSGiServiceWrapper.class, DebugOptions.class);
>>> Function<String, Boolean> f = options.createFunction( (o,s) -> {
>>> return s.getBooleanOption(o, true);
>>> } );
>>> f.apply("Test");
>>> }
>>> }
>>
>> Tom
>>
>>> On 25.09.15 23:42, Alex Blewitt wrote:
>>> BTW I updated the examples in my blog post to use AutoClosable and have
>>> a finalize method. So Tom’s comments below reflected the version before
>>> I made those changes.
>>>
>>>> On 25 Sep 2015, at 22:20, Alex Blewitt <alex.blewitt@xxxxxxxxx
>>>> <mailto:alex.blewitt@xxxxxxxxx>> wrote:
>>>>
>>>> Thanks for the feedback. I didn’t claim that they followed best OSGi
>>>> practice but you make valid points about the ServiceTracker not being
>>>> closed. A finalize method will help with that, though will defer the
>>>> cleanup to a different part.
>>>>
>>>> The problem, of course, is that the java.util.function.Supplier has no
>>>> way of codifying a ‘close’ type resource (though perhaps this might be
>>>> modified if the tracker variant implements AutoClosable?).
>>>>
>>>> However, there’s a lot of cruft in Eclipse generally that are doing
>>>> this kind of pattern anyway. For example, the ContextsActivator
>>>>
>>>> https://github.com/eclipse/eclipse.platform.runtime/blob/master/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/osgi/ContextsActivator.java
>>>>
>>>> sets up a debug tracker and starts/stops it in the Activator (which
>>>> I’m trying to remove). Yet the code only gets used in DebugHelper:
>>>>
>>>> https://github.com/eclipse/eclipse.platform.runtime/blob/master/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/DebugHelper.java
>>>>
>>>> and even then it only calls it in a static block, once.
>>>>
>>>> So what’s happened here is:
>>>>
>>>> * Someone read up on ‘best practices’
>>>> * They set up a tracker
>>>> * They initialised it in the start and stop in the activator, as best
>>>> practice suggests
>>>> * They used it in an irrelevant way because that was the easiest way
>>>> to get the service
>>>>
>>>> I argue that in this specific case, it’s better to perform a one-off
>>>> lookup of the service instead of keeping a tracker for evermore:
>>>>
>>>> https://git.eclipse.org/r/#/c/56739/1/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/DebugHelper.java
>>>>
>>>> (Subsequent patch removes the class completely because it’s never used …)
>>>>
>>>> I argue that acquiring a service with a Supplier moves the
>>>> implementation from how to pick up the service into the implementation
>>>> instead of the class. In this case, the single-shot approach can be used.
>>>>
>>>> On the other hand, something like FrameworkLog is probably something
>>>> to keep around for a while instead of looking up each time.
>>>>
>>>> https://git.eclipse.org/r/#/c/56582/2/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/osgi/LogHelper.java
>>>>
>>>> These are in static references already (mainly to avoid the problems
>>>> with looking up services the whole time). These methods are repeated
>>>> time and time again throughout the Eclipse codebase. Yes, you can
>>>> argue it’s not best practice, and yes you can argue that DS is
>>>> probably better in most cases. But I can only delete code so quickly :)
>>>>
>>>> On the plus side, I am making progress in such situations - for
>>>> example, I decoupled static references from contenttype and moved from
>>>> bundle-activator lookups to references passed in from declarative
>>>> services as well.
>>>>
>>>> https://git.eclipse.org/r/#/c/54290/
>>>> https://git.eclipse.org/r/#/c/54257/
>>>>
>>>> I agree that the get/unget combo will cause oscillations; but the
>>>> options are between going from zero to one and to zero again, or going
>>>> from zero to one and leaking that way.
>>>>
>>>> In summary;
>>>>
>>>> * Yes, they could be improved
>>>> * No, they don’t exhibit best OSGi practices
>>>> * This code is being used in Eclipse platform whether or not there is
>>>> a standard implementation for the above or not
>>>> * They are intended to replace patterns where existing services are
>>>> kept for the lifetime of the bundle anyway
>>>> * For one-shot services (such as debugoptions usecases) a
>>>> servicetracker is probably overkill anyway
>>>> * For ongoing or frequent uses, the servicetracker option will
>>>> probably be the best. Such references are (commonly) stored in static
>>>> variables at the moment, which will be coupled with the lifetime of
>>>> the class
>>>>
>>>> I wonder how much of the above could be mitigated with the appropriate
>>>> documentation covering when and how they should be used.
>>>>
>>>> Alex
>>>>
>>>>> On 25 Sep 2015, at 21:45, Thomas Watson <tjwatson@xxxxxxxxxx
>>>>> <mailto:tjwatson@xxxxxxxxxx>> wrote:
>>>>>
>>>>> I have some issues with both examples you gave. For the code that
>>>>> uses ServiceTracker it will end up creating a ServiceTracker and
>>>>> never closing it which will leave a ServiceListener leaked
>>>>> (registered) with the framework for the lifetime of the active
>>>>> bundle. Once the bundle is stopped the leaked listeners will finally
>>>>> get cleared. So if you use the OSGiTracker in a short lived object
>>>>> that gets created often then you will quickly grind the service event
>>>>> bus of the framework to a halt. You may try to limit the damage of
>>>>> that leak by having a finalize() method that closes the tracker, but
>>>>> usually finalize() methods are not recommended best-practice.
>>>>>
>>>>> The OSGiSupplier is better but it has the unfortunate side-effect of
>>>>> ungetting the service object before even returning it to the client
>>>>> code for its first usage. In general this is not using good
>>>>> practices in OSGi because the service registration may be using a
>>>>> ServiceFactory that needs to track the state of the using bundles.
>>>>> Such usage on a ServiceFactory registration will cause the service
>>>>> usecount for the bundle to osilate between one and zero within the
>>>>> get() method. Each time the usecount goes to zero the service object
>>>>> is thrown away by the framework, then on next usage the service
>>>>> object will have to be recreated by the factory. This could result
>>>>> in a performance issue if the creating of the service object is
>>>>> expensive.
>>>>>
>>>>> The classes may have their uses in specific cases and not cause
>>>>> issues if used in a specific way. For example, if you know for a
>>>>> fact that the service you are getting is not stateful and does not
>>>>> use a ServiceFactory. Or if you make sure to use the OSGiTracker in
>>>>> an object that has a one-to-one relationship with the active
>>>>> lifecycle of the bundle. I am not sure they belong down in Equinox
>>>>> though because I do not believe they promote best-practices when
>>>>> dealing with the OSGi service registry.
>>>>>
>>>>> I also wonder if the latest DS specification helps deal with some of
>>>>> the short-comings you mention in your blog.
>>>>>
>>>>> Finally I do thank you for proposing a solution to a problem and
>>>>> bringing it here for discussion, I just don't feel comfortable with
>>>>> the current solution :-)
>>>>>
>>>>> Tom
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> From: Alex Blewitt <alex.blewitt@xxxxxxxxx
>>>>> <mailto:alex.blewitt@xxxxxxxxx>>
>>>>> To: Equinox development mailing list <equinox-dev@xxxxxxxxxxx
>>>>> <mailto:equinox-dev@xxxxxxxxxxx>>
>>>>> Date: 09/25/2015 01:33 PM
>>>>> Subject: [equinox-dev] OSGiServiceSupplier
>>>>> Sent by: equinox-dev-bounces@xxxxxxxxxxx
>>>>> <mailto:equinox-dev-bounces@xxxxxxxxxxx>
>>>>> ------------------------------------------------------------------------
>>>>>
>>>>>
>>>>>
>>>>> I posted on
>>>>> _http://alblue.bandlem.com/2015/10/osgi-services-in-java-8.html_earlier
>>>>> today about using Java 8’s Supplier to acquire a service on-demand
>>>>> without having to do expensive client-side coding, and that would
>>>>> fail fast in the absence of any OSGi classes and return null in such
>>>>> situations.
>>>>>
>>>>> I’d like to contribute this to Eclipse so that it can be used in
>>>>> places where Declarative Services aren’t the right solution
>>>>> (specifically; for integrating in where places have static Log or
>>>>> DebugOptions classes).
>>>>>
>>>>> Which would be the right project/bundle to contribute this into?
>>>>> Clearly it could go into something like Platform or Core.runtime, but
>>>>> I wondered if it might be sensible to have this in equinox.util or
>>>>> equivalent.
>>>>>
>>>>> Alex_______________________________________________
>>>>> equinox-dev mailing list
>>>>> equinox-dev@xxxxxxxxxxx <mailto:equinox-dev@xxxxxxxxxxx>
>>>>> To change your delivery options, retrieve your password, or
>>>>> unsubscribe from this list, visit
>>>>> https://dev.eclipse.org/mailman/listinfo/equinox-dev
>>>>>
>>>>> _______________________________________________
>>>>> equinox-dev mailing list
>>>>> equinox-dev@xxxxxxxxxxx <mailto:equinox-dev@xxxxxxxxxxx>
>>>>> To change your delivery options, retrieve your password, or
>>>>> unsubscribe from this list, visit
>>>>> https://dev.eclipse.org/mailman/listinfo/equinox-dev
>>>
>>>
>>>
>>> _______________________________________________
>>> equinox-dev mailing list
>>> equinox-dev@xxxxxxxxxxx
>>> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
>>> https://dev.eclipse.org/mailman/listinfo/equinox-dev
>
>
> --
> Thomas Schindl, CTO
> BestSolution.at EDV Systemhaus GmbH
> Eduard-Bodem-Gasse 5-7, A-6020 Innsbruck
> http://www.bestsolution.at/
> Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck
> _______________________________________________
> equinox-dev mailing list
> equinox-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://dev.eclipse.org/mailman/listinfo/equinox-dev
