perf tools: Speedup DWARF unwind

Hi Jiri,

On Thu, 15 May 2014 19:23:27 +0200, Jiri Olsa wrote:

[SNIP]

tools/perf/util/dso.c static void close_dso(struct dso *dso) · line 255

Quoted 7 lines

tools/perf/util/dso.c +7 -0

+static void data_close(void)
+{
+	bool cache_fd = may_cache_fd();
+
+	if (!cache_fd)
+		close_first_dso();
+}

Why do you do this at close()? As long as there's no attempt to open a new file, we can keep existing fd, no?

tools/perf/util/dso.c static void close_dso(struct dso *dso) · line 261

Quoted 7 lines

tools/perf/util/dso.c +6 -0

+
+void dso__data_close(struct dso *dso)
+{
+	if (dso->data.fd >= 0)
+		data_close();
+}

Hmm.. it's confusing dso__data_close(dso) closes an other dso rather than the given dso. And this dso__data_close() is not paired with any _open() also these close calls make me confusing which one to use. ;-p

Quoted 5 lines · Namhyung Kim

Hi Jiri,

On Thu, 15 May 2014 19:23:27 +0200, Jiri Olsa wrote:

[SNIP]

Quoted 10 lines

tools/perf/util/dso.c static void close_dso(struct dso *dso) · line 255

Quoted 7 lines

tools/perf/util/dso.c +7 -0

+static void data_close(void)
+{
+	bool cache_fd = may_cache_fd();
+
+	if (!cache_fd)
+		close_first_dso();
+}

Why do you do this at close()? As long as there's no attempt to open a new file, we can keep existing fd, no?

so the way it works now is:

- we keep up to the 'RLIMIT_NOFILE / 2' of open dso objects
- if we try to open dso and it fails, because we are out of file descriptors, we close dso objects and try to reopen (check do_open function)
- when we close the dso object we check if number of opened dso objects is below 'RLIMIT_NOFILE / 2'.. if it is, we keep the dso opened, if not we close first dso in the list

util/dso.h tries to describe that

Quoted 12 lines

tools/perf/util/dso.c static void close_dso(struct dso *dso) · line 261

Quoted 8 lines

tools/perf/util/dso.c +6 -0

+
+void dso__data_close(struct dso *dso)
+{
+	if (dso->data.fd >= 0)
+		data_close();
+}

Hmm.. it's confusing dso__data_close(dso) closes an other dso rather than the given dso. And this dso__data_close() is not paired with any _open() also these close calls make me confusing which one to use. ;-p

thats due to the caching.. as explained above

About the pairing.. originally the interface was only dso__data_fd that opened and returned fd, which the caller needed to close.

I added dso__data_close so we could keep track of file descriptors.

I could add dso__data_open I guess, but it is dso__data_fd which is needed for elf interface anyway.

Quoted 27 lines · Jiri Olsa

Quoted 5 lines · Namhyung Kim

Hi Jiri,

On Thu, 15 May 2014 19:23:27 +0200, Jiri Olsa wrote:

[SNIP]

Quoted 10 lines

tools/perf/util/dso.c static void close_dso(struct dso *dso) · line 255

Quoted 7 lines

tools/perf/util/dso.c +7 -0

+static void data_close(void)
+{
+	bool cache_fd = may_cache_fd();
+
+	if (!cache_fd)
+		close_first_dso();
+}

Why do you do this at close()? As long as there's no attempt to open a new file, we can keep existing fd, no?

so the way it works now is:

- we keep up to the 'RLIMIT_NOFILE / 2' of open dso objects
- if we try to open dso and it fails, because we are out of file descriptors, we close dso objects and try to reopen (check do_open function)
- when we close the dso object we check if number of opened dso objects is below 'RLIMIT_NOFILE / 2'.. if it is, we keep the dso opened, if not we close first dso in the list

Quoted 1 line

util/dso.h tries to describe that

Yes, I know. But my question is why do this at close()? Isn't it sufficient to check the file limit at open() and close previous one if necessary?

