Snappy and compression abstraction

[Zack Rusin]

CC'ing the mailing list.

On Tuesday, August 23, 2011 09:06:24 AM you wrote:
> On Tue, Aug 23, 2011 at 12:14 AM, Zack Rusin <zack at kde.org> wrote:
> > On Monday, August 22, 2011 06:38:56 PM you wrote:
> >> - on a signal, the sighandler needs to flush everything to disk -- it
> >> can't delay anything
> > 
> > Unfortunately if we want the traces to be complete it has to delay. The
> > initial implementation I had would just flush everything but the
> > resulting traces were always broken.
> > 
> > That's because the odds that CTRL-C is hit exactly between calls are
> > almost non-existant, we're essentially always in some call so just
> > flushing the data moves us from:
> > - inconsistent data
> > to
> > - possibly bit more inconsistent data
> > I didn't think that was an improvement so that's why I've added the delay
> > to guarantee consistent data.
> > 
> > So either we delay or we keep producing traces that have the last call
> > broken.
> 
> Half (incomplete) calls are OK. That's the same thing one would likely
> get till date on master too -- the parser will detect the half call,
> output a warning and then drop the call.
> 
> Note that it often a crash happens inside GL driver, which invariably
> leaves the entry call incomplete, and that's the most important use
> case -- to be able to see all calls up to a crash. Trying to get
> Ctrl-C happens between calls is, by comparison, a minor issue.

Yea, that makes sense. If we're ok with CTRL-C breaking the last call then the 
POSIX code is ready, I'll just have to flush on SIGSEGV as well. I'll look at 
windows tonight and I think we'll be ready to merge.

> BTW, it would be nice to show the incomplete calls on the gui too
> eventually.

Yea, I gotta do that.

> >> - it should not close anything on a signal -- it's quite possible the
> >> signal is later handled by a different handler (set by the application
> > 
> > Yea, that makes sense.
> > 
> >> - There is no need to bother the GUI/tools with signals handlers /
> >> mutex /flushes /etc -- only the wrapper DLLs need it . I 've already
> >> cleanup this up a bit on master, in anticipation to your work, so it
> >> would be better for you to rebase this on master.
> > 
> > Agreed, the current implementation doesn't do that. I've seen your
> > commit, I wanted to ask you how does LocalWriter supposed to be
> > different from the Writer.
> 
> Writer is the mean and lean writer. No multi-threading protection, flushes,
> etc.
> 
> LocalWriter is the one meant to be used by the wrapper DLL: it will
> use a mutex, and (on master) will flush after every call. The idea is
> that LocalWriter is the one that should install a sighandler to flush
> on a signal.

Alright, I'll try to merge this code.

> LocalWriter is always used as a static singleton, which may help.

Unfortunately the default behavior for signals is to terminate the process 
without calling the descrutors of statically allocated objects so we still 
need to catch and handle them.
 
> >> I
> >> think that if we tweak the snappy binary representation so that we can
> >> read random position with a mere index (basically, for each call
> >> maintain the file offset of the snappy block where it lives, plus the
> >> offset inside the block) then we can do. Then, what we really need is
> >> a version of Trace::Parser which keeps track of those offsets, and
> >> allows both linear/random iteration of the trace by maintaining a
> >> cache.
> > 
> > Yea, I thought about it as well. There's also a question of whether we
> > want to allow on-demand loading from gzip or snappy traces. Also bzip is
> > a lot better when it comes to random seeking.
> > But for now I was thinking about exactly the same solution as you. I was
> > wondering whether it's better to adjust the representation to store call
> > locations or whether to have Trace::Parser parse the file once and store
> > the index in memory (the latter could work with any trace file, but of
> > course takes up RAM).
> 
> The index should either be in memory or in a separate file (e.g.,
> xxx.trace.idx). I don't think that there's any benefit in putting this
> the actual trace file.

Well, it makes distrubution a lot easier.

> The best approach depends on how you want the gui to operate: e.g., if
> you want the gui to operate by having only one frame open at a time,
> then we could index only at frames, and ensure the cache is big enough
> for the current frame.

The ideal implementation is described in:
https://github.com/apitrace/apitrace/issues/36

Basically the GUI first needs to show the list of expandable frames. So it 
needs to know how many frames to show. Then when a user clicks on a frame the 
GUI goes and asks the parser for all the calls within that frame. 
Ideally the parser would have a cache holding, lets say between 10 and 100 
frames. So if someone in the GUI clicks on Frame 1 then Frame 2 and then Frame 
1 again, Frame 1 isn't parsed but just fetched from the cache.
I think that's the ideal scenario.

z