Re: Multiple lifecycles per container

I really don't know where to start for an incremental patch. It'll
basically require me to do it all over again from scratch with double
the work to make the whole thing work during the increments. I basically
saw how it worked at the time, then how I thought the design was heading
to and what was most intuitive, did a code reshuffle, added fixes for
problems as I came across solutions, then ironed out the problems during
testing. I'd be willing to add extra documentation, write something up
in xdoc, add a few diagrams.

Just apply a patch to your working code and follow it through. You'll
see its cleaner and pick it up quickly.  I've added comments in the
source so there shouldn't be to much confusion. I'm not that smart so I
haven't done anything really tricky. Most interesting methods are
ComponentRepository.lookup(..) and ComponentRepository.initialize().

I think just applying the patch and carrying on from there is the most
productive route. It adds the extra features wanted and  fixes a number
of the issues, all for little work on your part. We could try to apply
it incrementally but is that the most productive? The code base is
fairly small so unintended consequences are minimal, all the current
tests pass (bar the ComponentRepository.getConfiguredComponent(), but
for obvious reasons), the end result is most likely going to be very
similar, so why bother?

If the worry is about breaking things which the tests won't pick up on,
then more tests should be written. After all, that's the whole point. 

You could also start a new branch in cvs and have folks play with that
for a while if you're really worried. 

Looking at the patch it seems quite complicated, however in concept it's
quite simple. There's been some decoupling which should make it easier
to understand. 

Essentially the component repository no longer handles any lifecycles
for components. It's job is just to load the InstanceManagers,
LifecycleHandlers, and ComponentDescriptors. It makes the
LifecycleHandlers available for general use (still need work to make
these immutable when passed out, don't want naughty components
added/removing phases). Upon component request it looks for the instance
manager handling the requested component (InstanceManager per class), if
the manager isn't instantiated it does so, then calls getComponent() on
the manager and passes the component back. When the component is
released, it again looks for the InstanceManager and calls release(..).
The changes put the onus on the InstanceManager to decide on component 
lifecycle handling, which also simplifies the component repository.

The instanceManager can decide on whether to dispose of components
immediately , pool them, or whatever. Basically the same as you had
before, except it now also handles release(..). It also asks the
ComponentRepository for the lifecycleHandler for the component (in the
component configuration)

That's the concept, stuff below is what makes it happen:

----

In the component repository the old mapping between component instance
==> ComponentHousing has been changed to component instance ==>
InstanceManager to facilitate finding the InstanceManager when
releasing. I've also added a lock object (lookupLock) in here to ensure
that when new InstanceManagers are created that two separate threads
don't create the same manager. Shouldn't drag down performance as an
InstanceManager is only created once per component class. Separate
object as we don't know what's locking on the ComponentRepository.

Added a LoggerManager to the component repository to be used for
component logging

The InstanceManagers hold the ComponentHousing and  track component
connections, and ask the component repository for the LifecycleHandler
it wishes to use (specified in the ComponentDescriptor). The instance
manager is responsible for complete handling of the component lifecycle.
The component repository no longer does this. While writing this I think
that the ComponentHousing can be removed and just use the
ComponentDescriptor and InstanceManagers directly, ditto with the
ComponentManager (just common code in here. Just put it in the
AbstractInstanceManager.)


The LifecycleHandlerHolder performs a similar role to the
ComponentDescriptor. Holding a LifecycleHandlers configuration, instance
etc.. hmmm, I should of called it LifecycleHandlerDescriptor. The
component repository uses this to create a new LifecycleHandler.

Because the lifecycle stuff is no longer passed to the component
repository by the container, the container is also affected. Just
removed the lifecycle stuff. Also there was a problem with the container
obtaining components on startup (load-on-start), but never releasing
them. Now they are released straight away. This was I think the cause of
an extra instance being created. If one wants a single instance to hang
around, then just use a different InstanceManager( eg
KeepAliveSingletonInstanceManager). The container only starts the
component repository. 

ComponentHousings don't track connections, InstanceManagers do.

Not even sure we should keep the ComponentManager. Just use the
InstanceManagers directly. Adds an extra layer of complexity for no
gain.

By plugging in an InstanceManager which tracks component connections
pete's problem with singletons go away.


------

Not sure really how the above can be applied incremenally, as it affects
nearly everything. The stuff below possibly could.

------

Added extra logging statements at various locations to track what's
going on. Hardly any previously, making it hard to find bugs.

Modified LogEnablePhase to name the logger by ComponentClass.

AvalonServiceSelectorTest used a deprecated method, getComponent(..),
changed it to lookup(..).

PlexusLoaderServlet didn't interpolate my configuration file with
'plexus.home' so I added that to the container context.

SingltonInstanceManager renamed to ClassicSingletonInstanceManager, as
there's a new one to keep an instance alive, which seems more like
expected behaviour (like the old turbine).
ClassicSingletonInstanceManager is still the default.


DefaultPlexusTest has been updated with the multi thread singleton test
(one instance for ever). Had to add a new ServiceG along with changes to
the test configuration.

Added  a Tracer class as a convenience to tack stackTraces to exception
messages for exceptions which don't take in a  cause.

I've also added a ThreadSafeMap (with TestCase, could be fleshed out),
to allow multiple synchronized reads, locking writes. Only useful with
lots of reads, little writes. This is used in a few locations like the
Map holding the InstanceManagers in the ComponentRepository. Here
InstanceManagers are looked up very often, but only created once, so a
synchronized map would  slow things down. Before the whole component
repository was not threadSafe, as un-synchronized HashMaps were used in
areas with possible concurrent reads/writes. Works on put/add/remove.
Iterators require a bit more thought (possibly by using a copy of the
Map). I still use arrays instead.

Added a TestThreadManager, AbstractTestThread used in testing threading
issues. 

Added a SweeperPool (with TestCase) for the PoolableInstanceManager for
true pooling support. Needed to add configuration support to the
InstanceManagers to allow configuration of the pool.

Some more cleanup can be performed, like dropping the ComponentManager,
ComponentHousing, renaming the LifeCycleHandlerHousing to
LifecycleHandlerDescriptor, modify the LifecycleHandler to deal directly
with the InstanceManager instead of going through the
ComponentHousing.getComponentManager() to reduce coupling even further.


Also added documentation changes which also made it bigger, but by that
time I'd was already making changes to the workings. I think a few
unintentional formatting changes were also made by my code formatter, so
apologies if that's added to the patch size, though I tried to fix these
again when making the patch.

I know it's a big patch, bigger than I originally thought, but hey
that's life. Call it punctuated evolution of plexus.

You can of course not bother with it and do all that work yourself. It's
yours if you want it, if not, that's fine too, thought it would be nice
to share the upgrade.

Let me know what you think. I'd be happy to help out but I really don't
like doing things twice.

The patch should be for current HEAD. Give it a whirl, you'll be glad
you did :)

Cheers,
-bert


On Sun, 2003-07-27 at 10:51, Jason van Zyl wrote: 
> On Fri, 2003-07-25 at 00:22, Bert van Brakel wrote:
> > I've just put it into jira. Diffed off the cvs HEAD at round about 1600
> > NZ (GMT +12).
> > 
> > I reproduce what I put in there, and attached the patch:
> > ==============
> > 
> 
> Well I can work with you to take pieces out of this patch but it's just
> too big to integrate in one shot.
> 
> How about we start simple with Pete's notes on the singleton problems
> and getting that in? I think that would be fairly easy and get us
> started.