Messages in this thread | | | Date | Wed, 8 Jan 2014 15:59:34 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH 01/28] perf tools: Insert filtered entries to hists also |
| |
Em Wed, Jan 08, 2014 at 05:22:53PM +0100, Jiri Olsa escreveu: > On Wed, Jan 08, 2014 at 09:41:13AM -0300, Arnaldo Carvalho de Melo wrote: > > Em Wed, Jan 08, 2014 at 05:46:06PM +0900, Namhyung Kim escreveu: > > > Currently if a sample was filtered by command line option, it just > > > dropped. But this affects final output in that the percentage can be > > > different since the filtered entries were not included to the total. > > > > > > For example, if an original output looked like below: > > > > Humm, if one says that he/she is interested on just samples for a and b, > > the current behaviour will state how many of the filtered samples are > > for a and b, which is valid. > > > > I bet the number of samples will reflect that as well, but you filtered > > it out, yes, it stays there, so the percentages are relative to the > > number of samples. > > > > So I think this change in behaviour is wrong, no? > > > hi, > haven't checked the implementation yet, but it kind of does > what I'd expect for symbol filtering: > > perf report > ... > 22.00% yes libc-2.17.so [.] __strlen_sse2 > 11.79% yes libc-2.17.so [.] fputs_unlocked > 9.65% yes libc-2.17.so [.] __GI___mempcpy > 1.91% yes yes [.] fputs_unlocked@plt > ... > > search (press '/') for fputs_unlocked (with Namhyung's change): > 11.79% yes libc-2.17.so [.] fputs_unlocked > 1.91% yes yes [.] fputs_unlocked@plt > > while the current one shows: > 86.08% yes libc-2.17.so [.] fputs_unlocked > 13.92% yes yes [.] fputs_unlocked@plt > > which annoys me when searching for 'invisible' symbol > within tons of others.. I had to do that grep thing > you showed. > > I'd like to have the Namhyung's change behaviour as default, > but I'll be happy with some switch as well ;-)
I understand the desire for this different mode, looks indeed useful.
So I think that this is a new feature and as so we should provide it as an option, that may (or not) become the default.
Some concerns I have are that when we go on filtering we have to have all the things that are zeroed to then get accrued for each hist entry that matches the filter being applied and now at least a nr_entries field got out of the if (al.filtered) block, i.e. in the end we will have the number of hist entries entries filtered but continue having the total period for all (filtered or not) hist entries.
Having it as a separate feature would allow to have both views:
1. the percentages relative to the filtered samples 2. the percentages relative to all (filtered or not) samples
Being selectable on the command line and also with a hotkey to provide two columns: %total, %filtered.
So we would have new field: hists->stats.total_filtered_period, and hists->stats.nr_filtered_entries, for this, etc.
What do you think?
- Arnaldo
| |