Re: PATCH: Circular trace buffers

[Pedro Alves <pedro@codesourcery.com>]

On Wednesday 17 March 2010 16:55:38, Stan Shebs wrote:
> Pedro Alves wrote:
> > On Tuesday 16 March 2010 21:42:02, Stan Shebs wrote:
> >   
> >> This patch adds a flag that requests the target agent to make the trace 
> >> buffer circular, so that instead of filling it up and then stopping, the 
> >> agent discards the oldest trace frames as necessary to accommodate new 
> >> ones.  Any hairy memory management code is going to be on the target 
> >> side; GDB just has to transmit the setting (and now always via target 
> >> vector), and report back status, which may now include a total number of 
> >> frames that were created.  This also adds complete documentation of the 
> >> qTStatus reply, per request.  Any comments before I commit?
> >>     
> >
> > Playing devil's advogate here, I'm still not 100% convinced that
> > "set circular-trace-buffer" is 100% well defined and that
> > is isn't confusing in some cases; it applies on the fly
> > in some cases, does somewhat not-completly clear
> > things in other cases, and errors out in others.
> Hey Joel, pass me that aspirin bottle, willya? :-)  This is one of those 
> places where I'd like more user feedback before getting too fancy.  It's 
> the opposite situation of expression evaluation - we play fast and loose 
> with language semantics, but we know from extensive experience that 
> users don't want GDB to be too pedantic about visibility, scopes, etc.  
> But for tracepoints we're still mostly guessing.

But I'm not proposing anything fancy, on the contrary, something like:

 (gdb) help set circular-trace-buffer
 Set target's use of circular trace buffer in the next trace run.
 Use this to make the trace buffer into a circular buffer,
 which will discard traceframes (oldest first) instead of filling
 up and stopping the trace run.

Instead of:

 (gdb) help set circular-trace-buffer
 Set target's use of circular trace buffer.
 Use this to make the trace buffer into a circular buffer,
 which will discard traceframes (oldest first) instead of filling
 up and stopping the trace run.

Which doesn't say anything about the setting applying immediately,
or when a new trace run is started.

And:

 (gdb) help show circular-trace-buffer
 Show target's use of circular trace buffer in the next trace run.
 Use this to make the trace buffer into a circular buffer,
 which will discard traceframes (oldest first) instead of filling
 up and stopping the trace run.

Instead of:

 (gdb) help show circular-trace-buffer
 Show target's use of circular trace buffer.
 Use this to make the trace buffer into a circular buffer,
 which will discard traceframes (oldest first) instead of filling
 up and stopping the trace run.


As is "show circular-trace-buffer" is pretty much useless.
Answer this question:  What does this mean, in all supported
cases?

 (gdb) show circular-trace-buffer
 Target's use of circular trace buffer is on.

It looks as though these commands help strings should be
improved to clarify exactly what it means.

> > I wonder
> > if we defined "set circular-trace-buffer" as another flag
> > that is respected at "tstart" time only, and made the
> > presently running trace run's circular-trace-buffer-ness reported
> > through "tstatus", and define "show circular-trace-buffer" as the
> > "circular-trace-buffer-ness" intent at next trace run start,
> > things would be more consistent and clear.

> I thought about that, but it seemed like one of its uses would be as a 
> hasty way to keep a trace run alive; you do a tstatus, say "oh sh*t" as 
> you see the buffer at 80% full before you've reached the code of 
> interest, and quickly switch to circular buffer.

... oh sh*t, I forgot to disable that tracepoint!  Oh darn, you can't
do that when the trace is running.  Same thing, same general problem,
it seems.  This special casing in the circularity-ness adds
inconsistency (everything else is set at tstart time) which I
suspect will byte back.  But it's fine.  I'll just refuse to
address any such inconstencies myself and push the problem
back to you when it happens.  :-)

> >  - all-stop/async + trace running + "set circular-trace-buffer"
> >    errors out because you can't talk to the target if it
> >    is running in all-stop.
> >   

> I think the user would know to interrupt the program, because there's no 
> prompt to type the command at?

Note: "async".  Frontends are switching to use async mode by
default.  "-gdb-set circular-trace-buffer on" does not work
in that case, only in non-stop mode.

> >  - E.g., what does "show circular-trace-buffer" mean when
> >    debugging a tfile?  "set circular-trace-buffer" changes
> >    the local GDB flag, and "show circular-trace-buffer"
> >    shows the according change, but, then we have no
> >    way of knowing when debugging a tfile had been
> >    in circular-trace-buffer mode or not when the tfile
> >    was created.
> >   
> You would know if circularity had kicked in because tstatus on the file 
> would show more frames created than were in the buffer.  If it hadn't 
> kicked in, then the flag's value wouldn't be of much interest, right?

- this shows that "show circular-trace-buffer" is useless.
- this requires users know that fact.
- this doesn't sound user friendly.

-- 
Pedro Alves