Re: [PATCH RFC 00/10] counter read during perf sampling

[Jiri Olsa <jolsa@redhat.com>]

On Thu, Sep 24, 2015 at 07:47:40PM +0000, Liang, Kan wrote:
> 
> 
> > On Tue, Sep 22, 2015 at 10:13:33AM -0400, kan.liang@intel.com wrote:
> > > From: Kan Liang <kan.liang@intel.com>
> > >
> > > The patch series intends to read counter statistics with fixed
> > > frequency during sampling. The instant benefit is that we can read
> > > memory bandwidth from uncore event during cpu PMU event is
> > sampling.
> > >
> > > Introduce 'C' event/group modifier. The event with this modifier will
> > > do counting not sampling. If a group with this modifier, only group
> > > leader do sampling. The counter statistics will be wrote in new RECORD
> > > type PERF_RECORD_COUNTER_READ and stored in perf.data.
> > > So perf report can present the counter statistics data accordingly.
> > >
> > > There may be an alternative way to get counter statistics during
> > > sampling by running perf record and perf stat together by script.
> > > But the script way have various issue and complex to parses the
> > > output.
> > 
> > just a thought, but isn't the way then llow to store the data from perf
> > stat? ;-) and be able to merge perf.data-s from perf record and stat
> > afterwards
> 
> Yes, the way to store the data from perf stat is better than pure script
> way. I guess your patch "perf stat record" can do that, right?
> 
> If so, how should we run perf record and stat in parallel? By scripts or
> modify perf record/stat? 
> 
> Also, we need an option in perf report to merge the perf.data-s. Right?

either that or extra step with 'perf data merge' or somthing like that

SNIP

> > but if we go this way I think we should keep/allow all the options perf stat
> 
> Do you mean something like "perf record stat"?
> That's not the way I designed. I don't want to run perf record and perf stat
> together in one command.
> 
> I just want to do similar thing like what sample read did. Sample read can read
> counters on each sample. While the counter read can read counters in a fix

yea, but sample read stores data into ring buffer in the kernel
while you read the data like in perf stat

> frequency (set by --counter-read-interval). So it's an extension of perf record.
> It applies all possible options of perf record, like -C -a -g... 
> I introduce a new option --counter-read-interval is because that there is no
>  interval options in perf record.

the way I see it you implemented 'perf stat' logic within record command
you create counter (non sampling) and read it via read syscall

I think it's good idea, but I think we should follow the way we do
in perf stat command and reuse the interface (and code)

like having the 'stat' keyword separating the non-sampling config:

  $ perf record -e cycles stat -e 'uncore_imc_1/cas_count_read/' -I 10000 - ./tchain_edit

just an idea.. but I dont think the :C modifier is a good way

SNIP

> > I think we'll need special output/display for non sampling events,
> > something like extra window in TUI and distinguished output in stdio, the
> > above is hacked sampling output ;-)
> 
> I think it depends on what way we finally use.
> 
> If we use the way which merging perf.data from perf stat and record, I think
> we need special output for the data from perf stat in TUI/stdio.
> 
> But if we use the way counter read (:C), I think the best place to show the
> counter read results is the header/title (just like the patch did). Because the
> results are the aggregate counts during the whole sampling process.
> Something like,
> # Event count: 35937 of event 'uncore_imc_1/cas_count_read/C'

hum, how the --counter-read-interval data displayed then? it's not single number right?


jirka