perf tools: Factor sort elide
Jiri Olsa
reviewercover perf tools: Factor sort elide
hi,
factoring the elide code to make perf_hpp__should_skip function cheap call/check again.
Also I was wondering.. do we want to get rid of sort_entry structs in favor of perf_hpp__* stuff? Any plans?
I'm sending this a RFC, since any other such refactoring would change this stuff for sure as well ;-).
thanks,
jirka
--- tools/perf/ui/browsers/hists.c | 8 ++++---- tools/perf/util/hist.h | 8 +++++++- tools/perf/util/sort.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------- tools/perf/util/sort.h | 2 +- 4 files changed, 67 insertions(+), 53 deletions(-)
Jiri Olsa
reviewerPATCH 1/2 perf tools: Remove elide setup for SORT_MODE__MEMORY mode
There's no need to setup elide of sort_dso sort entry again with symbol_conf.dso_list list.
The only difference were list names of memory mode data, which does not make much sense to me.
--- tools/perf/util/sort.c | 13 ------------- 1 file changed, 13 deletions(-)
tools/perf/util/sort.c +0 -13
@@ -1411,19 +1411,6 @@ void sort__setup_elide(FILE *output)sort_entry__setup_elide (&sort_sym_to, symbol_conf.sym_to_list,"sym_to" , output); - }else if (sort__mode ==SORT_MODE__MEMORY ) { -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"symbol_daddr" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"dso_daddr" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"mem" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"local_weight" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"tlb" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"snoop" , output); }/*
Signature · 1 line
1.8.3.1
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf tools: Remove elide setup for SORT_MODE__MEMORY mode
perf tools: Remove elide setup for SORT_MODE__MEMORY mode
Commit-ID: 2ec85c628c4cecef0f82d177279c579aed0f9706 Gitweb: http://git.kernel.org/tip/2ec85c628c4cecef0f82d177279c579aed0f9706 Author: Jiri Olsa <[email protected]> AuthorDate: Fri, 23 May 2014 17:15:46 +0200 Committer: Jiri Olsa <[email protected]> CommitDate: Tue, 3 Jun 2014 21:34:53 +0200
perf tools: Remove elide setup for SORT_MODE__MEMORY mode
There's no need to setup elide of sort_dso sort entry again with symbol_conf.dso_list list.
The only difference were list names of memory mode data, which does not make much sense to me.
--- tools/perf/util/sort.c | 13 ------------- 1 file changed, 13 deletions(-)
tools/perf/util/sort.c +0 -13
@@ -1412,19 +1412,6 @@ void sort__setup_elide(FILE *output)sort_entry__setup_elide (&sort_sym_to, symbol_conf.sym_to_list,"sym_to" , output); - }else if (sort__mode ==SORT_MODE__MEMORY ) { -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"symbol_daddr" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"dso_daddr" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"mem" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"local_weight" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"tlb" , output); -sort_entry__setup_elide (&sort_dso, symbol_conf.dso_list, -"snoop" , output); }/*
Jiri Olsa
reviewerPATCH 2/2 perf tools: Move elide bool into perf_hpp_fmt struct
In reply to Jiri OlsaAfter output/sort fields refactoring, it's expensive to check the elide bool in its current location inside the 'struct sort_entry'.
The perf_hpp__should_skip function gets highly noticable in workloads with high number of output/sort fields, like for:
$ perf report -i perf-test.data -F overhead,sample,period,comm,pid,dso,symbol,cpu --stdio
Performance report:
9.70% perf [.] perf_hpp__should_skip ◆
Moving the elide bool into the 'struct perf_hpp_fmt', which makes the perf_hpp__should_skip just single struct read.
Got speedup of around 22% for my test perf.data workload. The change should not harm any other workload types.
Performance counter stats for (10 runs):
before:
358,319,732,626 cycles ( +- 0.55% ) 467,129,581,515 instructions # 1.30 insns per cycle ( +- 0.00% )
150.943975206 seconds time elapsed ( +- 0.62% )
now:
278,785,972,990 cycles ( +- 0.12% ) 370,146,797,640 instructions # 1.33 insns per cycle ( +- 0.00% )
116.416670507 seconds time elapsed ( +- 0.31% )
--- tools/perf/ui/browsers/hists.c | 8 ++-- tools/perf/util/hist.h | 8 +++- tools/perf/util/sort.c | 89 ++++++++++++++++++++++++++---------------- tools/perf/util/sort.h | 2 +- 4 files changed, 67 insertions(+), 40 deletions(-)
tools/perf/ui/browsers/hists.c +4 -4
@@ -1699,14 +1699,14 @@ zoom_dso: zoom_out_dso:ui_helpline__pop (); browser->hists->dso_filter =NULL ; - sort_dso.elide = false; +perf_hpp__set_elide (HISTC_DSO , false); }else {if (dso ==NULL )continue ;ui_helpline__fpush ("To zoom out press <- or -> + \"Zoom out of %s DSO\"" , dso->kernel ?"the Kernel" : dso->short_name); browser->hists->dso_filter = dso; - sort_dso.elide = true; +perf_hpp__set_elide (HISTC_DSO , true);pstack__push (fstack, &browser->hists->dso_filter); }hists__filter_by_dso (hists);
@@ -1718,13 +1718,13 @@ zoom_thread: zoom_out_thread:ui_helpline__pop (); browser->hists->thread_filter =NULL ; - sort_thread.elide = false; +perf_hpp__set_elide (HISTC_THREAD , false); }else {ui_helpline__fpush ("To zoom out press <- or -> + \"Zoom out of %s(%d) thread\"" , thread->comm_set ?thread__comm_str (thread) :"" , thread->tid); browser->hists->thread_filter = thread; - sort_thread.elide = true; +perf_hpp__set_elide (HISTC_THREAD , false);pstack__push (fstack, &browser->hists->thread_filter); }hists__filter_by_thread (hists);
tools/perf/util/hist.h +7 -1
@@ -166,6 +166,7 @@ struct perf_hpp_fmt {struct list_head list;struct list_head sort_list; +bool elide; };extern struct list_head perf_hpp__list;
@@ -208,7 +209,12 @@ void perf_hpp__append_sort_keys(void);bool perf_hpp__is_sort_entry (struct perf_hpp_fmt *format);bool perf_hpp__same_sort_entry (struct perf_hpp_fmt *a,struct perf_hpp_fmt *b); -bool perf_hpp__should_skip (struct perf_hpp_fmt *format); + +static inline bool perf_hpp__should_skip (struct perf_hpp_fmt *format) +{ +return format->elide; +} +void perf_hpp__reset_width (struct perf_hpp_fmt *fmt,struct hists *hists);typedef u64 (*hpp_field_fn )(struct hist_entry *he);
tools/perf/util/sort.c +55 -34
124 lines not on this page · raw message
tools/perf/util/sort.h +1 -1
@@ -186,7 +186,6 @@ struct sort_entry {int (*se_snprintf)(struct hist_entry *he,char *bf,size_t size,unsigned int width);u8 se_width_idx; -bool elide; };extern struct sort_entry sort_thread;
@@ -197,6 +196,7 @@ int setup_output_field(void);void reset_output_field (void );extern int sort_dimension__add (const char *);void sort__setup_elide (FILE *fp); +void perf_hpp__set_elide (int idx,bool elide);int report_parse_ignore_callees_opt (const struct option *opt,const char *arg,int unset);
Signature · 1 line
1.8.3.1
Quoted 3 lines · Jiri Olsa
hi,
factoring the elide code to makeperf_hpp__should_skipfunction cheap call/check again.
Both look good to me!
Quoted 3 lines
Also I was wondering.. do we want to get rid of
sort_entrystructs in favor of perf_hpp__* stuff? Any plans?
Probably.. but there's no plan for me currently. :)
Thanks,
Namhyung
PATCH v2 1/1 perf tools: Move elide bool into perf_hpp_fmt struct
Quoted 11 lines · Namhyung Kim
Quoted 3 lines · Jiri Olsa
hi,
factoring the elide code to makeperf_hpp__should_skipfunction cheap call/check again.Both look good to me!
Acked-by Namhyung Kim <[email protected]>Quoted 1 line
Quoted 7 lines
Quoted 2 lines
Also I was wondering.. do we want to get rid of
sort_entrystructs in favor of perf_hpp__* stuff? Any plans?Probably.. but there's no plan for me currently. :)
Thanks,
Namhyung
while hunting another bug I noticed I did not initialized properly the elide bool in __sort_dimension__alloc_hpp.. attached v2 of patch 2/2
thanks,
jirka
---
After output/sort fields refactoring, it's expensive to check the elide bool in its current location inside the 'struct sort_entry'.
The perf_hpp__should_skip function gets highly noticable in workloads with high number of output/sort fields, like for:
$ perf report -i perf-test.data -F overhead,sample,period,comm,pid,dso,symbol,cpu --stdio
Performance report:
9.70% perf [.] perf_hpp__should_skip
Moving the elide bool into the 'struct perf_hpp_fmt', which makes the perf_hpp__should_skip just single struct read.
Got speedup of around 22% for my test perf.data workload. The change should not harm any other workload types.
Performance counter stats for (10 runs):
before:
358,319,732,626 cycles ( +- 0.55% ) 467,129,581,515 instructions # 1.30 insns per cycle ( +- 0.00% )
150.943975206 seconds time elapsed ( +- 0.62% )
now:
278,785,972,990 cycles ( +- 0.12% ) 370,146,797,640 instructions # 1.33 insns per cycle ( +- 0.00% )
116.416670507 seconds time elapsed ( +- 0.31% )
--- tools/perf/ui/browsers/hists.c | 8 ++-- tools/perf/util/hist.h | 8 +++- tools/perf/util/sort.c | 90 ++++++++++++++++++++++++++---------------- tools/perf/util/sort.h | 2 +- 4 files changed, 68 insertions(+), 40 deletions(-)
tools/perf/ui/browsers/hists.c +4 -4
@@ -1706,14 +1706,14 @@ zoom_dso: zoom_out_dso:ui_helpline__pop (); browser->hists->dso_filter =NULL ; - sort_dso.elide = false; +perf_hpp__set_elide (HISTC_DSO , false); }else {if (dso ==NULL )continue ;ui_helpline__fpush ("To zoom out press <- or -> + \"Zoom out of %s DSO\"" , dso->kernel ?"the Kernel" : dso->short_name); browser->hists->dso_filter = dso; - sort_dso.elide = true; +perf_hpp__set_elide (HISTC_DSO , true);pstack__push (fstack, &browser->hists->dso_filter); }hists__filter_by_dso (hists);
@@ -1725,13 +1725,13 @@ zoom_thread: zoom_out_thread:ui_helpline__pop (); browser->hists->thread_filter =NULL ; - sort_thread.elide = false; +perf_hpp__set_elide (HISTC_THREAD , false); }else {ui_helpline__fpush ("To zoom out press <- or -> + \"Zoom out of %s(%d) thread\"" , thread->comm_set ?thread__comm_str (thread) :"" , thread->tid); browser->hists->thread_filter = thread; - sort_thread.elide = true; +perf_hpp__set_elide (HISTC_THREAD , false);pstack__push (fstack, &browser->hists->thread_filter); }hists__filter_by_thread (hists);
tools/perf/util/hist.h +7 -1
@@ -205,6 +205,7 @@ struct perf_hpp_fmt {struct list_head list;struct list_head sort_list; +bool elide; };extern struct list_head perf_hpp__list;
@@ -252,7 +253,12 @@ void perf_hpp__append_sort_keys(void);bool perf_hpp__is_sort_entry (struct perf_hpp_fmt *format);bool perf_hpp__same_sort_entry (struct perf_hpp_fmt *a,struct perf_hpp_fmt *b); -bool perf_hpp__should_skip (struct perf_hpp_fmt *format); + +static inline bool perf_hpp__should_skip (struct perf_hpp_fmt *format) +{ +return format->elide; +} +void perf_hpp__reset_width (struct perf_hpp_fmt *fmt,struct hists *hists);typedef u64 (*hpp_field_fn )(struct hist_entry *he);
tools/perf/util/sort.c +56 -34
131 lines not on this page · raw message
tools/perf/util/sort.h +1 -1
@@ -202,7 +202,6 @@ struct sort_entry {int (*se_snprintf)(struct hist_entry *he,char *bf,size_t size,unsigned int width);u8 se_width_idx; -bool elide; };extern struct sort_entry sort_thread;
@@ -213,6 +212,7 @@ int setup_output_field(void);void reset_output_field (void );extern int sort_dimension__add (const char *);void sort__setup_elide (FILE *fp); +void perf_hpp__set_elide (int idx,bool elide);int report_parse_ignore_callees_opt (const struct option *opt,const char *arg,int unset);
Signature · 1 line
1.8.1.4
[RFC/BUG] not working LLC-load-misses on SandyBridge (42) and Haswell
hi,
Joe asked about issues with LLC-load-misses on Haswell. I tried and found it non functional on SandyBridge (42) as well:
[jolsa@krava perf]$ ./perf stat -e LLC-load-misses ls
...
Performance counter stats for 'ls':
<not supported> LLC-load-misses
0.007195547 seconds time elapsed
both SandyBridge (42 and 45) share following config arrays:
snb_hw_cache_event_ids and snb_hw_cache_extra_regs
but have different 'x86_pmu.extra_regs' which makes LLC-load-misses event fail (in valid_mask check) in x86_pmu_extra_regs function
Haswell is sharing the same extra regs/ids config as SandyBridge (42), so it suffers in the same way.
Not sure if thats known issue, but it looks like we need special set of snb_hw_cache_event_ids for SandyBridge (42) ?
thanks for info,
jirka
Re: [RFC/BUG] not working LLC-load-misses on SandyBridge (42) and Haswell
Quoted 2 lines
Joe asked about issues with LLC-load-misses on Haswell. I tried and found it non functional on SandyBridge (42) as well:
There are lots of issues with the event tables. They need a large scale revamp.
I have some changes already, but they need more
-Andi
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf tools: Move elide bool into perf_hpp_fmt struct
In reply to Jiri Olsaperf tools: Move elide bool into perf_hpp_fmt struct
Commit-ID: f29984226978313039d7dfe9b45eaa55a3aad03d Gitweb: http://git.kernel.org/tip/f29984226978313039d7dfe9b45eaa55a3aad03d Author: Jiri Olsa <[email protected]> AuthorDate: Fri, 23 May 2014 17:15:47 +0200 Committer: Jiri Olsa <[email protected]> CommitDate: Tue, 3 Jun 2014 21:34:59 +0200
perf tools: Move elide bool into perf_hpp_fmt struct
After output/sort fields refactoring, it's expensive to check the elide bool in its current location inside the 'struct sort_entry'.
The perf_hpp__should_skip function gets highly noticable in workloads with high number of output/sort fields, like for:
$ perf report -i perf-test.data -F overhead,sample,period,comm,pid,dso,symbol,cpu --stdio
Performance report:
9.70% perf [.] perf_hpp__should_skip
Moving the elide bool into the 'struct perf_hpp_fmt', which makes the perf_hpp__should_skip just single struct read.
Got speedup of around 22% for my test perf.data workload. The change should not harm any other workload types.
Performance counter stats for (10 runs):
before:
358,319,732,626 cycles ( +- 0.55% ) 467,129,581,515 instructions # 1.30 insns per cycle ( +- 0.00% )
150.943975206 seconds time elapsed ( +- 0.62% )
now:
278,785,972,990 cycles ( +- 0.12% ) 370,146,797,640 instructions # 1.33 insns per cycle ( +- 0.00% )
116.416670507 seconds time elapsed ( +- 0.31% )
--- tools/perf/ui/browsers/hists.c | 8 ++-- tools/perf/util/hist.h | 8 +++- tools/perf/util/sort.c | 90 ++++++++++++++++++++++++++---------------- tools/perf/util/sort.h | 2 +- 4 files changed, 68 insertions(+), 40 deletions(-)
tools/perf/ui/browsers/hists.c +4 -4
@@ -1706,14 +1706,14 @@ zoom_dso: zoom_out_dso:ui_helpline__pop (); browser->hists->dso_filter =NULL ; - sort_dso.elide = false; +perf_hpp__set_elide (HISTC_DSO , false); }else {if (dso ==NULL )continue ;ui_helpline__fpush ("To zoom out press <- or -> + \"Zoom out of %s DSO\"" , dso->kernel ?"the Kernel" : dso->short_name); browser->hists->dso_filter = dso; - sort_dso.elide = true; +perf_hpp__set_elide (HISTC_DSO , true);pstack__push (fstack, &browser->hists->dso_filter); }hists__filter_by_dso (hists);
@@ -1725,13 +1725,13 @@ zoom_thread: zoom_out_thread:ui_helpline__pop (); browser->hists->thread_filter =NULL ; - sort_thread.elide = false; +perf_hpp__set_elide (HISTC_THREAD , false); }else {ui_helpline__fpush ("To zoom out press <- or -> + \"Zoom out of %s(%d) thread\"" , thread->comm_set ?thread__comm_str (thread) :"" , thread->tid); browser->hists->thread_filter = thread; - sort_thread.elide = true; +perf_hpp__set_elide (HISTC_THREAD , false);pstack__push (fstack, &browser->hists->thread_filter); }hists__filter_by_thread (hists);
tools/perf/util/hist.h +7 -1
@@ -205,6 +205,7 @@ struct perf_hpp_fmt {struct list_head list;struct list_head sort_list; +bool elide; };extern struct list_head perf_hpp__list;
@@ -252,7 +253,12 @@ void perf_hpp__append_sort_keys(void);bool perf_hpp__is_sort_entry (struct perf_hpp_fmt *format);bool perf_hpp__same_sort_entry (struct perf_hpp_fmt *a,struct perf_hpp_fmt *b); -bool perf_hpp__should_skip (struct perf_hpp_fmt *format); + +static inline bool perf_hpp__should_skip (struct perf_hpp_fmt *format) +{ +return format->elide; +} +void perf_hpp__reset_width (struct perf_hpp_fmt *fmt,struct hists *hists);typedef u64 (*hpp_field_fn )(struct hist_entry *he);
tools/perf/util/sort.c +56 -34
131 lines not on this page · raw message
tools/perf/util/sort.h +1 -1
@@ -202,7 +202,6 @@ struct sort_entry {int (*se_snprintf)(struct hist_entry *he,char *bf,size_t size,unsigned int width);u8 se_width_idx; -bool elide; };extern struct sort_entry sort_thread;
@@ -213,6 +212,7 @@ int setup_output_field(void);void reset_output_field (void );extern int sort_dimension__add (const char *);void sort__setup_elide (FILE *fp); +void perf_hpp__set_elide (int idx,bool elide);int report_parse_ignore_callees_opt (const struct option *opt,const char *arg,int unset);