Re: [PATCH v6 3/5] perf symbols: Point to the symbol name instead of embedding it

[Ian Rogers]

On Wed, Oct 28, 2026 at 4:42 PM Alireza Haghdoost via B4 Relay
<devnull+haghdoost.uber.com@xxxxxxxxxx> wrote:
>
> From: Alireza Haghdoost <haghdoost@xxxxxxxx>
>
> This patch changes the type of the name field in struct symbol from a
> flexible array (char name[]) to a pointer (char *name). It is a
> refactoring and does not change behavior, but it adds 8 bytes to each
> symbol allocation on 64-bit hosts.

I think it is a little worse as we'll need 3 bytes of padding. You
could reduce the padding by moving name to before namelen in the
struct.

> The next patch creates symbols whose names are read only when they are
> needed. When perf creates such a symbol, it does not know the length of
> the name, so it cannot allocate space for the name after the symbol.
> Perf also cannot allocate a larger symbol after it reads the name,
> because the rb-tree and the callers keep pointers to the symbol. With a
> pointer, perf can allocate the name separately and set the pointer later.
>
> Signed-off-by: Alireza Haghdoost <haghdoost@xxxxxxxx>

This change makes sense to me. I'm wondering if any other things
should be cleaned up with this change? For example, namelen is only
referenced in about 8 places and we may be able to replace it with
`strlen(name)`. Anyway, ordering the variables to pack them is a nit.

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> ---
>  tools/perf/ui/stdio/hist.c | 1 +
>  tools/perf/util/symbol.c   | 1 +
>  tools/perf/util/symbol.h   | 2 +-
>  3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
> index 8c4c8925df2c..c65e3aa73a13 100644
> --- a/tools/perf/ui/stdio/hist.c
> +++ b/tools/perf/ui/stdio/hist.c
> @@ -106,6 +106,7 @@ static void init_rem_hits(void)
>                 return;
>         }
>
> +       rem_sq_bracket->name = (char *)(rem_sq_bracket + 1);
>         strcpy(rem_sq_bracket->name, "[...]");
>         rem_hits.ms.sym = rem_sq_bracket;
>  }
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 2d611c05d000..02c077d7e2a6 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -362,6 +362,7 @@ struct symbol *symbol__new(u64 start, u64 len, u8 binding, u8 type, const char *
>
>         pr_debug4("%s: %s %#" PRIx64 "-%#" PRIx64 "\n",
>                   __func__, name, start, sym->end);
> +       sym->name = (char *)(sym + 1);
>         memcpy(sym->name, name, namelen);
>
>         return sym;
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index d8386c4a7fc5..c231d8e4427c 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -95,7 +95,7 @@ struct symbol {
>         /** Architecture specific. Unused except on PPC where it holds st_other. */
>         u8              arch_sym;
>         /** The name of length namelen associated with the symbol. */
> -       char            name[];
> +       char            *name;
>  };
>
>  void symbol__delete(struct symbol *sym);
>
> --
> Git-157)
>
>