Re: [PATCH v2 06/18] perf annotate: Move struct annotation to a hashmap and remove symbol_conf.priv_size

[Alireza Haghdoost]

On Tue, Oct 27, 2026 at 4:15 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> +static size_t sym_ptr_hash(long key, void *ctx __maybe_unused)
> +{
> + return hash_bits((size_t)key, 16);
> +}

hashmap.h hashes the callback result again with
hash_bits(h, cap_bits), which seems to leave this function with only
64K possible results. If the map grows beyond that, I think the
extra buckets cannot help and the chains just get longer. Would it be
better to return the key here and let hashmap.h do the hashing? Patch
05/18 appears to have the same issue for the db-export ids.

This seems worth avoiding for the annotation map in particular. After
the first annotation access for a symbol, symbol__annotation() is a
read-locked hashmap lookup on the per-sample path in perf top and TUI
perf report. Before this it was pointer arithmetic.

> + notes = zalloc(sizeof(*notes));
> [...]
> + annotation__init(notes);
> + if (hashmap__add(annotations, (long)sym, notes) < 0) {

One smaller thing: symbol__annotation() is now a get-or-create
operation, but some callers only want to inspect existing annotation
data. Those calls allocate a hashmap entry and set the annotated flag
even when there is no annotation data to use. In patch 15/18 that flag
also makes symbols__shrink() retain the symbol.

Could the read-only callers use a non-creating lookup instead?

> + } else if (hashmap__find(annotations, (long)sym, &val)) {
> + notes = (struct annotation *)val;
> + up_write(annotations_lock());
> + return notes;
> + }

The write path returns an existing entry without setting the annotated
flag. I don't see how the entry and flag can get out of sync with the
current locking, so this may only be a defensive concern. If they ever
do diverge, though, symbol__delete() would skip removing the entry.

Thanks,
Alireza