| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <133a66fe-a7e7-42e2-822b-1f3caea7862a@wanadoo.fr>
Date: Fri, 3 Nov 2023 06:41:30 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Namhyung Kim <namhyung@...nel.org>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Jiri Olsa <jolsa@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 4/5] perf annotate: Move some fields to annotated_source
Le 02/11/2023 à 23:26, Namhyung Kim a écrit :
> Some fields in the struct annotation are only used with annotated_source
> so better to be moved there in order to reduce memory consumption for
> other symbols.
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
> tools/perf/ui/browsers/annotate.c | 12 ++++++------
> tools/perf/util/annotate.c | 17 +++++++++--------
> tools/perf/util/annotate.h | 14 +++++++-------
> 3 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index d2470f87344d..1b42db70c998 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -384,7 +384,7 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> if (al->idx_asm < offset)
> offset = al->idx;
>
> - browser->b.nr_entries = notes->nr_entries;
> + browser->b.nr_entries = notes->src->nr_entries;
> notes->options->hide_src_code = false;
> browser->b.seek(&browser->b, -offset, SEEK_CUR);
> browser->b.top_idx = al->idx - offset;
> @@ -402,7 +402,7 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> if (al->idx_asm < offset)
> offset = al->idx_asm;
>
> - browser->b.nr_entries = notes->nr_asm_entries;
> + browser->b.nr_entries = notes->src->nr_asm_entries;
> notes->options->hide_src_code = true;
> browser->b.seek(&browser->b, -offset, SEEK_CUR);
> browser->b.top_idx = al->idx_asm - offset;
> @@ -435,7 +435,7 @@ static void ui_browser__init_asm_mode(struct ui_browser *browser)
> {
> struct annotation *notes = browser__annotation(browser);
> ui_browser__reset_index(browser);
> - browser->nr_entries = notes->nr_asm_entries;
> + browser->nr_entries = notes->src->nr_asm_entries;
> }
>
> static int sym_title(struct symbol *sym, struct map *map, char *title,
> @@ -860,7 +860,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> browser->b.height,
> browser->b.index,
> browser->b.top_idx,
> - notes->nr_asm_entries);
> + notes->src->nr_asm_entries);
> }
> continue;
> case K_ENTER:
> @@ -991,8 +991,8 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
>
> ui_helpline__push("Press ESC to exit");
>
> - browser.b.width = notes->max_line_len;
> - browser.b.nr_entries = notes->nr_entries;
> + browser.b.width = notes->src->max_line_len;
> + browser.b.nr_entries = notes->src->nr_entries;
> browser.b.entries = ¬es->src->source,
> browser.b.width += 18; /* Percentage */
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 92a9adf9d5eb..ee7b8e1848a8 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2808,19 +2808,20 @@ void annotation__mark_jump_targets(struct annotation *notes, struct symbol *sym)
> void annotation__set_offsets(struct annotation *notes, s64 size)
> {
> struct annotation_line *al;
> + struct annotated_source *src = notes->src;
>
> - notes->max_line_len = 0;
> - notes->nr_entries = 0;
> - notes->nr_asm_entries = 0;
> + src->max_line_len = 0;
> + src->nr_entries = 0;
> + src->nr_asm_entries = 0;
>
> - list_for_each_entry(al, ¬es->src->source, node) {
> + list_for_each_entry(al, &src->source, node) {
> size_t line_len = strlen(al->line);
>
> - if (notes->max_line_len < line_len)
> - notes->max_line_len = line_len;
> - al->idx = notes->nr_entries++;
> + if (src->max_line_len < line_len)
> + src->max_line_len = line_len;
> + al->idx = src->nr_entries++;
> if (al->offset != -1) {
> - al->idx_asm = notes->nr_asm_entries++;
> + al->idx_asm = src->nr_asm_entries++;
> /*
> * FIXME: short term bandaid to cope with assembly
> * routines that comes with labels in the same column
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index d8a221591926..c51ceb857bd6 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -268,10 +268,13 @@ struct cyc_hist {
> * returns.
> */
> struct annotated_source {
> - struct list_head source;
> - int nr_histograms;
> - size_t sizeof_sym_hist;
> - struct sym_hist *histograms;
> + struct list_head source;
> + int nr_histograms;
If this int...
> + size_t sizeof_sym_hist;
> + struct sym_hist *histograms;
> + int nr_entries;
> + int nr_asm_entries;
> + u16 max_line_len;
... and these int and u16 were grouped, you would also save a hole in
the struct and reduce padding.
On x86_64, after patch 4/5:
struct annotated_source {
struct list_head source; /* 0 16 */
int nr_histograms; /* 16 4 */
/* XXX 4 bytes hole, try to pack */ <====
size_t sizeof_sym_hist; /* 24 8 */
struct sym_hist * histograms; /* 32 8 */
int nr_entries; /* 40 4 */
int nr_asm_entries; /* 44 4 */
u16 max_line_len; /* 48 2 */
/* size: 56, cachelines: 1, members: 7 */
/* sum members: 46, holes: 1, sum holes: 4 */
/* padding: 6 */ <====
/* last cacheline: 56 bytes */
};
After patch 5/5, the struct would be just 64 bytes. If fields are
re-ordered, it would be 56 bytes.
Because of rounding in memory allocations, 56 or 64 shouldn't change
anything. But it would be more future proof, should something else be
added here in the future.
CJ
> };
>
> struct annotated_branch {
> @@ -289,9 +292,6 @@ struct LOCKABLE annotation {
> struct annotation_line **offsets;
> int nr_events;
> int max_jump_sources;
> - int nr_entries;
> - int nr_asm_entries;
> - u16 max_line_len;
> struct {
> u8 addr;
> u8 jumps;
Powered by blists - more mailing lists