On Fri, Nov 03, 2017 at 01:29:42PM -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, Oct 24, 2017 at 09:16:59AM +0200, Jiri Olsa escreveu: > > On Tue, Oct 24, 2017 at 10:03:05AM +0800, Jin, Yao wrote: > > > > SNIP > > > > > > hum, could you still unset the sample if there's no time given? > > > > and keep the speed in this case.. > > > > > > > > jirka > > > > > > > > > > Hi Jiri, > > > > > > I check this question again. The '--time' option is for perf report but > > > not > > > for perf record. > > > > > > For perf record, we have to always walk on all samples to get the time of > > > first sample and the time of last sample whatever buildid_all is enabled > > > or > > > not enabled. So 'rec->tool.sample = NULL' is removed. > > > > > > Sorry, the previous mail was replied at midnight, I was drowsy. :( > > > > > > If my answer is correct, I will not send v6. If my understanding is still > > > not correct, please let me know. > > > > right, I did not realize we store this unconditionaly.. then yes, it's ok > > And should we store this unconditionally? What this patch is doing is > making 'perf record' unconditionally slower so that the generated > perf.data file becomes useful for some usecases, but not for all, only > people interested in using 'perf report/script --time' will benefit, > right?
maybe we can also silently enable that when processing buildids, (which is set by default), there's no big performance hit once we already go through samples jirka > > I thought that we could get this sorted out in a different fashion, i.e. > getting the first timestamp is easy, even if we don't process build-ids, > right? To get the last one we could ask the kernel to insert an extra > dummy sample at the end, one that we know the size and thus can to to > the end of the file, rewind that size, get the event and parse the > sample, agreed? > > So I suggest that first make this conditional, i.e. 'perf record > --timestamps' will enable the logic you implemented, and as a followup, > if you agree, add the dummy, known size event at the end, and then even > when build-ids are not processed, the cost for getting the timestamps > will be next to zero. > > - Arnaldo > > - Arnaldo > > > I think I've already acked this, anyway for the patchset: > > > > Acked-by: Jiri Olsa <jo...@kernel.org> > > > > thanks, > > jirka