Mailing-List: contact dev-help@cocoon.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cocoon.apache.org
Received-SPF: neutral (asf.osuosl.org: local policy)
Message-ID: <44677EFD.4070602@apache.org>
Date: Sun, 14 May 2006 21:03:25 +0200
From: Sylvain Wallez <sylvain@apache.org>
User-Agent: Thunderbird 1.5.0.2 (Macintosh/20060308)
MIME-Version: 1.0
To: dev@cocoon.apache.org
Subject: Re: CallFunctionNode problems in trunk
References: <446473E9.6060207@apache.org> <44648C37.60101@apache.org>
 <446491B2.8040009@apache.org> <446493A9.6080509@apache.org>
 <4464961C.9010207@apache.org>
 <Pine.LNX.4.64.0605121615070.27721@lapgp.otego.com>
 <44649A7E.6080704@apache.org> <4465D818.6010302@apache.org>
 <44670393.4080700@apache.org> <44674191.7050103@apache.org>
In-Reply-To: <44674191.7050103@apache.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Reinhard Poetz wrote:
> Carsten Ziegeler wrote:
>> Reinhard Poetz schrieb:
>>
>>> Carsten Ziegeler wrote:
>>>
>>>
>>>> Ok, so let's change the strategy and fix the standalone service
>>>> selector... :) If svn works again I can do this on monday evening - if
>>>> noone beats me to it.
>>>
>>> I didn't fix the StandaloneServiceSelector but replaced it with a
>>> factory. Basically the factory works for me and the code is very
>>> simple. Exception handling needs to be improved but that's the next
>>> step.
>>>
>>> Could somebody verify and review the attached patch?
>>>
>>
>> I'm not sure but I think your solution is the opposite of the current
>> problem: you are always creating a new instance, so thread safe
>> components are not handled correctly.
>
> Right, but that's easily fixed (see
> http://people.apache.org/~reinhard/ProcessingNodeBuilderFactory.java).
> I would prefer getting rid of the StandaloneServiceSelector as it is
> everything else than a simple class for a simple task and why do all
> those builders need to be components? I know there are historical
> reasons but do we need them to be components _now or in the future_?

As the original culprit, let me expand on the historical reasons. The
TreeProcessor was written during the winter of 2000/2001 at a time where
we were discussing flowmaps. There was a need for an extensible pipeline
language so that we could experiment new approaches. That's why the
TreeProcessor was designed around ProcessingNodeBuilders as components.

There are two different kinds of processing nodes: those that do not
need to know any other node except their children, and those that need
to be linked to other parts of the tree (making it actually a graph) to
call resources and flow, and also to call views in pipeline statements.

Two important concerns in the design were speed and strong separation
between build-time and run-time structures. So linking isn't a concern
of the node itself, but a concern of its builder. A linked node builder
therefore has to keep a reference to the node it has created, and is
therefore stateful, whereas other builders are completely stateless and
are pure factories (create a node and forget about it).

Using ThreadSafe ensures that only one instance of a stateless builders
exists, therefore speeding up the sitemap creation process. Linked
builders, being stateful, are not ThreadSafe so that one instance is
created anew for each node. Since Avalon considers no lifestyle
interface as being the same as SingleThreaded, the LinkedNodeBuilders
had no particular marker interface, but SingleThreaded would have
produced the exact same result.

So, as a conclusion:
- making NodeBuilders ThreadSafe is an optimization thing,
- LinkedNodeBuilders are statefull and _must_ be created a new for each
node,
- all these builders are linked to a particular TreeBuilder (see
setBuilder(TreeBuilder)).

> Anything else that could be a problem with my implementation?

I basically looks good, except:
- you should swap the test for Configurable and LogEnabled (this one
should come first)
- in 2.1 a AbstractProcessingNodeBuilder is Composable. In trunk, it
gets the ServiceManager from the builder, which is good also (and BTW,
why does Spring's bean factory appear here? It would be better to be in
an Avalon-only world in this part).

Sylvain

-- 
Sylvain Wallez - http://bluxte.net