event-loop.c changes

event-loop.c changes

[Elena Zannoni]

Jim, I am looking at some of the changes in event-loop.c (from Keith's
branch).

I am not sure about a couple of things:

This comment:

void
gdb_setup_readline (void)
{
  /* This function is a noop for the async case.  The assumption is that
     the async setup is ALL done in gdb_init, and we would only mess it up
     here.  The async stuff should really go away over time. */

  if (event_loop_p)
    {
[...]
    }
}

You meant 'sync' instead of 'async'?


The creation of these:
      gdb_stdout = stdio_fileopen (stdout);
      gdb_stderr = stdio_fileopen (stderr);
      gdb_stdlog = gdb_stderr;  /* for moment */
      gdb_stdtarg = gdb_stderr; /* for moment */

Is done in main.c, then again here, in gdb-setup-readline().
But a comment in gdb_disable_readline suggests that they really
shouldn't be created here at all.
     /* FIXME - It is too heavyweight to delete and remake these
         every time you run an interpreter that needs readline.
         It is probably better to have the interpreters cache these,
         which in turn means that this needs to be moved into interpreter
         specific code. */


Thanks
Elena

Re: event-loop.c changes

[Jim Ingham]

Elena,

On Wednesday, October 2, 2002, at 04:08  PM, Elena Zannoni wrote:

>
> Jim, I am looking at some of the changes in event-loop.c (from Keith's
> branch).
>
> I am not sure about a couple of things:
>
> This comment:
>
> void
> gdb_setup_readline (void)
> {
>   /* This function is a noop for the async case.  The assumption is 
> that
>      the async setup is ALL done in gdb_init, and we would only mess 
> it up
>      here.  The async stuff should really go away over time. */
>
>   if (event_loop_p)
>     {
> [...]
>     }
> }
>
> You meant 'sync' instead of 'async'?


Yup...

>
>
> The creation of these:
>       gdb_stdout = stdio_fileopen (stdout);
>       gdb_stderr = stdio_fileopen (stderr);
>       gdb_stdlog = gdb_stderr;  /* for moment */
>       gdb_stdtarg = gdb_stderr; /* for moment */
>
> Is done in main.c, then again here, in gdb-setup-readline().
> But a comment in gdb_disable_readline suggests that they really
> shouldn't be created here at all.
>      /* FIXME - It is too heavyweight to delete and remake these
>          every time you run an interpreter that needs readline.
>          It is probably better to have the interpreters cache these,
>          which in turn means that this needs to be moved into 
> interpreter
>          specific code. */

Theoretically, I think this is true.  But when we did some profiling of 
gdb under PB, this really didn't show up at all, and changing this 
didn't seem worth the effort.  This only happens when you switch 
interpreters, and that is a pretty heavy-weight operation anyway, so 
this is in the noise.

At the same time I was trying to get the interpreter stuff working, I 
was also fixing a bunch of bugs in getting a real asynchronous target 
to work (not just event_loop_p = 1, but with real async execution).  
One of the big problems was that if anything went the teeniest bit 
wrong, then the wrong stdio handler would be left in place and though 
in fact gdb was just stopped waiting for input, it thought it was still 
talking to the inferior, so it was effectively hung...  So I was very 
suspicious about anything that mucked with the input & outputs to gdb 
at the time, which was probably why I was so harsh on this...

But I got all the async stuff working without having to fix this, so...

Jim
--
Jim Ingham                                   jingham@apple.com
Developer Tools
Apple Computer