Re: [PATCH v2 05/25] Make the intepreters output to all UIs

[Pedro Alves <palves@redhat.com>]

On 03/22/2016 09:33 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
> Hi Pedro,
> We have three interpreters, cli, mi and tui.  Each of them has a
> ${inter}_on_normal_stop registered to normal_stop observer, as below,
> 
> Why don't we have 'struct ui' have a method 'print_stop_event', so that
> we can put the different handling there.

That's what I tried in an earlier attempt as I tried to describe in
the commit log:

    An earlier prototype of this series did the looping over struct UIs in
    common code, and then dispatched events to the interpreters through a
    matching interp_on_foo method for each observer.  That turned out a
    lot more complicated than the present solution, as we'd end up with
    having to create a new interp method every time some interpreter
    wanted to listen to some observer notification, resulting in a lot of
    duplicated make-work and more coupling than desirable.

I have an earlier version of the series on my github that goes all
the way to make it possible to have multiple independent TUIs active.
And then the TUI has even more random observers.  It turned out to be
a huge mess.

So even though this particular observer could seemingly be handled
like that, there's a larger pattern here that applies to all
observers all interpreters use.

I think that if we do switch to the mechanism of calling a
"virtual" method directly, it'll be by calling into the
method on the UI's top level interpreter directly.

A wrinkle is that some interpreters install an observer than
works with the current interpreter, while others install an
observer that works with the top level interpreter.  That should
probably be changed to always dispatch through the top level
interpreter.  I tried that too, but, it was getting unwildly,
and decided to back off.  Yet another large-ish change for another
day.  I suspect it'll be easier in C++.

But even that isn't clear to me is the best approach.  The
lose coupling between common code and the interpreters provided
by the observers is quite convenient.  I think of it as the
common code posting an event, and then the interpreters consuming
it.  I thought of making it possible to associate a "data" pointer
with each observer, thus have each UI instance register itself
as an observer for a particular event, instead of registering
an observer for the whole set of UI instances of a type of UI
and then have each observer iterate over all instances.
But then that would be painful to do with the current observers
design, it'd much much more natural in C++11 with lambdas or
some such...

>  In this way, we can only
> register one to normal_stop observer, in interps.c for example, and the
> code can be simplified, like this,
> 
> static void
> interp_on_normal_stop (struct bpstats *bs, int print_frame)
> {
>   struct switch_thru_all_uis state;
> 
>   SWITCH_THRU_ALL_UIS (state)
>     {
>        current_ui->print_stop_event (bs, print_frame);
>     }
> }
> 
> we don't have to check the type of interpreter in the loop anymore.
> 

I'd much prefer to go the direction of calling into the top interpreter,
instead of adding virtual methods to the UI:

   SWITCH_THRU_ALL_UIS (state)
     {
        current_ui->top_level_interpreter->print_stop_event (bs, print_frame);
     }

Because an UI has-a interpreter, not is-a one.

But I've tried this, and ended up with too-ugly template-like
macros and too much boilerplace, and lots of duplication in
in UI's interface, and I never managed to make it regression free.

Thanks,
Pedro Alves