Commit 259dce91 authored by Ian Rogers's avatar Ian Rogers Committed by Namhyung Kim
Browse files

perf symbol: Remove symbol_name_rb_node 



Most perf commands want to sort symbols by name and this is done via
an invasive rbtree that on 64-bit systems costs 24 bytes. Sorting the
symbols in a DSO by name is optional and not done by default, however,
if sorting is requested the 24 bytes is allocated for every
symbol.

This change removes the rbtree and uses a sorted array of symbol
pointers instead (costing 8 bytes per symbol). As the array is created
on demand then there are further memory savings. The complexity of
sorting the array and using the rbtree are the same.

To support going to the next symbol, the index of the current symbol
needs to be passed around as a pair with the current symbol. This
requires some API changes.

Signed-off-by: default avatarIan Rogers <irogers@google.com>
Acked-by: default avatarNamhyung Kim <namhyung@kernel.org>
Cc: Carsten Haitzler <carsten.haitzler@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Jason Wang <wangborong@cdjrlc.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: Yang Jihong <yangjihong1@huawei.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Link: https://lore.kernel.org/r/20230623054520.4118442-3-irogers@google.com


[ minimize change in symbols__sort_by_name() ]
Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
parent ce5b2934

tools/perf/util/dso.c

+5 −2
Original line number Diff line number Diff line
@@ -1320,7 +1320,9 @@ struct dso *dso__new_id(const char *name, struct dso_id *id) 
			dso->id = *id;

		dso__set_long_name_id(dso, dso->name, id, false);

		dso__set_short_name(dso, dso->name, false);

		dso->symbols = dso->symbol_names = RB_ROOT_CACHED;

		dso->symbols = RB_ROOT_CACHED;

		dso->symbol_names = NULL;

		dso->symbol_names_len = 0;

		dso->data.cache = RB_ROOT;

		dso->inlined_nodes = RB_ROOT_CACHED;

		dso->srclines = RB_ROOT_CACHED;
@@ -1364,7 +1366,8 @@ void dso__delete(struct dso *dso) 
	inlines__tree_delete(&dso->inlined_nodes);

	srcline__tree_delete(&dso->srclines);

	symbols__delete(&dso->symbols);



	dso->symbol_names_len = 0;

	zfree(&dso->symbol_names);

	if (dso->short_name_allocated) {

		zfree((char **)&dso->short_name);

		dso->short_name_allocated = false;

tools/perf/util/dso.h

+2 −1
Original line number Diff line number Diff line
@@ -150,7 +150,8 @@ struct dso { 
	struct rb_node	 rb_node;	/* rbtree node sorted by long name */

	struct rb_root	 *root;		/* root of rbtree that rb_node is in */

	struct rb_root_cached symbols;

	struct rb_root_cached symbol_names;

	struct symbol	 **symbol_names;

	size_t		 symbol_names_len;

	struct rb_root_cached inlined_nodes;

	struct rb_root_cached srclines;

	struct {

tools/perf/util/map.c

+9 −2
Original line number Diff line number Diff line
@@ -390,7 +390,7 @@ struct symbol *map__find_symbol(struct map *map, u64 addr) 
	return dso__find_symbol(map__dso(map), addr);

}



struct symbol *map__find_symbol_by_name(struct map *map, const char *name)

struct symbol *map__find_symbol_by_name_idx(struct map *map, const char *name, size_t *idx)

{

	struct dso *dso;
@@ -400,7 +400,14 @@ struct symbol *map__find_symbol_by_name(struct map *map, const char *name) 
	dso = map__dso(map);

	dso__sort_by_name(dso);



	return dso__find_symbol_by_name(dso, name);

	return dso__find_symbol_by_name(dso, name, idx);

}



struct symbol *map__find_symbol_by_name(struct map *map, const char *name)

{

	size_t idx;



	return map__find_symbol_by_name_idx(map, name, &idx);

}



struct map *map__clone(struct map *from)

tools/perf/util/map.h

+7 −5
Original line number Diff line number Diff line
@@ -148,16 +148,17 @@ struct thread; 
 * @map: the 'struct map *' in which symbols are iterated

 * @sym_name: the symbol name

 * @pos: the 'struct symbol *' to use as a loop cursor

 * @idx: the cursor index in the symbol names array

 */

#define __map__for_each_symbol_by_name(map, sym_name, pos)	\

	for (pos = map__find_symbol_by_name(map, sym_name);	\

#define __map__for_each_symbol_by_name(map, sym_name, pos, idx)		\

	for (pos = map__find_symbol_by_name_idx(map, sym_name, &idx);	\

	     pos &&						\

	     !symbol__match_symbol_name(pos->name, sym_name,	\

					SYMBOL_TAG_INCLUDE__DEFAULT_ONLY); \

	     pos = symbol__next_by_name(pos))

	     pos = dso__next_symbol_by_name(map__dso(map), &idx))



#define map__for_each_symbol_by_name(map, sym_name, pos)		\

	__map__for_each_symbol_by_name(map, sym_name, (pos))

#define map__for_each_symbol_by_name(map, sym_name, pos, idx)	\

	__map__for_each_symbol_by_name(map, sym_name, (pos), idx)



void map__init(struct map *map,

	       u64 start, u64 end, u64 pgoff, struct dso *dso);
@@ -202,6 +203,7 @@ int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix, 
int map__load(struct map *map);

struct symbol *map__find_symbol(struct map *map, u64 addr);

struct symbol *map__find_symbol_by_name(struct map *map, const char *name);

struct symbol *map__find_symbol_by_name_idx(struct map *map, const char *name, size_t *idx);

void map__fixup_start(struct map *map);

void map__fixup_end(struct map *map);

tools/perf/util/probe-event.c

+6 −6
Original line number Diff line number Diff line
@@ -382,6 +382,7 @@ static int find_alternative_probe_point(struct debuginfo *dinfo, 
	struct symbol *sym;

	u64 address = 0;

	int ret = -ENOENT;

	size_t idx;



	/* This can work only for function-name based one */

	if (!pp->function || pp->file)
@@ -392,7 +393,7 @@ static int find_alternative_probe_point(struct debuginfo *dinfo, 
		return -EINVAL;



	/* Find the address of given function */

	map__for_each_symbol_by_name(map, pp->function, sym) {

	map__for_each_symbol_by_name(map, pp->function, sym, idx) {

		if (uprobes) {

			address = sym->start;

			if (sym->type == STT_GNU_IFUNC)
@@ -3738,7 +3739,6 @@ int del_perf_probe_events(struct strfilter *filter) 
int show_available_funcs(const char *target, struct nsinfo *nsi,

			 struct strfilter *_filter, bool user)

{

        struct rb_node *nd;

	struct map *map;

	struct dso *dso;

	int ret;
@@ -3772,11 +3772,11 @@ int show_available_funcs(const char *target, struct nsinfo *nsi, 
	/* Show all (filtered) symbols */

	setup_pager();



	for (nd = rb_first_cached(&dso->symbol_names); nd; nd = rb_next(nd)) {

		struct symbol_name_rb_node *pos = rb_entry(nd, struct symbol_name_rb_node, rb_node);

	for (size_t i = 0; i < dso->symbol_names_len; i++) {

		struct symbol *pos = dso->symbol_names[i];



		if (strfilter__compare(_filter, pos->sym.name))

			printf("%s\n", pos->sym.name);

		if (strfilter__compare(_filter, pos->name))

			printf("%s\n", pos->name);

	}

end:

	map__put(map);