+/**
+ * dso__data_read_addr - Read data from dso address
+ * @dso: dso object
+ * @machine: machine object
+ * @offset: file offset

+ * @data: buffer to store data
+ * @size: size of the @data buffer
+ *
+ * Read data from dso address.
+ */
ssize_t dso__data_read_addr(struct dso *dso, struct map *map,

tools/perf/util/dso.c +0 -0

 			    struct machine *machine, u64 addr,
 			    u8 *data, ssize_t size)

On Thu, 15 May 2014 19:23:30 +0200, Jiri Olsa wrote:

[SNIP]

Quoted 5 lines

+/**
+ * dso__data_read_addr - Read data from dso address
+ * @dso: dso object
+ * @machine: machine object
+ * @offset: file offset

s/offset/addr/

In upcoming tests we will setup process limits, which might affect other tests. Spawning child for each test to prevent this.

Quoted 3 lines · Jiri Olsa

In upcoming tests we will setup process limits, which might affect other tests. Spawning child for each test to prevent this.

But you can restore original limits after the test using soft limits? But I think it's better to run the tests in a child anyway. :)

Thanks,
Namhyung

Adding a way to setup test dso limit by global variable test_dso_data__fd_limit. It'll be used in the dso data cache tests.

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Corey Ashford <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Jean Pihet <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/dso.c | 5 +++++
 tools/perf/util/dso.h | 3 +++
 2 files changed, 8 insertions(+)

tools/perf/util/dso.c +5 -0

index 80b54e7..9310369 100644
@@ -7,6 +7,8 @@
 #include "util.h"
 #include "debug.h"
 
+rlim_t test_dso_data__fd_limit;
+
 char dso__symtab_origin(const struct dso *dso)
 {
 	static const char origin[] = {

@@ -262,6 +264,9 @@ static bool may_cache_fd(void)
 	if (!limit)
 		limit = get_fd_limit();
 
+	if (unlikely(test_dso_data__fd_limit))
+		limit = test_dso_data__fd_limit;
+
 	if (limit == RLIM_INFINITY)
 		return true;
 

tools/perf/util/dso.h +3 -0

index d713184..3807014 100644
@@ -1,6 +1,7 @@
 #ifndef __PERF_DSO
 #define __PERF_DSO
 
+#include <sys/resource.h>
 #include <linux/types.h>
 #include <linux/rbtree.h>
 #include <stdbool.h>

@@ -8,6 +9,8 @@
 #include "map.h"
 #include "build-id.h"
 
+extern rlim_t test_dso_data__fd_limit;
+
 enum dso_binary_type {
 	DSO_BINARY_TYPE__KALLSYMS = 0,
 	DSO_BINARY_TYPE__GUEST_KALLSYMS,

Quoted 3 lines · Jiri Olsa

Adding a way to setup test dso limit by global variable test_dso_data__fd_limit. It'll be used in the dso data cache tests.

Why is this needed? Why not justing setting RLIMIT_NOFILE in the test cases?

Adding test that setup test_dso_data__fd_limit and test dso data file descriptors are cached appropriately.

tools/perf/tests/dso-data.c +17 -0

+
+	pr_debug("fd path: %s\n", path);
+
+	dir = opendir(path);
+	TEST_ASSERT_VAL("failed to open fd directory", dir);
+
+	while ((dent = readdir(dir)) != NULL) {
+		if (!strcmp(dent->d_name, ".") ||
+		    !strcmp(dent->d_name, ".."))
+			continue;
+
+		nr++;
+	}
+
+	closedir(dir);
+	return nr - 1;
+}

tools/perf/tests/dso-data.c +17 -0

+static int set_fd_limit(int n)
+{
+	struct rlimit rlim;
+
+	if (getrlimit(RLIMIT_NOFILE, &rlim))
+		return -1;
+
+	pr_debug("file limit %ld, new %d\n", (long) rlim.rlim_cur, n);
+
+	rlim.rlim_cur = n;
+	return setrlimit(RLIMIT_NOFILE, &rlim);
+}
+
+int test__dso_data_cache(void)
+{
+	struct machine machine;
+	long nr = open_files_cnt();

tools/perf/tests/dso-data.c +13 -0

+	 * dso_0 should remain open.
+	 */
+	dso__data_close(dso_0);
+	TEST_ASSERT_VAL("failed to not close dso", dso_0->data.fd != -1);
+
+	/* open dso_1 */
+	n = dso__data_read_offset(dso_1, &machine, 0, buf, BUFSIZE);
+	TEST_ASSERT_VAL("failed to read dso", n == BUFSIZE);
+
+	/*
+	 * Close dso_1 data with cache = true,
+	 * dso_0 and dso_1 should remain open.
+	 */

