linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>, Thomas Gleixner <tglx@linutronix.de>
Cc: Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Clark Williams <williams@redhat.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Andi Kleen <ak@linux.intel.com>
Subject: [PATCH 06/25] perf maps: Do not use an rbtree to sort by map name
Date: Tue, 19 Nov 2019 08:32:26 -0300	[thread overview]
Message-ID: <20191119113245.19593-7-acme@kernel.org> (raw)
In-Reply-To: <20191119113245.19593-1-acme@kernel.org>

From: Arnaldo Carvalho de Melo <acme@redhat.com>

This is only used for the kernel maps, shave 24 bytes out 'struct map'
and just traverse the existing per ip rbtree to look for maps by name,
use a front end cache to reuse the last search if its the same name.

After this 'struct map' is down to just two cachelines:

  $ pahole -C map ~/bin/perf
  struct map {
  	union {
  		struct rb_node rb_node __attribute__((__aligned__(8))); /*     0    24 */
  		struct list_head node;                   /*     0    16 */
  	} __attribute__((__aligned__(8)));                                               /*     0    24 */
  	u64                        start;                /*    24     8 */
  	u64                        end;                  /*    32     8 */
  	_Bool                      erange_warned;        /*    40     1 */

  	/* XXX 3 bytes hole, try to pack */

  	u32                        priv;                 /*    44     4 */
  	u32                        prot;                 /*    48     4 */
  	u32                        flags;                /*    52     4 */
  	u64                        pgoff;                /*    56     8 */
  	/* --- cacheline 1 boundary (64 bytes) --- */
  	u64                        reloc;                /*    64     8 */
  	u32                        maj;                  /*    72     4 */
  	u32                        min;                  /*    76     4 */
  	u64                        ino;                  /*    80     8 */
  	u64                        ino_generation;       /*    88     8 */
  	u64                        (*map_ip)(struct map *, u64); /*    96     8 */
  	u64                        (*unmap_ip)(struct map *, u64); /*   104     8 */
  	struct dso *               dso;                  /*   112     8 */
  	refcount_t                 refcnt;               /*   120     4 */

  	/* size: 128, cachelines: 2, members: 17 */
  	/* sum members: 121, holes: 1, sum holes: 3 */
  	/* padding: 4 */
  	/* forced alignments: 1 */
  } __attribute__((__aligned__(8)));
  $

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-bvr8fqfgzxtgnhnwt5sssx5g@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/map_groups.c |  2 +-
 tools/perf/util/map.c         | 30 ------------------------------
 tools/perf/util/map.h         |  1 -
 tools/perf/util/map_groups.h  |  1 -
 tools/perf/util/symbol.c      | 16 ++--------------
 5 files changed, 3 insertions(+), 47 deletions(-)

diff --git a/tools/perf/tests/map_groups.c b/tools/perf/tests/map_groups.c
index b52adad55f8d..6b9f1cdcbe5b 100644
--- a/tools/perf/tests/map_groups.c
+++ b/tools/perf/tests/map_groups.c
@@ -25,7 +25,7 @@ static int check_maps(struct map_def *merged, unsigned int size, struct map_grou
 		TEST_ASSERT_VAL("wrong map start",  map->start == merged[i].start);
 		TEST_ASSERT_VAL("wrong map end",    map->end == merged[i].end);
 		TEST_ASSERT_VAL("wrong map name",  !strcmp(map->dso->name, merged[i].name));
-		TEST_ASSERT_VAL("wrong map refcnt", refcount_read(&map->refcnt) == 2);
+		TEST_ASSERT_VAL("wrong map refcnt", refcount_read(&map->refcnt) == 1);
 
 		i++;
 	}
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 69b9e9b3d915..49e353eaa337 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -26,7 +26,6 @@
 #include "ui/ui.h"
 
 static void __maps__insert(struct maps *maps, struct map *map);
-static void __maps__insert_name(struct maps *maps, struct map *map);
 
 static inline int is_anon_memory(const char *filename, u32 flags)
 {
@@ -566,7 +565,6 @@ u64 map__objdump_2mem(struct map *map, u64 ip)
 static void maps__init(struct maps *maps)
 {
 	maps->entries = RB_ROOT;
-	maps->names = RB_ROOT;
 	init_rwsem(&maps->lock);
 }
 
@@ -589,8 +587,6 @@ static void __maps__purge(struct maps *maps)
 	maps__for_each_entry_safe(maps, pos, next) {
 		rb_erase_init(&pos->rb_node,  &maps->entries);
 		map__put(pos);
-		rb_erase_init(&pos->rb_node_name, &maps->names);
-		map__put(pos);
 	}
 }
 
@@ -736,7 +732,6 @@ size_t map_groups__fprintf(struct map_groups *mg, FILE *fp)
 static void __map_groups__insert(struct map_groups *mg, struct map *map)
 {
 	__maps__insert(&mg->maps, map);
-	__maps__insert_name(&mg->maps, map);
 }
 
 int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, FILE *fp)
@@ -893,32 +888,10 @@ static void __maps__insert(struct maps *maps, struct map *map)
 	map__get(map);
 }
 
