Re: [PATCH] perf tools: fix write_event()

[Arnaldo Carvalho de Melo <acme@infradead.org>]

Em Wed, Jan 20, 2010 at 03:37:52PM +0100, Peter Zijlstra escreveu:
> On Wed, 2010-01-20 at 12:26 -0200, Arnaldo Carvalho de Melo wrote:
> 
> > > Can't report/archive/etc.. sort all this out?
> > 
> > We need to record the buildids at record time, preferrably at
> > PERF_RECORD_MMAP time, from the kernel, for long running, low frequency
> > sessions were DSOs may be updated.
> > 
> > Or for when a developer is updating his binary and doing perf record
> > runs to then use perf diff:
> > 
> > 	perf record ./myapp
> > 	vi myapp.c
> > 	make
> > 	perf record ./myapp
> > 	perf diff
> 
> > Works now because the first 'perf record' saved a copy of myapp so that
> > 'perf diff', finding the right build-id in ~/.debug/.build-id/ can do
> > the right thing.
> > 
> > But till we send the buildid in an extended PERF_RECORD_MMAP event sent
> > from the kernel, I try to reduce the window by doing this at record
> > time.
> > 
> > The current way of intercepting events at write_event() is deemed
> > broken, so I can do it atexit(), when we go to write the header, but
> > this will hurt because we'll have to reprocess it all.
> > 
> > Yes, since we'd still have the window, we could do it on perf archive,
> > but it would be an extra explicit step and since I think the right place
> > to do this is at perf record (with the kernel injecting the buildid, if
> > present in the binary being loaded), I'd prefer to keep it that way.
> > 
> > Then work on having the buildid stored in some suitable kernel data
> > structure and injected in the PERF_RECORD_MMAP event.
> 
> OK, so my typical workflow is something like:
> 
> perf record -o baseline ./foo
> 
> # again:
> 
> vi foo.c
> make
> perf record ./foo
> 
> perf diff -i baseline -i perf.data
> 
> # if good enough, quit, else goto again
> 
> I don't find intermediate diff's all that interesting.

Well, I plan to write 'perf diff --plot N' and have it look at the N
last perf.data files (perf.data, perf.data.N, perf.data.(N - 1)), that
may be per tag (v2.6.30, v2.6.31, v2.6.32, v2.6.33) or per
changeset to show if we are regressing over time.

Like people do from time to time to find regressions on subsystems they
don't pay that much attention.
 
> So we could create a perf snapshot like command that collects everything
> (and maybe allow something like: perf record --snapshot), and have perf
> diff compare against that by default, instead of perf.data~ or whatever
> that was called again. 

Oh, --baseline for 'perf record' to set a baseline file that then would
by default be used and compared against the latest perf.data file
collected seems useful, that way it would be just:

	perf record --baseline ./myapp
repeat:
	vi myapp.c
	make
	perf record ./myapp
	perf diff
	goto repeat;

> As to buildids from kernel space, I don't think the kernel actually
> knows or cares enough about executable maps to be able to extract those.
> The initial ELF loader could (maybe) for the main executable, but all
> DSOs and esp stuff like dlopen() would become mightly tricky.

Having a way to unequivocally connect the DSO that appeared on a
PERF_RECORD_MMAP to a file where a symtab is still seems a requirement
for me.

I'll try to figure out a way to do it cheaply and non-intrusively if at
all possible.

- Arnaldo