tools/perf/tests/dso-data.c +39 -0

+	 * dso_0 should get closed now
+	 */
+	dso__data_close(dso_2);
+	TEST_ASSERT_VAL("failed to close dso_0", dso_0->data.fd == -1);
+
+	/* reopen dso_0 */
+	fd = dso__data_fd(dso_0, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/*
+	 * Close dso_0 data with cache = true,
+	 * dso_1 should get closed now.
+	 */
+	dso__data_close(dso_0);
+	TEST_ASSERT_VAL("failed to close dso_1", dso_1->data.fd == -1);
+
+	/* reopen dso_1 */
+	n = dso__data_read_offset(dso_1, &machine, 0, buf, BUFSIZE);
+	TEST_ASSERT_VAL("failed to read dso", n == BUFSIZE);
+
+	/*
+	 * Close dso_1 data with cache = true,
+	 * dso_2 should get closed now.
+	 */
+	dso__data_close(dso_1);
+	TEST_ASSERT_VAL("failed to close dso_2", dso_2->data.fd == -1);
+
+	/* dso_0 remains open */
+	TEST_ASSERT_VAL("failed to keep open dso_0", dso_0->data.fd >= 0);
+
+	/* cleanup everything */
+	dsos__delete();
+
+	pr_debug("nr start %ld, nr stop %ld\n", nr, open_files_cnt());
+
+	/* Make sure we did not leak any file descriptor. */
+	TEST_ASSERT_VAL("failed leadking files", nr == open_files_cnt());
+	return 0;
+}

tools/perf/tests/tests.h +1 -0

index a9d7cb0..61e12b6 100644
@@ -28,6 +28,7 @@ int test__syscall_open_tp_fields(void);
 int test__pmu(void);
 int test__attr(void);
 int test__dso_data(void);
+int test__dso_data_cache(void);
 int test__parse_events(void);
 int test__hists_link(void);
 int test__python_use(void);

Quoted 2 lines · Jiri Olsa

Adding test that setup test_dso_data__fd_limit and test dso data file descriptors are cached appropriately.

[SNIP]

tools/perf/tests/dso-data.c int test__dso_data(void) · line 168

Quoted 10 lines

tools/perf/tests/dso-data.c +10 -0

+static long open_files_cnt(void)
+{
+	char path[PATH_MAX];
+	struct dirent *dent;
+	DIR *dir;
+	long nr = 0;
+	int n;
+
+	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
+	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);

Looks like an unnecessary check since the scnprintf() cannot return more than (or equal to) PATH_MAX.

Quoted 5 lines · Namhyung Kim

Quoted 2 lines · Jiri Olsa

Adding test that setup test_dso_data__fd_limit and test dso data file descriptors are cached appropriately.

[SNIP]

Quoted 13 lines

tools/perf/tests/dso-data.c int test__dso_data(void) · line 168

Quoted 10 lines

tools/perf/tests/dso-data.c +10 -0

+static long open_files_cnt(void)
+{
+	char path[PATH_MAX];
+	struct dirent *dent;
+	DIR *dir;
+	long nr = 0;
+	int n;
+
+	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
+	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);

Looks like an unnecessary check since the scnprintf() cannot return more than (or equal to) PATH_MAX.

once it's equal it's bad.. as the man says:
"return value of size or more means that the output was truncated"

Quoted 20 lines · Jiri Olsa

Quoted 18 lines · Namhyung Kim

Quoted 2 lines · Jiri Olsa

Adding test that setup test_dso_data__fd_limit and test dso data file descriptors are cached appropriately.

[SNIP]

tools/perf/tests/dso-data.c int test__dso_data(void) · line 168

Quoted 10 lines