-static void __maps__insert_name(struct maps *maps, struct map *map)
-{
-	struct rb_node **p = &maps->names.rb_node;
-	struct rb_node *parent = NULL;
-	struct map *m;
-	int rc;
-
-	while (*p != NULL) {
-		parent = *p;
-		m = rb_entry(parent, struct map, rb_node_name);
-		rc = strcmp(m->dso->short_name, map->dso->short_name);
-		if (rc < 0)
-			p = &(*p)->rb_left;
-		else
-			p = &(*p)->rb_right;
-	}
-	rb_link_node(&map->rb_node_name, parent, p);
-	rb_insert_color(&map->rb_node_name, &maps->names);
-	map__get(map);
-}
-
 void maps__insert(struct maps *maps, struct map *map)
 {
 	down_write(&maps->lock);
 	__maps__insert(maps, map);
-	__maps__insert_name(maps, map);
 	up_write(&maps->lock);
 }
 
@@ -926,9 +899,6 @@ static void __maps__remove(struct maps *maps, struct map *map)
 {
 	rb_erase_init(&map->rb_node, &maps->entries);
 	map__put(map);
-
-	rb_erase_init(&map->rb_node_name, &maps->names);
-	map__put(map);
 }
 
 void maps__remove(struct maps *maps, struct map *map)
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 365deb6375ab..a31e80991189 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -23,7 +23,6 @@ struct map {
 		struct rb_node	rb_node;
 		struct list_head node;
 	};
-	struct rb_node          rb_node_name;
 	u64			start;
 	u64			end;
 	bool			erange_warned;
diff --git a/tools/perf/util/map_groups.h b/tools/perf/util/map_groups.h
index 3f361405a4d4..26fc68bd4f60 100644
--- a/tools/perf/util/map_groups.h
+++ b/tools/perf/util/map_groups.h
@@ -16,7 +16,6 @@ struct thread;
 
 struct maps {
 	struct rb_root      entries;
-	struct rb_root	    names;
 	struct rw_semaphore lock;
 };
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 88f4cfbdb69a..0fb9bd8bcf0d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1764,24 +1764,12 @@ struct map *map_groups__find_by_name(struct map_groups *mg, const char *name)
 {
 	struct maps *maps = &mg->maps;
 	struct map *map;
-	struct rb_node *node;
 
 	down_read(&maps->lock);
 
-	for (node = maps->names.rb_node; node; ) {
-		int rc;
-
-		map = rb_entry(node, struct map, rb_node_name);
-
-		rc = strcmp(map->dso->short_name, name);
-		if (rc < 0)
-			node = node->rb_left;
-		else if (rc > 0)
-			node = node->rb_right;
-		else
-
+	maps__for_each_entry(maps, map)
+		if (strcmp(map->dso->short_name, name) == 0)
 			goto out_unlock;
-	}
 
 	map = NULL;
 
-- 
2.21.0

  parent reply	other threads:[~2019-11-19 11:32 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-19 11:32 [GIT PULL] perf/core improvements and fixes Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 01/25] perf vendor events arm64: Fix commas so PMU event files are valid JSON Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 02/25] perf vendor events power8: " Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 03/25] perf vendor events power9: " Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 04/25] perf scripts python: exported-sql-viewer.py: Fix use of TRUE with SQLite Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 05/25] perf maps: Purge the entries from maps->names in __maps__purge() Arnaldo Carvalho de Melo
2019-11-19 11:32 ` Arnaldo Carvalho de Melo [this message]
2019-11-19 11:32 ` [PATCH 07/25] perf map_groups: Add a front end cache for map lookups by name Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 08/25] perf map: No need to adjust the long name of modules Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 09/25] perf record: No need to process the synthesized MMAP events twice Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 10/25] perf machine: No need to check if kernel module maps pre-exist Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 11/25] perf map_groups: Auto sort maps by name, if needed Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 12/25] perf callchain: Fix segfault in thread__resolve_callchain_sample() Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 13/25] libtraceevent: Fix parsing of event %o and %X argument types Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 14/25] perf map: Use bitmap for booleans Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 15/25] perf map: Move seldom used ->flags field to second cacheline Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 16/25] x86/insn: perf tools: Add some instructions to the new instructions test Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 17/25] x86/insn: Add some Intel instructions to the opcode map Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 18/25] perf probe: Show correct statement line number by perf probe -l Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 19/25] perf probe: Verify given line is a representive line Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 20/25] perf probe: Do not show non representive lines by perf-probe -L Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 21/25] perf probe: Generate event name with line number Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 22/25] perf probe: Support multiprobe event Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 23/25] perf probe: Support DW_AT_const_value constant value Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 24/25] perf probe: Trace a magic number if variable is not found Arnaldo Carvalho de Melo
2019-11-19 11:32 ` [PATCH 25/25] perf parse: Report initial event parsing error Arnaldo Carvalho de Melo
2019-11-19 12:00 ` [GIT PULL] perf/core improvements and fixes Ingo Molnar

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=20191119113245.19593-7-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=williams@redhat.com \
    /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 NNTP newsgroup(s).