[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [equinox-dev] OSGiServiceSupplier

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