tools/perf/tests/dso-data.c +10 -0

+static long open_files_cnt(void)
+{
+	char path[PATH_MAX];
+	struct dirent *dent;
+	DIR *dir;
+	long nr = 0;
+	int n;
+
+	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
+	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);

Looks like an unnecessary check since the scnprintf() cannot return more than (or equal to) PATH_MAX.

Quoted 2 lines

once it's equal it's bad.. as the man says:
"return value of size or more means that the output was truncated"

Did you see "sn"printf?

Quoted 21 lines · Namhyung Kim

Quoted 20 lines · Jiri Olsa

Quoted 18 lines · Namhyung Kim

Quoted 2 lines · Jiri Olsa

Adding test that setup test_dso_data__fd_limit and test dso data file descriptors are cached appropriately.

[SNIP]

tools/perf/tests/dso-data.c int test__dso_data(void) · line 168

Quoted 10 lines

tools/perf/tests/dso-data.c +10 -0

+static long open_files_cnt(void)
+{
+	char path[PATH_MAX];
+	struct dirent *dent;
+	DIR *dir;
+	long nr = 0;
+	int n;
+
+	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
+	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);

Looks like an unnecessary check since the scnprintf() cannot return more than (or equal to) PATH_MAX.

Quoted 4 lines

Quoted 2 lines

once it's equal it's bad.. as the man says:
"return value of size or more means that the output was truncated"

Did you see "sn"printf?

yes, I just double checked.. jirka

Hi Jiri,

Quoted 25 lines · Jiri Olsa

Quoted 24 lines · Namhyung Kim

Quoted 22 lines · Jiri Olsa

Quoted 5 lines · Namhyung Kim

Quoted 2 lines · Jiri Olsa

Adding test that setup test_dso_data__fd_limit and test dso data file descriptors are cached appropriately.

[SNIP]

Quoted 13 lines

tools/perf/tests/dso-data.c int test__dso_data(void) · line 168

Quoted 10 lines

tools/perf/tests/dso-data.c +10 -0

+static long open_files_cnt(void)
+{
+	char path[PATH_MAX];
+	struct dirent *dent;
+	DIR *dir;
+	long nr = 0;
+	int n;
+
+	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
+	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);

Looks like an unnecessary check since the scnprintf() cannot return more than (or equal to) PATH_MAX.

once it's equal it's bad.. as the man says:
"return value of size or more means that the output was truncated"

Quoted 3 lines

Quoted 1 line

Did you see "sn"printf?

yes, I just double checked.. jirka

Please see "scn"printf then. :) It does something like below..

  i = snprintf(buf, size, ...);
  return (i >= size) ? : size - 1 : i;

Testing that perf properly closes opened dso objects and tries to reopen in case we run out of allowed file descriptors for dso data.

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Corey Ashford <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Jean Pihet <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/tests/builtin-test.c |  4 +++
 tools/perf/tests/dso-data.c     | 70 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |  1 +
 3 files changed, 75 insertions(+)

tools/perf/tests/builtin-test.c +4 -0

index c4d581a..a489cda 100644
@@ -60,6 +60,10 @@ static struct test {
 		.func = test__dso_data_cache,
 	},
 	{
+		.desc = "Test dso data reopen",
+		.func = test__dso_data_reopen,
+	},
+	{
 		.desc = "roundtrip evsel->name check",
 		.func = test__perf_evsel__roundtrip_name_test,
 	},

tools/perf/tests/dso-data.c +0 -0

index 84ab939..ecc8acd 100644

tools/perf/tests/dso-data.c +43 -0

