From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>,
Ian Rogers <irogers@google.com>,
Kan Liang <kan.liang@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
linux-perf-users@vger.kernel.org
Subject: [PATCH] perf maps: Remove check_invariants() from maps__lock()
Date: Mon, 29 Apr 2024 15:57:38 -0700 [thread overview]
Message-ID: <20240429225738.1491791-1-namhyung@kernel.org> (raw)
I found that the debug build was a lot slowed down by the maps lock code
since it checks the invariants whenever it gets the pointer to the lock.
This means it checks twice the invariants before and after the access.
Instead, let's move the checking code within the lock area but after any
modification and remove it from the read paths. This would remove (more
than) the half of the maps lock overhead.
The time for perf report with a huge data file (200k+ of MMAP2 events).
Non-debug Before After
--------- -------- --------
2m 43s 6m 45s 4m 21s
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/maps.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index 725300896f38..61eb742d91e3 100644
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -211,11 +211,6 @@ void maps__set_unwind_libunwind_ops(struct maps *maps, const struct unwind_libun
static struct rw_semaphore *maps__lock(struct maps *maps)
{
- /*
- * When the lock is acquired or released the maps invariants should
- * hold.
- */
- check_invariants(maps);
return &RC_CHK_ACCESS(maps)->lock;
}
@@ -358,6 +353,7 @@ static int map__strcmp(const void *a, const void *b)
static int maps__sort_by_name(struct maps *maps)
{
int err = 0;
+
down_write(maps__lock(maps));
if (!maps__maps_by_name_sorted(maps)) {
struct map **maps_by_name = maps__maps_by_name(maps);
@@ -384,6 +380,7 @@ static int maps__sort_by_name(struct maps *maps)
maps__set_maps_by_name_sorted(maps, true);
}
}
+ check_invariants(maps);
up_write(maps__lock(maps));
return err;
}
@@ -502,6 +499,7 @@ int maps__insert(struct maps *maps, struct map *map)
down_write(maps__lock(maps));
ret = __maps__insert(maps, map);
+ check_invariants(maps);
up_write(maps__lock(maps));
return ret;
}
@@ -536,6 +534,7 @@ void maps__remove(struct maps *maps, struct map *map)
{
down_write(maps__lock(maps));
__maps__remove(maps, map);
+ check_invariants(maps);
up_write(maps__lock(maps));
}
@@ -602,6 +601,7 @@ void maps__remove_maps(struct maps *maps, bool (*cb)(struct map *map, void *data
else
i++;
}
+ check_invariants(maps);
up_write(maps__lock(maps));
}
@@ -942,6 +942,8 @@ int maps__copy_from(struct maps *dest, struct maps *parent)
map__put(new);
}
}
+ check_invariants(dest);
+
up_read(maps__lock(parent));
up_write(maps__lock(dest));
return err;
@@ -1097,6 +1099,7 @@ void maps__fixup_end(struct maps *maps)
map__set_end(maps_by_address[n - 1], ~0ULL);
RC_CHK_ACCESS(maps)->ends_broken = false;
+ check_invariants(maps);
up_write(maps__lock(maps));
}
@@ -1147,6 +1150,8 @@ int maps__merge_in(struct maps *kmaps, struct map *new_map)
map__start(kmaps_maps_by_address[first_after_]) >= map__end(new_map)) {
/* No overlap so regular insert suffices. */
int ret = __maps__insert(kmaps, new_map);
+
+ check_invariants(kmaps);
up_write(maps__lock(kmaps));
return ret;
}
@@ -1184,6 +1189,7 @@ int maps__merge_in(struct maps *kmaps, struct map *new_map)
map__zput(kmaps_maps_by_address[i]);
free(kmaps_maps_by_address);
+ check_invariants(kmaps);
up_write(maps__lock(kmaps));
return 0;
}
--
2.44.0.769.g3c40516874-goog
next reply other threads:[~2024-04-29 22:57 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 22:57 Namhyung Kim [this message]
2024-04-30 2:09 ` [PATCH] perf maps: Remove check_invariants() from maps__lock() Ian Rogers
2024-05-02 19:38 ` Arnaldo Carvalho de Melo
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=20240429225738.1491791-1-namhyung@kernel.org \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).