Re: async-calls and Dispatchers

On Fri, 2007-12-21 at 09:00 +1100, Robert Collins wrote:

> Please do read the
> 'proactor' ACE pattern if you haven't.
> http://www.cs.wustl.edu/~schmidt/PDF/proactor.pdf

I only had time to skim that paper, but I think what they call
Completion Dispatcher is what we call "comm". Their Completion
Dispatcher class is I/O-aware and converts I/O notifications (possibly
supporting various models) into callbacks to the core.

I see no connection between CompletionDispatcher in Squid and the
Completion Dispatcher in that article, except for the name.

> As for concrete reasons to have dispatcher and engine separate:
> - code reuse (all engines of a similar type/behaviour can be given the
> same dispatcher)
> - scaling (as we start working towards multiple core in the future a
> single engine will correctly dispatch back to the originating thread or
> process because the dispatcher instance given is for that thread)
> - loose coupling - we can experiment with different async call
> mechanisms without having to do an all-at-once conversion, by just
> adding a different dispatcher instance for the code that needs that.

The above does not explain to me why you need both classes. You can do
all of the above with the current AsyncEngine interface alone. Perhaps
you assign some unimplemented semantics to Dispatchers and we end up
discussing two different things: you talk about the intended/future API,
while I talk about the deleted code.

> But looking at the patch I can see that all you've done is move the
> abstract interface one level further out. So the entire discussion is
> really moot - I don't care if its a level further out or not, though I
> think the extra indirection you are adding via
> AsyncCallQueue::Instance().fire() is more complexity not less.

I do not know what interface levels are here, but I am glad you do not
have a problem with deleting CompletionDispatcher classes after all.

> I presume you've made the signal handler dispatcher into an engine, or
> will do so.

Already done.

> More importantly I can't see any unit tests. If we want to end up with a
> rigourously tested code base - which is both well worth doing and
> something I thought we as a group were interested in - replacing tested
> code with untested code is a step backwards.

We are not moving in just one dimension here. I agree that rigorously
tested code base is a goal. Yes, we will probably lose some old unit
tests while simplifying the code. Better code and/or new tests will
probably compensate for that.

> > > Note also that the
> > > current design was in the process of removing global state to make
> > > testing and modularisation easier, so please do take care not to
> > > reintroduce dependencies on global variables etc.
> >
> > From that point of view, we are essentially removing things here, not
> > adding new ones so modularization can only improve. As for unit testing,
> > if it makes the actual code a lot more complex, it is doing more harm
> > than good.
>
> I'm not sure if you are saying that you are adding global variables
> because its 'simpler' (its not, and I'm *really* against more
> globals)

I do not recall adding global variables. The old code just hid them
behind register(Instance()) calls. We can quickly hide them again if
needed.

Alex.
Received on Thu Dec 20 2007 - 22:30:48 MST