+	int fd, fd_extra;
+
+	memset(&machine, 0, sizeof(machine));
+
+	/*
+	 * Test scenario:
+	 * - create 3 dso objects
+	 * - set process file descriptor limit to current
+	 *   files count + 3
+	 * - test that the first dso gets closed when we
+	 *   reach the files count limit
+	 */
+
+	/* Make sure we are able to open 3 fds anyway */
+	TEST_ASSERT_VAL("failed to set file limit",
+			!set_fd_limit((nr + 3)));
+
+	TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(TEST_FILE_SIZE));
+
+	/* open dso_0 */
+	fd = dso__data_fd(dso_0, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/* open dso_1 */
+	fd = dso__data_fd(dso_1, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/*
+	 * open extra file descriptor and we just
+	 * reached the files count limit
+	 */
+	fd_extra = open("/dev/null", O_RDONLY);
+	TEST_ASSERT_VAL("failed to open extra fd", fd_extra > 0);
+
+	/* open dso_2 */
+	fd = dso__data_fd(dso_2, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/*
+	 * dso_0 should get closed, because we reached
+	 * the file descriptor limit
+	 */
+	TEST_ASSERT_VAL("failed to close dso_0", dso_0->data.fd == -1);

tools/perf/tests/dso-data.c +11 -0

+
+	/* cleanup everything */
+	close(fd_extra);
+	dsos__delete();
+
+	pr_debug("nr start %ld, nr stop %ld\n", nr, open_files_cnt());
+
+	/* Make sure we did not leak any file descriptor. */
+	TEST_ASSERT_VAL("failed leadking files", nr == open_files_cnt());
+	return 0;
+}

tools/perf/tests/tests.h +1 -0

index 61e12b6..3247ca1 100644
@@ -29,6 +29,7 @@ int test__pmu(void);
 int test__attr(void);
 int test__dso_data(void);
 int test__dso_data_cache(void);
+int test__dso_data_reopen(void);
 int test__parse_events(void);
 int test__hists_link(void);
 int test__python_use(void);

Quoted 43 lines · Jiri Olsa

Testing that perf properly closes opened dso objects and tries to reopen in case we run out of allowed file descriptors for dso data.

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Corey Ashford <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Jean Pihet <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/tests/builtin-test.c |  4 +++
 tools/perf/tests/dso-data.c     | 70 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |  1 +
 3 files changed, 75 insertions(+)

tools/perf/tests/builtin-test.c +4 -0

index c4d581a..a489cda 100644
@@ -60,6 +60,10 @@ static struct test {
 		.func = test__dso_data_cache,
 	},
 	{
+		.desc = "Test dso data reopen",
+		.func = test__dso_data_reopen,
+	},
+	{
 		.desc = "roundtrip evsel->name check",
 		.func = test__perf_evsel__roundtrip_name_test,
 	},

tools/perf/tests/dso-data.c +0 -0

index 84ab939..ecc8acd 100644
@@ -328,3 +328,73 @@ int test__dso_data_cache(void)
 	TEST_ASSERT_VAL("failed leadking files", nr == open_files_cnt());
 	return 0;
 }

tools/perf/tests/dso-data.c int test__dso_data_cache(void) · line 336

Quoted 6 lines

tools/perf/tests/dso-data.c +6 -0

+
+int test__dso_data_reopen(void)
+{
+	struct machine machine;
+	long nr = open_files_cnt();
+#define BUFSIZE 10

Looks like a copy-n-paste error.. :)

tools/perf/tests/dso-data.c int test__dso_data_cache(void) · line 389

Quoted 6 lines

tools/perf/tests/dso-data.c +6 -0

+
+	/*
+	 * dso_1 should get closed, because we reached
+	 * the file descriptor limit
+	 */
+	TEST_ASSERT_VAL("failed to close dso_0", dso_1->data.fd == -1);

s/dso_0/dso_1/

Btw, I don't see a big difference between this and previous testcase. Any chance to merge them into one?

hi,
trying to speedup DWARF unwind report code by factoring related code:
- caching sample's registers access
- keep dso data file descriptor open for the life of the dso object
- replace dso cache code by mapping dso data file directly for the life of the dso object

The speedup is mainly for libunwind unwind. The libdw will benefit mainly from cached registers access, because it handles dso data accesses by itself.. and anyway it's still faster ;-).

v2 changes:
- adding limit for open dso objects with sort of LRU mechanism to pick up and close dso objects if we are over the limit
- file mmaping changes are omitted, because I couldn't prove the improvement exactly, will resubmit later
- added dso close logic in case of no memory
- added tests

On 10GB perf data file with dwarf unwind data I've got around 30% speed up.

Also reachable in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/core_unwind_speedup

thanks,
jirka

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Corey Ashford <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Jean Pihet <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
Jiri Olsa (14):
perf tools: Cache register accesses for unwind processing perf tools: Separate dso data related variables
perf tools: Add data_fd into dso object
perf tools: Add global list of opened dso objects perf tools: Add global count of opened dso objects perf tools: Cache dso data file descriptor
perf tools: Add file size check and factor dso__data_read_offset perf tools: Allow to close dso fd in case of open failure perf tools: Add dso__data_* interface descriptons perf tests: Spawn child for each test
perf tests: Allow reuse of test_file function
perf tests: Add test interface for dso data fd limit perf tests: Add test for caching dso file descriptors perf tests: Add test for closing dso objects on EMFILE error

 tools/perf/tests/builtin-test.c    |  42 ++++++++++++++-
 tools/perf/tests/dso-data.c        | 256 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 tools/perf/tests/tests.h           |   2 +
 tools/perf/util/dso.c              | 274 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 tools/perf/util/dso.h              |  56 +++++++++++++++++++-
 tools/perf/util/event.h            |   5 ++
 tools/perf/util/perf_regs.c        |  10 +++-
 tools/perf/util/perf_regs.h        |   4 +-
 tools/perf/util/unwind-libunwind.c |   4 +-
 9 files changed, 617 insertions(+), 36 deletions(-)

ping ;-)

