Hi, Am 12.10.2012 um 02:29 schrieb David Jencks:
> > On Oct 5, 2012, at 12:35 PM, Felix Meschberger wrote: > >> Hi, >> >> Am 03.10.2012 um 19:28 schrieb David Jencks: >> >>> I've had several ideas about DS enhancements, some of which I've >>> implemented, and would like some feedback about how desirable they are >>> before committing or proceeding with them. >>> >>> 1. (FELIX-3692) If you manually enable/disable components some of the >>> work gets done asynchronously. I propose an api for finding out whether >>> this work is done or waiting for it, something like >>> >>> boolean tasksCompleted(); >>> >>> void waitForTasksCompleted(); >>> >>> >>> on ScrService. (suggestions for better names welcome :-) One use would >>> be in our tests to replace the delay() call. >> >> -1 >> >> I think such information is without any value. And our own tests being the >> sole use cases is a bit weak to add API. > > The use cases I have are: > > 1. shutdown. We want to make sure the queue is empty before turning off > logging. (FELIX-3704) This does not need new external API. I think the actor's terminate method can do this before adding the "Terminator" task. > 2. You might want to install a lot of application deployer components and > make sure they are all fully started before starting say a file listener that > looks for applications to deploy. If some of the first set of components are > enabled through code, it's hard to tell if they are all completely started > without knowing exactly what they are. Similarly you might want to know if a > server is "fully started", i.e. all the components that are enabled have at > least tried to activate. This pretty much sounds like YAGNI. > > I'm not 100% sure this is a good idea, but I think it could have some use. Ok, then lets not do it right now. > >> >>> >>> 2. (FELIX-3557) There are several circumstances in which, as the spec >>> warns, you can't establish a circular dependency between components. In >>> some of these cases, the order in which the components are activated >>> determines whether all the references are established. This is hard to >>> understand from a users point of view :-). Sometimes it's possible to >>> detect these situations and establish the reference asynchronously. The >>> patch attached to the issue does this but needs a little more work to only >>> try with services from DS components. >> >> If I understand the issue/patch correctly, it does something like this: If >> an optional service cannot be eagerly bound (to call a bind method taking >> the service instance) due to a circularity (exception thrown from the >> BundleContext.getService method) it will not be bound but may later be bound >> if the actual service instance is created. Correct ? >> >> I think, this sounds good. > > OK, I'll commit it. >> >>> >>> For these two, I'm wondering if they would be useful enough to propose for >>> the DS 1.3 spec. >> >> Does FELIX-3557 really imply a spec change ? At most it might be kind of an >> implementation hint. > > If you don't think it needs a spec change I won't worry. > >> >>> >>> 3. (re-proposal) I'd like to propose moving the implementation to java 5 >>> again with generics etc. The last time I suggested this there was a lot of >>> pushback on the grounds that there are a lot of people using DS on limited >>> platforms. However, none of these alleged :-) people is using trunk, >>> because for several months the classes pulled from the concurrent library >>> were wrong and trunk just didn't run on pre-java-5 vms. Are the compendium >>> 4.3 spec classes we pull in even compatible with pre-java-5 vms? >> >> The longer I think about it, the more I have to admit that I agree.... > > I'm going to change this now because it will make some refactoring easier. > If people don't like it we can change back after I'm done. We should cut a release before refactoring ... A release is long overdue and has been prevented by work around concurrency issues. Regards Felix > >> >>> >>> 4. (radical idea I haven't tried yet) I'm becoming increasingly convinced >>> that the state objects in AbstractComponentManager mostly cause confusion >>> and make the code more complicated and less reliable. >> >> Well, these objects have been quite simple and easy to understand and worked >> perfectly (admittedly with some glitches). Now over time and patches >> applied, I agree they became quite complex and the concurrency behaviour >> became more complex. So for concurrency having immutable objects is a lot >> easier to handle than mutable ones. >> >>> The spec really only describes two states, enabled and disabled. The >>> variations on enabled -- whether the component has all its dependencies >>> satisfied, whether the service is registered, whether there are any >>> implementation objects created -- all seem somewhat orthogonal and depend >>> very much on the environment and don't seem to relate well to a single >>> "dimension" of state. I'm considering trying to refactor the code that >>> responds to outside actions (activate/deactivate and dependencies >>> appearing/disappearing) to be more "straight-through" with checks on the >>> specific aspects of state that they need. Possibly we would want to put >>> the "dynamic state" such as dependencies + instances in a single state >>> object, but this is a different approach to the current state objects which >>> have no internal state. >> >> I agree, that the states enabled and disabled might be confusing. But I am >> not sure, whether those are really states of component instances (or >> component configurations as the spec calls them). Rather they are states of >> the "abstract" component. >> >> The component instances on the other hand have states like unsatisfied, >> activating, active, registered, etc. >> >> In our implementation the component instance is represented by the >> AbstractComponentManager and its extensions while the "abstract" component >> is represented by combination of the ComponentHolder and the >> ComponentMetadata. > > I'm going to think about this some more and try some experiments. > > thanks for the comments! > david jencks > >> >> Regards >> Felix >
