From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Ian Rogers <irogers@google.com>,
Kan Liang <kan.liang@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
linux-perf-users@vger.kernel.org,
Stephane Eranian <eranian@google.com>,
Andi Kleen <ak@linux.intel.com>,
Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Subject: Re: [PATCH v2 1/3] perf hist: Move histogram related code to hist.h
Date: Tue, 16 Apr 2024 18:17:24 -0300 [thread overview]
Message-ID: <Zh7q5PFBdtXusMBI@x1> (raw)
In-Reply-To: <20240411181718.2367948-1-namhyung@kernel.org>
On Thu, Apr 11, 2024 at 11:17:16AM -0700, Namhyung Kim wrote:
> It's strange that sort.h has the definition of struct hist_entry. As
> sort.h already includes hist.h, let's move the data structure to hist.h.
Thanks, applied the three patches.
- Arnaldo
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/hist.h | 191 +++++++++++++++++++++++++++++++++++++--
> tools/perf/util/sort.h | 188 --------------------------------------
> tools/perf/util/values.h | 1 +
> 3 files changed, 184 insertions(+), 196 deletions(-)
>
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 4a0aea0c9e00..8f072f3749eb 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -4,21 +4,22 @@
>
> #include <linux/rbtree.h>
> #include <linux/types.h>
> -#include "evsel.h"
> +#include "callchain.h"
> #include "color.h"
> #include "events_stats.h"
> +#include "evsel.h"
> +#include "map_symbol.h"
> #include "mutex.h"
> +#include "sample.h"
> +#include "spark.h"
> +#include "stat.h"
>
> -struct hist_entry;
> -struct hist_entry_ops;
> struct addr_location;
> -struct map_symbol;
> struct mem_info;
> struct kvm_info;
> struct branch_info;
> struct branch_stack;
> struct block_info;
> -struct symbol;
> struct ui_progress;
>
> enum hist_filter {
> @@ -150,6 +151,159 @@ extern const struct hist_iter_ops hist_iter_branch;
> extern const struct hist_iter_ops hist_iter_mem;
> extern const struct hist_iter_ops hist_iter_cumulative;
>
> +struct res_sample {
> + u64 time;
> + int cpu;
> + int tid;
> +};
> +
> +struct he_stat {
> + u64 period;
> + u64 period_sys;
> + u64 period_us;
> + u64 period_guest_sys;
> + u64 period_guest_us;
> + u32 nr_events;
> +};
> +
> +struct namespace_id {
> + u64 dev;
> + u64 ino;
> +};
> +
> +struct hist_entry_diff {
> + bool computed;
> + union {
> + /* PERF_HPP__DELTA */
> + double period_ratio_delta;
> +
> + /* PERF_HPP__RATIO */
> + double period_ratio;
> +
> + /* HISTC_WEIGHTED_DIFF */
> + s64 wdiff;
> +
> + /* PERF_HPP_DIFF__CYCLES */
> + s64 cycles;
> + };
> + struct stats stats;
> + unsigned long svals[NUM_SPARKS];
> +};
> +
> +struct hist_entry_ops {
> + void *(*new)(size_t size);
> + void (*free)(void *ptr);
> +};
> +
> +/**
> + * struct hist_entry - histogram entry
> + *
> + * @row_offset - offset from the first callchain expanded to appear on screen
> + * @nr_rows - rows expanded in callchain, recalculated on folding/unfolding
> + */
> +struct hist_entry {
> + struct rb_node rb_node_in;
> + struct rb_node rb_node;
> + union {
> + struct list_head node;
> + struct list_head head;
> + } pairs;
> + struct he_stat stat;
> + struct he_stat *stat_acc;
> + struct map_symbol ms;
> + struct thread *thread;
> + struct comm *comm;
> + struct namespace_id cgroup_id;
> + u64 cgroup;
> + u64 ip;
> + u64 transaction;
> + s32 socket;
> + s32 cpu;
> + u64 code_page_size;
> + u64 weight;
> + u64 ins_lat;
> + u64 p_stage_cyc;
> + u8 cpumode;
> + u8 depth;
> + int mem_type_off;
> + struct simd_flags simd_flags;
> +
> + /* We are added by hists__add_dummy_entry. */
> + bool dummy;
> + bool leaf;
> +
> + char level;
> + u8 filtered;
> +
> + u16 callchain_size;
> + union {
> + /*
> + * Since perf diff only supports the stdio output, TUI
> + * fields are only accessed from perf report (or perf
> + * top). So make it a union to reduce memory usage.
> + */
> + struct hist_entry_diff diff;
> + struct /* for TUI */ {
> + u16 row_offset;
> + u16 nr_rows;
> + bool init_have_children;
> + bool unfolded;
> + bool has_children;
> + bool has_no_entry;
> + };
> + };
> + char *srcline;
> + char *srcfile;
> + struct symbol *parent;
> + struct branch_info *branch_info;
> + long time;
> + struct hists *hists;
> + struct mem_info *mem_info;
> + struct block_info *block_info;
> + struct kvm_info *kvm_info;
> + void *raw_data;
> + u32 raw_size;
> + int num_res;
> + struct res_sample *res_samples;
> + void *trace_output;
> + struct perf_hpp_list *hpp_list;
> + struct hist_entry *parent_he;
> + struct hist_entry_ops *ops;
> + struct annotated_data_type *mem_type;
> + union {
> + /* this is for hierarchical entry structure */
> + struct {
> + struct rb_root_cached hroot_in;
> + struct rb_root_cached hroot_out;
> + }; /* non-leaf entries */
> + struct rb_root sorted_chain; /* leaf entry has callchains */
> + };
> + struct callchain_root callchain[0]; /* must be last member */
> +};
> +
> +static __pure inline bool hist_entry__has_callchains(struct hist_entry *he)
> +{
> + return he->callchain_size != 0;
> +}
> +
> +static inline bool hist_entry__has_pairs(struct hist_entry *he)
> +{
> + return !list_empty(&he->pairs.node);
> +}
> +
> +static inline struct hist_entry *hist_entry__next_pair(struct hist_entry *he)
> +{
> + if (hist_entry__has_pairs(he))
> + return list_entry(he->pairs.node.next, struct hist_entry, pairs.node);
> + return NULL;
> +}
> +
> +static inline void hist_entry__add_pair(struct hist_entry *pair,
> + struct hist_entry *he)
> +{
> + list_add_tail(&pair->pairs.node, &he->pairs.head);
> +}
> +
> struct hist_entry *hists__add_entry(struct hists *hists,
> struct addr_location *al,
> struct symbol *parent,
> @@ -186,6 +340,8 @@ int hist_entry__sort_snprintf(struct hist_entry *he, char *bf, size_t size,
> struct hists *hists);
> int hist_entry__snprintf_alignment(struct hist_entry *he, struct perf_hpp *hpp,
> struct perf_hpp_fmt *fmt, int printed);
> +int hist_entry__sym_snprintf(struct hist_entry *he, char *bf, size_t size,
> + unsigned int width);
> void hist_entry__delete(struct hist_entry *he);
>
> typedef int (*hists__resort_cb_t)(struct hist_entry *he, void *arg);
> @@ -238,6 +394,20 @@ void hists__match(struct hists *leader, struct hists *other);
> int hists__link(struct hists *leader, struct hists *other);
> int hists__unlink(struct hists *hists);
>
> +static inline float hist_entry__get_percent_limit(struct hist_entry *he)
> +{
> + u64 period = he->stat.period;
> + u64 total_period = hists__total_period(he->hists);
> +
> + if (unlikely(total_period == 0))
> + return 0;
> +
> + if (symbol_conf.cumulate_callchain)
> + period = he->stat_acc->period;
> +
> + return period * 100.0 / total_period;
> +}
> +
> struct hists_evsel {
> struct evsel evsel;
> struct hists hists;
> @@ -460,15 +630,20 @@ struct hist_browser_timer {
> int refresh;
> };
>
> -struct res_sample;
> -
> enum rstype {
> A_NORMAL,
> A_ASM,
> A_SOURCE
> };
>
> -struct block_hist;
> +struct block_hist {
> + struct hists block_hists;
> + struct perf_hpp_list block_list;
> + struct perf_hpp_fmt block_fmt;
> + int block_idx;
> + bool valid;
> + struct hist_entry he;
> +};
>
> #ifdef HAVE_SLANG_SUPPORT
> #include "../ui/keysyms.h"
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 6f6b4189a389..690892a92cf3 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -3,19 +3,9 @@
> #define __PERF_SORT_H
> #include <regex.h>
> #include <stdbool.h>
> -#include <linux/list.h>
> -#include <linux/rbtree.h>
> -#include "map_symbol.h"
> -#include "symbol_conf.h"
> -#include "callchain.h"
> -#include "values.h"
> #include "hist.h"
> -#include "stat.h"
> -#include "spark.h"
>
> struct option;
> -struct thread;
> -struct annotated_data_type;
>
> extern regex_t parent_regex;
> extern const char *sort_order;
> @@ -39,175 +29,6 @@ extern struct sort_entry sort_type;
> extern const char default_mem_sort_order[];
> extern bool chk_double_cl;
>
> -struct res_sample {
> - u64 time;
> - int cpu;
> - int tid;
> -};
> -
> -struct he_stat {
> - u64 period;
> - u64 period_sys;
> - u64 period_us;
> - u64 period_guest_sys;
> - u64 period_guest_us;
> - u32 nr_events;
> -};
> -
> -struct namespace_id {
> - u64 dev;
> - u64 ino;
> -};
> -
> -struct hist_entry_diff {
> - bool computed;
> - union {
> - /* PERF_HPP__DELTA */
> - double period_ratio_delta;
> -
> - /* PERF_HPP__RATIO */
> - double period_ratio;
> -
> - /* HISTC_WEIGHTED_DIFF */
> - s64 wdiff;
> -
> - /* PERF_HPP_DIFF__CYCLES */
> - s64 cycles;
> - };
> - struct stats stats;
> - unsigned long svals[NUM_SPARKS];
> -};
> -
> -struct hist_entry_ops {
> - void *(*new)(size_t size);
> - void (*free)(void *ptr);
> -};
> -
> -/**
> - * struct hist_entry - histogram entry
> - *
> - * @row_offset - offset from the first callchain expanded to appear on screen
> - * @nr_rows - rows expanded in callchain, recalculated on folding/unfolding
> - */
> -struct hist_entry {
> - struct rb_node rb_node_in;
> - struct rb_node rb_node;
> - union {
> - struct list_head node;
> - struct list_head head;
> - } pairs;
> - struct he_stat stat;
> - struct he_stat *stat_acc;
> - struct map_symbol ms;
> - struct thread *thread;
> - struct comm *comm;
> - struct namespace_id cgroup_id;
> - u64 cgroup;
> - u64 ip;
> - u64 transaction;
> - s32 socket;
> - s32 cpu;
> - u64 code_page_size;
> - u64 weight;
> - u64 ins_lat;
> - u64 p_stage_cyc;
> - u8 cpumode;
> - u8 depth;
> - int mem_type_off;
> - struct simd_flags simd_flags;
> -
> - /* We are added by hists__add_dummy_entry. */
> - bool dummy;
> - bool leaf;
> -
> - char level;
> - u8 filtered;
> -
> - u16 callchain_size;
> - union {
> - /*
> - * Since perf diff only supports the stdio output, TUI
> - * fields are only accessed from perf report (or perf
> - * top). So make it a union to reduce memory usage.
> - */
> - struct hist_entry_diff diff;
> - struct /* for TUI */ {
> - u16 row_offset;
> - u16 nr_rows;
> - bool init_have_children;
> - bool unfolded;
> - bool has_children;
> - bool has_no_entry;
> - };
> - };
> - char *srcline;
> - char *srcfile;
> - struct symbol *parent;
> - struct branch_info *branch_info;
> - long time;
> - struct hists *hists;
> - struct mem_info *mem_info;
> - struct block_info *block_info;
> - struct kvm_info *kvm_info;
> - void *raw_data;
> - u32 raw_size;
> - int num_res;
> - struct res_sample *res_samples;
> - void *trace_output;
> - struct perf_hpp_list *hpp_list;
> - struct hist_entry *parent_he;
> - struct hist_entry_ops *ops;
> - struct annotated_data_type *mem_type;
> - union {
> - /* this is for hierarchical entry structure */
> - struct {
> - struct rb_root_cached hroot_in;
> - struct rb_root_cached hroot_out;
> - }; /* non-leaf entries */
> - struct rb_root sorted_chain; /* leaf entry has callchains */
> - };
> - struct callchain_root callchain[0]; /* must be last member */
> -};
> -
> -static __pure inline bool hist_entry__has_callchains(struct hist_entry *he)
> -{
> - return he->callchain_size != 0;
> -}
> -
> -int hist_entry__sym_snprintf(struct hist_entry *he, char *bf, size_t size, unsigned int width);
> -
> -static inline bool hist_entry__has_pairs(struct hist_entry *he)
> -{
> - return !list_empty(&he->pairs.node);
> -}
> -
> -static inline struct hist_entry *hist_entry__next_pair(struct hist_entry *he)
> -{
> - if (hist_entry__has_pairs(he))
> - return list_entry(he->pairs.node.next, struct hist_entry, pairs.node);
> - return NULL;
> -}
> -
> -static inline void hist_entry__add_pair(struct hist_entry *pair,
> - struct hist_entry *he)
> -{
> - list_add_tail(&pair->pairs.node, &he->pairs.head);
> -}
> -
> -static inline float hist_entry__get_percent_limit(struct hist_entry *he)
> -{
> - u64 period = he->stat.period;
> - u64 total_period = hists__total_period(he->hists);
> -
> - if (unlikely(total_period == 0))
> - return 0;
> -
> - if (symbol_conf.cumulate_callchain)
> - period = he->stat_acc->period;
> -
> - return period * 100.0 / total_period;
> -}
> -
> enum sort_mode {
> SORT_MODE__NORMAL,
> SORT_MODE__BRANCH,
> @@ -299,15 +120,6 @@ struct sort_entry {
> u8 se_width_idx;
> };
>
> -struct block_hist {
> - struct hists block_hists;
> - struct perf_hpp_list block_list;
> - struct perf_hpp_fmt block_fmt;
> - int block_idx;
> - bool valid;
> - struct hist_entry he;
> -};
> -
> extern struct sort_entry sort_thread;
>
> struct evlist;
> diff --git a/tools/perf/util/values.h b/tools/perf/util/values.h
> index 8c41f22f42cf..791c1ad606c2 100644
> --- a/tools/perf/util/values.h
> +++ b/tools/perf/util/values.h
> @@ -2,6 +2,7 @@
> #ifndef __PERF_VALUES_H
> #define __PERF_VALUES_H
>
> +#include <stdio.h>
> #include <linux/types.h>
>
> struct perf_read_values {
> --
> 2.44.0.683.g7961c838ac-goog
prev parent reply other threads:[~2024-04-16 21:17 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-11 18:17 [PATCH v2 1/3] perf hist: Move histogram related code to hist.h Namhyung Kim
2024-04-11 18:17 ` [PATCH v2 2/3] perf hist: Add weight fields to hist entry stats Namhyung Kim
2024-04-11 18:17 ` [PATCH v2 3/3] perf report: Add weight[123] output fields Namhyung Kim
2024-04-16 21:17 ` Arnaldo Carvalho de Melo [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zh7q5PFBdtXusMBI@x1 \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=atrajeev@linux.vnet.ibm.com \
--cc=eranian@google.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).