thanks,
jirka

Quoted 69 lines · Jiri Olsa

hi,
trying to speedup DWARF unwind report code by factoring related code:
- caching sample's registers access
- keep dso data file descriptor open for the life of the dso object
- replace dso cache code by mapping dso data file directly for the life of the dso object

The speedup is mainly for libunwind unwind. The libdw will benefit mainly from cached registers access, because it handles dso data accesses by itself.. and anyway it's still faster ;-).

v2 changes:
- adding limit for open dso objects with sort of LRU mechanism to pick up and close dso objects if we are over the limit
- file mmaping changes are omitted, because I couldn't prove the improvement exactly, will resubmit later
- added dso close logic in case of no memory
- added tests

On 10GB perf data file with dwarf unwind data I've got around 30% speed up.

Also reachable in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/core_unwind_speedup

thanks,
jirka

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Corey Ashford <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Jean Pihet <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
Jiri Olsa (14):
perf tools: Cache register accesses for unwind processing perf tools: Separate dso data related variables
perf tools: Add data_fd into dso object
perf tools: Add global list of opened dso objects perf tools: Add global count of opened dso objects perf tools: Cache dso data file descriptor
perf tools: Add file size check and factor dso__data_read_offset perf tools: Allow to close dso fd in case of open failure perf tools: Add dso__data_* interface descriptons perf tests: Spawn child for each test
perf tests: Allow reuse of test_file function
perf tests: Add test interface for dso data fd limit perf tests: Add test for caching dso file descriptors perf tests: Add test for closing dso objects on EMFILE error

 tools/perf/tests/builtin-test.c    |  42 ++++++++++++++-
 tools/perf/tests/dso-data.c        | 256 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 tools/perf/tests/tests.h           |   2 +
 tools/perf/util/dso.c              | 274 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 tools/perf/util/dso.h              |  56 +++++++++++++++++++-
 tools/perf/util/event.h            |   5 ++
 tools/perf/util/perf_regs.c        |  10 +++-
 tools/perf/util/perf_regs.h        |   4 +-
 tools/perf/util/unwind-libunwind.c |   4 +-
 9 files changed, 617 insertions(+), 36 deletions(-)

Jiri,

Quoted 1 line · Jiri Olsa

ping ;-)

This looks a ping to yourself ;-)

Here are the results on ARMv7:
- libunwind: between -29% in execution time for light load (i.e. using not-so-deep backtraces from the stress app.) and -49% for deep backtrace (the 'stress_bt' app.),
- libdw: no significant improvement (0-2% improvement).

Cf. https://wiki.linaro.org/LEG/Engineering/TOOLS/perf-callstack-unwinding#Speed_improvement
for more details.

FWIW:

Acked-by Jean Pihet <[email protected]>