* [PATCH 1/5] gdb.perf/: Fix tcl_string_list_to_python_list {x}
2022-05-25 22:37 [PATCH 0/5] gdb.perf/: Add JIT performance test Ilya Leoshkevich
@ 2022-05-25 22:37 ` Ilya Leoshkevich
2022-05-26 14:39 ` Andrew Burgess
2022-05-25 22:37 ` [PATCH 2/5] gdb.perf/: Add binary_link_with_shlibs setting to GenPerfTest Ilya Leoshkevich
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2022-05-25 22:37 UTC (permalink / raw)
To: Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Andrew Burgess, Pedro Alves,
gdb-patches, Ilya Leoshkevich
tcl_string_list_to_python_list {x} returns "(x)", which is not a tuple
(the valid tuple syntax would be "(x,)"). Instead of special-casing
this, just change the function to return what its name says: lists.
This way in the problematic case we will get "[x]".
---
gdb/testsuite/lib/perftest.exp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/testsuite/lib/perftest.exp b/gdb/testsuite/lib/perftest.exp
index bf4eb45bba5..0df4ff15864 100644
--- a/gdb/testsuite/lib/perftest.exp
+++ b/gdb/testsuite/lib/perftest.exp
@@ -159,7 +159,7 @@ proc tcl_string_list_to_python_list { l } {
foreach elm $l {
lappend quoted_list [quote $elm]
}
- return "([join $quoted_list {, }])"
+ return "\[[join $quoted_list {, }]\]"
}
# Helper routine for PerfTest::assemble "run" step implementations.
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/5] gdb.perf/: Fix tcl_string_list_to_python_list {x}
2022-05-25 22:37 ` [PATCH 1/5] gdb.perf/: Fix tcl_string_list_to_python_list {x} Ilya Leoshkevich
@ 2022-05-26 14:39 ` Andrew Burgess
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-05-26 14:39 UTC (permalink / raw)
To: Ilya Leoshkevich via Gdb-patches, Tom Tromey
Cc: Ilya Leoshkevich, Pedro Alves, Ulrich Weigand, Andreas Arnez,
gdb-patches
Ilya Leoshkevich via Gdb-patches <gdb-patches@sourceware.org> writes:
> tcl_string_list_to_python_list {x} returns "(x)", which is not a tuple
> (the valid tuple syntax would be "(x,)"). Instead of special-casing
> this, just change the function to return what its name says: lists.
> This way in the problematic case we will get "[x]".
LGTM.
Thanks,
Andrew
> ---
> gdb/testsuite/lib/perftest.exp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/lib/perftest.exp b/gdb/testsuite/lib/perftest.exp
> index bf4eb45bba5..0df4ff15864 100644
> --- a/gdb/testsuite/lib/perftest.exp
> +++ b/gdb/testsuite/lib/perftest.exp
> @@ -159,7 +159,7 @@ proc tcl_string_list_to_python_list { l } {
> foreach elm $l {
> lappend quoted_list [quote $elm]
> }
> - return "([join $quoted_list {, }])"
> + return "\[[join $quoted_list {, }]\]"
> }
>
> # Helper routine for PerfTest::assemble "run" step implementations.
> --
> 2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/5] gdb.perf/: Add binary_link_with_shlibs setting to GenPerfTest
2022-05-25 22:37 [PATCH 0/5] gdb.perf/: Add JIT performance test Ilya Leoshkevich
2022-05-25 22:37 ` [PATCH 1/5] gdb.perf/: Fix tcl_string_list_to_python_list {x} Ilya Leoshkevich
@ 2022-05-25 22:37 ` Ilya Leoshkevich
2022-05-26 15:38 ` Andrew Burgess
2022-05-25 22:37 ` [PATCH 3/5] gdb.perf/: Compile the binary with -DSHLIB Ilya Leoshkevich
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2022-05-25 22:37 UTC (permalink / raw)
To: Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Andrew Burgess, Pedro Alves,
gdb-patches, Ilya Leoshkevich
Performance tests that dynamically load generated shared libraries do
not need to link with them. Provide an option to avoid this.
---
gdb/testsuite/lib/gen-perf-test.exp | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/gdb/testsuite/lib/gen-perf-test.exp b/gdb/testsuite/lib/gen-perf-test.exp
index 0a5607c2c9a..f0bfdc9317d 100644
--- a/gdb/testsuite/lib/gen-perf-test.exp
+++ b/gdb/testsuite/lib/gen-perf-test.exp
@@ -100,6 +100,9 @@ namespace eval GenPerfTest {
# The number of compunits in each objfile.
set DEFAULT_NR_COMPUNITS 1
+ # Link the binary with the generated shared libraries by default
+ set DEFAULT_BINARY_LINK_WITH_SHLIBS 1
+
# The number of public globals in each compunit.
set DEFAULT_NR_EXTERN_GLOBALS 1
@@ -254,6 +257,7 @@ namespace eval GenPerfTest {
set testcase(tail_shlib_headers) $GenPerfTest::DEFAULT_TAIL_SHLIB_HEADERS
set testcase(nr_gen_shlibs) $GenPerfTest::DEFAULT_NR_GEN_SHLIBS
set testcase(nr_compunits) $GenPerfTest::DEFAULT_NR_COMPUNITS
+ set testcase(binary_link_with_shlibs) $GenPerfTest::DEFAULT_BINARY_LINK_WITH_SHLIBS
set testcase(nr_extern_globals) $GenPerfTest::DEFAULT_NR_EXTERN_GLOBALS
set testcase(nr_static_globals) $GenPerfTest::DEFAULT_NR_STATIC_GLOBALS
@@ -281,7 +285,7 @@ namespace eval GenPerfTest {
binary_extra_sources binary_extra_headers
gen_shlib_extra_sources gen_shlib_extra_headers
tail_shlib_sources tail_shlib_headers
- nr_gen_shlibs nr_compunits
+ nr_gen_shlibs nr_compunits binary_link_with_shlibs
nr_extern_globals nr_static_globals
nr_extern_functions nr_static_functions
class_specs
@@ -1298,14 +1302,16 @@ namespace eval GenPerfTest {
proc _make_shlib_options { self_var static run_nr } {
upvar 1 $self_var self
- set nr_gen_shlibs [_get_param $self(nr_gen_shlibs) $run_nr]
set result ""
- for { set i 0 } { $i < $nr_gen_shlibs } { incr i } {
- lappend result "shlib=[_make_shlib_name self $static $run_nr $i]"
- }
- set tail_shlib_name [_make_tail_shlib_name self $static $run_nr]
- if { "$tail_shlib_name" != "" } {
- lappend result "shlib=$tail_shlib_name"
+ if { [_get_param $self(binary_link_with_shlibs) $run_nr] } {
+ set nr_gen_shlibs [_get_param $self(nr_gen_shlibs) $run_nr]
+ for { set i 0 } { $i < $nr_gen_shlibs } { incr i } {
+ lappend result "shlib=[_make_shlib_name self $static $run_nr $i]"
+ }
+ set tail_shlib_name [_make_tail_shlib_name self $static $run_nr]
+ if { "$tail_shlib_name" != "" } {
+ lappend result "shlib=$tail_shlib_name"
+ }
}
return $result
}
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/5] gdb.perf/: Add binary_link_with_shlibs setting to GenPerfTest
2022-05-25 22:37 ` [PATCH 2/5] gdb.perf/: Add binary_link_with_shlibs setting to GenPerfTest Ilya Leoshkevich
@ 2022-05-26 15:38 ` Andrew Burgess
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-05-26 15:38 UTC (permalink / raw)
To: Ilya Leoshkevich, Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Pedro Alves, gdb-patches,
Ilya Leoshkevich
Ilya Leoshkevich <iii@linux.ibm.com> writes:
> Performance tests that dynamically load generated shared libraries do
> not need to link with them. Provide an option to avoid this.
> ---
> gdb/testsuite/lib/gen-perf-test.exp | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/testsuite/lib/gen-perf-test.exp b/gdb/testsuite/lib/gen-perf-test.exp
> index 0a5607c2c9a..f0bfdc9317d 100644
> --- a/gdb/testsuite/lib/gen-perf-test.exp
> +++ b/gdb/testsuite/lib/gen-perf-test.exp
> @@ -100,6 +100,9 @@ namespace eval GenPerfTest {
> # The number of compunits in each objfile.
> set DEFAULT_NR_COMPUNITS 1
>
> + # Link the binary with the generated shared libraries by default
> + set DEFAULT_BINARY_LINK_WITH_SHLIBS 1
I think this should be:
# Should the binary be linked with the generated shared libraries?
set DEFAULT_BINARY_LINK_WITH_SHLIBS true
As this matches the style of the existing comment. Plus use 'true'
rather than '1'.
OK with those changes.
Thanks,
Andrew
> +
> # The number of public globals in each compunit.
> set DEFAULT_NR_EXTERN_GLOBALS 1
>
> @@ -254,6 +257,7 @@ namespace eval GenPerfTest {
> set testcase(tail_shlib_headers) $GenPerfTest::DEFAULT_TAIL_SHLIB_HEADERS
> set testcase(nr_gen_shlibs) $GenPerfTest::DEFAULT_NR_GEN_SHLIBS
> set testcase(nr_compunits) $GenPerfTest::DEFAULT_NR_COMPUNITS
> + set testcase(binary_link_with_shlibs) $GenPerfTest::DEFAULT_BINARY_LINK_WITH_SHLIBS
>
> set testcase(nr_extern_globals) $GenPerfTest::DEFAULT_NR_EXTERN_GLOBALS
> set testcase(nr_static_globals) $GenPerfTest::DEFAULT_NR_STATIC_GLOBALS
> @@ -281,7 +285,7 @@ namespace eval GenPerfTest {
> binary_extra_sources binary_extra_headers
> gen_shlib_extra_sources gen_shlib_extra_headers
> tail_shlib_sources tail_shlib_headers
> - nr_gen_shlibs nr_compunits
> + nr_gen_shlibs nr_compunits binary_link_with_shlibs
> nr_extern_globals nr_static_globals
> nr_extern_functions nr_static_functions
> class_specs
> @@ -1298,14 +1302,16 @@ namespace eval GenPerfTest {
>
> proc _make_shlib_options { self_var static run_nr } {
> upvar 1 $self_var self
> - set nr_gen_shlibs [_get_param $self(nr_gen_shlibs) $run_nr]
> set result ""
> - for { set i 0 } { $i < $nr_gen_shlibs } { incr i } {
> - lappend result "shlib=[_make_shlib_name self $static $run_nr $i]"
> - }
> - set tail_shlib_name [_make_tail_shlib_name self $static $run_nr]
> - if { "$tail_shlib_name" != "" } {
> - lappend result "shlib=$tail_shlib_name"
> + if { [_get_param $self(binary_link_with_shlibs) $run_nr] } {
> + set nr_gen_shlibs [_get_param $self(nr_gen_shlibs) $run_nr]
> + for { set i 0 } { $i < $nr_gen_shlibs } { incr i } {
> + lappend result "shlib=[_make_shlib_name self $static $run_nr $i]"
> + }
> + set tail_shlib_name [_make_tail_shlib_name self $static $run_nr]
> + if { "$tail_shlib_name" != "" } {
> + lappend result "shlib=$tail_shlib_name"
> + }
> }
> return $result
> }
> --
> 2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/5] gdb.perf/: Compile the binary with -DSHLIB
2022-05-25 22:37 [PATCH 0/5] gdb.perf/: Add JIT performance test Ilya Leoshkevich
2022-05-25 22:37 ` [PATCH 1/5] gdb.perf/: Fix tcl_string_list_to_python_list {x} Ilya Leoshkevich
2022-05-25 22:37 ` [PATCH 2/5] gdb.perf/: Add binary_link_with_shlibs setting to GenPerfTest Ilya Leoshkevich
@ 2022-05-25 22:37 ` Ilya Leoshkevich
2022-05-26 15:43 ` Andrew Burgess
2022-05-25 22:37 ` [PATCH 4/5] gdb.base/: Introduce jit-protocol-util.h Ilya Leoshkevich
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2022-05-25 22:37 UTC (permalink / raw)
To: Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Andrew Burgess, Pedro Alves,
gdb-patches, Ilya Leoshkevich
Performance tests that dynamically load shared libraries may need to
know their number. Pass it via the preprocessor define.
---
gdb/testsuite/lib/gen-perf-test.exp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gdb/testsuite/lib/gen-perf-test.exp b/gdb/testsuite/lib/gen-perf-test.exp
index f0bfdc9317d..8c5413eaa19 100644
--- a/gdb/testsuite/lib/gen-perf-test.exp
+++ b/gdb/testsuite/lib/gen-perf-test.exp
@@ -1321,7 +1321,8 @@ namespace eval GenPerfTest {
set input_files [_make_binary_input_file_names self $static $run_nr]
set extra_headers [_get_param $self(binary_extra_headers) $run_nr]
set binary_file [_make_binary_name self $run_nr]
- set compile_options [_compile_options self]
+ set nr_gen_shlibs [_get_param $self(nr_gen_shlibs) $run_nr]
+ set compile_options "[_compile_options self] additional_flags=-DSHLIB=$nr_gen_shlibs"
set shlib_options [_make_shlib_options self $static $run_nr]
if { [llength $shlib_options] > 0 } {
append compile_options " " $shlib_options
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/5] gdb.perf/: Compile the binary with -DSHLIB
2022-05-25 22:37 ` [PATCH 3/5] gdb.perf/: Compile the binary with -DSHLIB Ilya Leoshkevich
@ 2022-05-26 15:43 ` Andrew Burgess
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-05-26 15:43 UTC (permalink / raw)
To: Ilya Leoshkevich via Gdb-patches, Tom Tromey
Cc: Ilya Leoshkevich, Pedro Alves, Ulrich Weigand, Andreas Arnez,
gdb-patches
Ilya Leoshkevich via Gdb-patches <gdb-patches@sourceware.org> writes:
> Performance tests that dynamically load shared libraries may need to
> know their number. Pass it via the preprocessor define.
> ---
> gdb/testsuite/lib/gen-perf-test.exp | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/lib/gen-perf-test.exp b/gdb/testsuite/lib/gen-perf-test.exp
> index f0bfdc9317d..8c5413eaa19 100644
> --- a/gdb/testsuite/lib/gen-perf-test.exp
> +++ b/gdb/testsuite/lib/gen-perf-test.exp
> @@ -1321,7 +1321,8 @@ namespace eval GenPerfTest {
> set input_files [_make_binary_input_file_names self $static $run_nr]
> set extra_headers [_get_param $self(binary_extra_headers) $run_nr]
> set binary_file [_make_binary_name self $run_nr]
> - set compile_options [_compile_options self]
> + set nr_gen_shlibs [_get_param $self(nr_gen_shlibs) $run_nr]
> + set compile_options "[_compile_options self] additional_flags=-DSHLIB=$nr_gen_shlibs"
I wonder if we should use a different name to avoid any chance of
confusion with the SHLIB define that is passed when compiling a shared
library? We use the same name, but they have subtly different
meanings...
I'd propose something like TOTAL_NR_SHLIBS or something similar.
Thanks,
Andrew
> set shlib_options [_make_shlib_options self $static $run_nr]
> if { [llength $shlib_options] > 0 } {
> append compile_options " " $shlib_options
> --
> 2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/5] gdb.base/: Introduce jit-protocol-util.h
2022-05-25 22:37 [PATCH 0/5] gdb.perf/: Add JIT performance test Ilya Leoshkevich
` (2 preceding siblings ...)
2022-05-25 22:37 ` [PATCH 3/5] gdb.perf/: Compile the binary with -DSHLIB Ilya Leoshkevich
@ 2022-05-25 22:37 ` Ilya Leoshkevich
2022-05-26 15:55 ` Andrew Burgess
2022-05-25 22:37 ` [PATCH 5/5] gdb.perf/: Add JIT performance test Ilya Leoshkevich
2022-05-27 9:43 ` [PATCH 0/5] " Andrew Burgess
5 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2022-05-25 22:37 UTC (permalink / raw)
To: Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Andrew Burgess, Pedro Alves,
gdb-patches, Ilya Leoshkevich
jit-elf-fork-main, jit-elf-main and jit-reader-host use the same
boilerplate to register and unregister JITed code. Move it to a new
header that can be shared with performance tests.
---
gdb/testsuite/gdb.base/jit-elf-fork-main.c | 37 ++---------
gdb/testsuite/gdb.base/jit-elf-main.c | 37 ++---------
gdb/testsuite/gdb.base/jit-protocol-util.h | 74 ++++++++++++++++++++++
gdb/testsuite/gdb.base/jit-reader-host.c | 8 +--
4 files changed, 84 insertions(+), 72 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/jit-protocol-util.h
diff --git a/gdb/testsuite/gdb.base/jit-elf-fork-main.c b/gdb/testsuite/gdb.base/jit-elf-fork-main.c
index 45792622548..96fadaa57b7 100644
--- a/gdb/testsuite/gdb.base/jit-elf-fork-main.c
+++ b/gdb/testsuite/gdb.base/jit-elf-fork-main.c
@@ -29,8 +29,8 @@
#include <sys/stat.h>
#include <unistd.h>
-#include "jit-protocol.h"
#include "jit-elf-util.h"
+#include "jit-protocol-util.h"
static void
usage (void)
@@ -77,17 +77,7 @@ main (int argc, char *argv[])
struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
entry->symfile_addr = (const char *)addr;
entry->symfile_size = obj_size;
- entry->prev_entry = __jit_debug_descriptor.relevant_entry;
- __jit_debug_descriptor.relevant_entry = entry;
-
- if (entry->prev_entry != NULL)
- entry->prev_entry->next_entry = entry;
- else
- __jit_debug_descriptor.first_entry = entry;
-
- /* Notify GDB. */
- __jit_debug_descriptor.action_flag = JIT_REGISTER;
- __jit_debug_register_code ();
+ jit_push_back (entry);
if (jit_function () != 42)
{
@@ -103,27 +93,8 @@ main (int argc, char *argv[])
i = 0; /* break after fork */
/* Now unregister them all in reverse order. */
- while (__jit_debug_descriptor.relevant_entry != NULL)
- {
- struct jit_code_entry *const entry =
- __jit_debug_descriptor.relevant_entry;
- struct jit_code_entry *const prev_entry = entry->prev_entry;
-
- if (prev_entry != NULL)
- {
- prev_entry->next_entry = NULL;
- entry->prev_entry = NULL;
- }
- else
- __jit_debug_descriptor.first_entry = NULL;
-
- /* Notify GDB. */
- __jit_debug_descriptor.action_flag = JIT_UNREGISTER;
- __jit_debug_register_code ();
-
- __jit_debug_descriptor.relevant_entry = prev_entry;
- free (entry);
- }
+ while (!jit_empty ())
+ free (jit_pop_back ());
return 0; /* break before return */
}
diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
index 948530f3197..8f482748ee5 100644
--- a/gdb/testsuite/gdb.base/jit-elf-main.c
+++ b/gdb/testsuite/gdb.base/jit-elf-main.c
@@ -29,8 +29,8 @@
#include <sys/stat.h>
#include <unistd.h>
-#include "jit-protocol.h"
#include "jit-elf-util.h"
+#include "jit-protocol-util.h"
static void
usage (void)
@@ -96,17 +96,7 @@ MAIN (int argc, char *argv[])
struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
entry->symfile_addr = (const char *)addr;
entry->symfile_size = obj_size;
- entry->prev_entry = __jit_debug_descriptor.relevant_entry;
- __jit_debug_descriptor.relevant_entry = entry;
-
- if (entry->prev_entry != NULL)
- entry->prev_entry->next_entry = entry;
- else
- __jit_debug_descriptor.first_entry = entry;
-
- /* Notify GDB. */
- __jit_debug_descriptor.action_flag = JIT_REGISTER;
- __jit_debug_register_code ();
+ jit_push_back (entry);
if (jit_function () != 42)
{
@@ -118,27 +108,8 @@ MAIN (int argc, char *argv[])
WAIT_FOR_GDB; i = 0; /* gdb break here 1 */
/* Now unregister them all in reverse order. */
- while (__jit_debug_descriptor.relevant_entry != NULL)
- {
- struct jit_code_entry *const entry =
- __jit_debug_descriptor.relevant_entry;
- struct jit_code_entry *const prev_entry = entry->prev_entry;
-
- if (prev_entry != NULL)
- {
- prev_entry->next_entry = NULL;
- entry->prev_entry = NULL;
- }
- else
- __jit_debug_descriptor.first_entry = NULL;
-
- /* Notify GDB. */
- __jit_debug_descriptor.action_flag = JIT_UNREGISTER;
- __jit_debug_register_code ();
-
- __jit_debug_descriptor.relevant_entry = prev_entry;
- free (entry);
- }
+ while (!jit_empty ())
+ free (jit_pop_back ());
WAIT_FOR_GDB; return 0; /* gdb break here 2 */
}
diff --git a/gdb/testsuite/gdb.base/jit-protocol-util.h b/gdb/testsuite/gdb.base/jit-protocol-util.h
new file mode 100644
index 00000000000..d0a59518429
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-protocol-util.h
@@ -0,0 +1,74 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright (C) 2022 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Frequently used functions for testing JITed objects. */
+
+#ifndef JIT_PROTOCOL_UTIL_H
+#define JIT_PROTOCOL_UTIL_H
+
+#include "jit-protocol.h"
+#include <stddef.h>
+
+/* Return whether there are no registered JITed objects. */
+static int
+jit_empty (void)
+{
+ return __jit_debug_descriptor.relevant_entry == NULL;
+}
+
+/* Register JITed object. symfile_addr and symfile_size must be set. */
+static void
+jit_push_back (struct jit_code_entry *entry)
+{
+ entry->prev_entry = __jit_debug_descriptor.relevant_entry;
+ __jit_debug_descriptor.relevant_entry = entry;
+
+ if (entry->prev_entry != NULL)
+ entry->prev_entry->next_entry = entry;
+ else
+ __jit_debug_descriptor.first_entry = entry;
+
+ /* Notify GDB. */
+ __jit_debug_descriptor.action_flag = JIT_REGISTER;
+ __jit_debug_register_code ();
+}
+
+/* Unregister the last registered JITed object. */
+static struct jit_code_entry *
+jit_pop_back (void)
+{
+ struct jit_code_entry *const entry = __jit_debug_descriptor.relevant_entry;
+ struct jit_code_entry *const prev_entry = entry->prev_entry;
+
+ if (prev_entry != NULL)
+ {
+ prev_entry->next_entry = NULL;
+ entry->prev_entry = NULL;
+ }
+ else
+ __jit_debug_descriptor.first_entry = NULL;
+
+ /* Notify GDB. */
+ __jit_debug_descriptor.action_flag = JIT_UNREGISTER;
+ __jit_debug_register_code ();
+
+ __jit_debug_descriptor.relevant_entry = prev_entry;
+
+ return entry;
+}
+
+#endif /* JIT_PROTOCOL_UTIL_H */
diff --git a/gdb/testsuite/gdb.base/jit-reader-host.c b/gdb/testsuite/gdb.base/jit-reader-host.c
index 0cca894a3a5..65f77b981d6 100644
--- a/gdb/testsuite/gdb.base/jit-reader-host.c
+++ b/gdb/testsuite/gdb.base/jit-reader-host.c
@@ -24,8 +24,8 @@
#include <sys/mman.h>
#include JIT_READER_H /* Please see jit-reader.exp for an explanation. */
+#include "jit-protocol-util.h"
#include "jit-reader-host.h"
-#include "jit-protocol.h"
struct jit_code_entry only_entry;
@@ -85,11 +85,7 @@ main (int argc, char **argv)
only_entry.symfile_addr = symfile;
only_entry.symfile_size = sizeof (struct jithost_abi);
- __jit_debug_descriptor.first_entry = &only_entry;
- __jit_debug_descriptor.relevant_entry = &only_entry;
- __jit_debug_descriptor.action_flag = JIT_REGISTER;
- __jit_debug_descriptor.version = 1;
- __jit_debug_register_code ();
+ jit_push_back (&only_entry);
function_stack_mangle ();
function_add (5, 6);
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/5] gdb.base/: Introduce jit-protocol-util.h
2022-05-25 22:37 ` [PATCH 4/5] gdb.base/: Introduce jit-protocol-util.h Ilya Leoshkevich
@ 2022-05-26 15:55 ` Andrew Burgess
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-05-26 15:55 UTC (permalink / raw)
To: Ilya Leoshkevich, Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Pedro Alves, gdb-patches,
Ilya Leoshkevich
Ilya Leoshkevich <iii@linux.ibm.com> writes:
> jit-elf-fork-main, jit-elf-main and jit-reader-host use the same
> boilerplate to register and unregister JITed code. Move it to a new
> header that can be shared with performance tests.
LGTM.
Thanks,
Andrew
> ---
> gdb/testsuite/gdb.base/jit-elf-fork-main.c | 37 ++---------
> gdb/testsuite/gdb.base/jit-elf-main.c | 37 ++---------
> gdb/testsuite/gdb.base/jit-protocol-util.h | 74 ++++++++++++++++++++++
> gdb/testsuite/gdb.base/jit-reader-host.c | 8 +--
> 4 files changed, 84 insertions(+), 72 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/jit-protocol-util.h
>
> diff --git a/gdb/testsuite/gdb.base/jit-elf-fork-main.c b/gdb/testsuite/gdb.base/jit-elf-fork-main.c
> index 45792622548..96fadaa57b7 100644
> --- a/gdb/testsuite/gdb.base/jit-elf-fork-main.c
> +++ b/gdb/testsuite/gdb.base/jit-elf-fork-main.c
> @@ -29,8 +29,8 @@
> #include <sys/stat.h>
> #include <unistd.h>
>
> -#include "jit-protocol.h"
> #include "jit-elf-util.h"
> +#include "jit-protocol-util.h"
>
> static void
> usage (void)
> @@ -77,17 +77,7 @@ main (int argc, char *argv[])
> struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
> entry->symfile_addr = (const char *)addr;
> entry->symfile_size = obj_size;
> - entry->prev_entry = __jit_debug_descriptor.relevant_entry;
> - __jit_debug_descriptor.relevant_entry = entry;
> -
> - if (entry->prev_entry != NULL)
> - entry->prev_entry->next_entry = entry;
> - else
> - __jit_debug_descriptor.first_entry = entry;
> -
> - /* Notify GDB. */
> - __jit_debug_descriptor.action_flag = JIT_REGISTER;
> - __jit_debug_register_code ();
> + jit_push_back (entry);
>
> if (jit_function () != 42)
> {
> @@ -103,27 +93,8 @@ main (int argc, char *argv[])
> i = 0; /* break after fork */
>
> /* Now unregister them all in reverse order. */
> - while (__jit_debug_descriptor.relevant_entry != NULL)
> - {
> - struct jit_code_entry *const entry =
> - __jit_debug_descriptor.relevant_entry;
> - struct jit_code_entry *const prev_entry = entry->prev_entry;
> -
> - if (prev_entry != NULL)
> - {
> - prev_entry->next_entry = NULL;
> - entry->prev_entry = NULL;
> - }
> - else
> - __jit_debug_descriptor.first_entry = NULL;
> -
> - /* Notify GDB. */
> - __jit_debug_descriptor.action_flag = JIT_UNREGISTER;
> - __jit_debug_register_code ();
> -
> - __jit_debug_descriptor.relevant_entry = prev_entry;
> - free (entry);
> - }
> + while (!jit_empty ())
> + free (jit_pop_back ());
>
> return 0; /* break before return */
> }
> diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
> index 948530f3197..8f482748ee5 100644
> --- a/gdb/testsuite/gdb.base/jit-elf-main.c
> +++ b/gdb/testsuite/gdb.base/jit-elf-main.c
> @@ -29,8 +29,8 @@
> #include <sys/stat.h>
> #include <unistd.h>
>
> -#include "jit-protocol.h"
> #include "jit-elf-util.h"
> +#include "jit-protocol-util.h"
>
> static void
> usage (void)
> @@ -96,17 +96,7 @@ MAIN (int argc, char *argv[])
> struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
> entry->symfile_addr = (const char *)addr;
> entry->symfile_size = obj_size;
> - entry->prev_entry = __jit_debug_descriptor.relevant_entry;
> - __jit_debug_descriptor.relevant_entry = entry;
> -
> - if (entry->prev_entry != NULL)
> - entry->prev_entry->next_entry = entry;
> - else
> - __jit_debug_descriptor.first_entry = entry;
> -
> - /* Notify GDB. */
> - __jit_debug_descriptor.action_flag = JIT_REGISTER;
> - __jit_debug_register_code ();
> + jit_push_back (entry);
>
> if (jit_function () != 42)
> {
> @@ -118,27 +108,8 @@ MAIN (int argc, char *argv[])
> WAIT_FOR_GDB; i = 0; /* gdb break here 1 */
>
> /* Now unregister them all in reverse order. */
> - while (__jit_debug_descriptor.relevant_entry != NULL)
> - {
> - struct jit_code_entry *const entry =
> - __jit_debug_descriptor.relevant_entry;
> - struct jit_code_entry *const prev_entry = entry->prev_entry;
> -
> - if (prev_entry != NULL)
> - {
> - prev_entry->next_entry = NULL;
> - entry->prev_entry = NULL;
> - }
> - else
> - __jit_debug_descriptor.first_entry = NULL;
> -
> - /* Notify GDB. */
> - __jit_debug_descriptor.action_flag = JIT_UNREGISTER;
> - __jit_debug_register_code ();
> -
> - __jit_debug_descriptor.relevant_entry = prev_entry;
> - free (entry);
> - }
> + while (!jit_empty ())
> + free (jit_pop_back ());
>
> WAIT_FOR_GDB; return 0; /* gdb break here 2 */
> }
> diff --git a/gdb/testsuite/gdb.base/jit-protocol-util.h b/gdb/testsuite/gdb.base/jit-protocol-util.h
> new file mode 100644
> index 00000000000..d0a59518429
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/jit-protocol-util.h
> @@ -0,0 +1,74 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright (C) 2022 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +/* Frequently used functions for testing JITed objects. */
> +
> +#ifndef JIT_PROTOCOL_UTIL_H
> +#define JIT_PROTOCOL_UTIL_H
> +
> +#include "jit-protocol.h"
> +#include <stddef.h>
> +
> +/* Return whether there are no registered JITed objects. */
> +static int
> +jit_empty (void)
> +{
> + return __jit_debug_descriptor.relevant_entry == NULL;
> +}
> +
> +/* Register JITed object. symfile_addr and symfile_size must be set. */
> +static void
> +jit_push_back (struct jit_code_entry *entry)
> +{
> + entry->prev_entry = __jit_debug_descriptor.relevant_entry;
> + __jit_debug_descriptor.relevant_entry = entry;
> +
> + if (entry->prev_entry != NULL)
> + entry->prev_entry->next_entry = entry;
> + else
> + __jit_debug_descriptor.first_entry = entry;
> +
> + /* Notify GDB. */
> + __jit_debug_descriptor.action_flag = JIT_REGISTER;
> + __jit_debug_register_code ();
> +}
> +
> +/* Unregister the last registered JITed object. */
> +static struct jit_code_entry *
> +jit_pop_back (void)
> +{
> + struct jit_code_entry *const entry = __jit_debug_descriptor.relevant_entry;
> + struct jit_code_entry *const prev_entry = entry->prev_entry;
> +
> + if (prev_entry != NULL)
> + {
> + prev_entry->next_entry = NULL;
> + entry->prev_entry = NULL;
> + }
> + else
> + __jit_debug_descriptor.first_entry = NULL;
> +
> + /* Notify GDB. */
> + __jit_debug_descriptor.action_flag = JIT_UNREGISTER;
> + __jit_debug_register_code ();
> +
> + __jit_debug_descriptor.relevant_entry = prev_entry;
> +
> + return entry;
> +}
> +
> +#endif /* JIT_PROTOCOL_UTIL_H */
> diff --git a/gdb/testsuite/gdb.base/jit-reader-host.c b/gdb/testsuite/gdb.base/jit-reader-host.c
> index 0cca894a3a5..65f77b981d6 100644
> --- a/gdb/testsuite/gdb.base/jit-reader-host.c
> +++ b/gdb/testsuite/gdb.base/jit-reader-host.c
> @@ -24,8 +24,8 @@
> #include <sys/mman.h>
>
> #include JIT_READER_H /* Please see jit-reader.exp for an explanation. */
> +#include "jit-protocol-util.h"
> #include "jit-reader-host.h"
> -#include "jit-protocol.h"
>
> struct jit_code_entry only_entry;
>
> @@ -85,11 +85,7 @@ main (int argc, char **argv)
> only_entry.symfile_addr = symfile;
> only_entry.symfile_size = sizeof (struct jithost_abi);
>
> - __jit_debug_descriptor.first_entry = &only_entry;
> - __jit_debug_descriptor.relevant_entry = &only_entry;
> - __jit_debug_descriptor.action_flag = JIT_REGISTER;
> - __jit_debug_descriptor.version = 1;
> - __jit_debug_register_code ();
> + jit_push_back (&only_entry);
>
> function_stack_mangle ();
> function_add (5, 6);
> --
> 2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/5] gdb.perf/: Add JIT performance test
2022-05-25 22:37 [PATCH 0/5] gdb.perf/: Add JIT performance test Ilya Leoshkevich
` (3 preceding siblings ...)
2022-05-25 22:37 ` [PATCH 4/5] gdb.base/: Introduce jit-protocol-util.h Ilya Leoshkevich
@ 2022-05-25 22:37 ` Ilya Leoshkevich
2022-05-27 13:32 ` Andrew Burgess
2022-05-27 9:43 ` [PATCH 0/5] " Andrew Burgess
5 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2022-05-25 22:37 UTC (permalink / raw)
To: Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Andrew Burgess, Pedro Alves,
gdb-patches, Ilya Leoshkevich
The test registers and unregisters 500 functions. The time it takes to
process each function (the more functions there are, the slower it is)
is written into a global array, which is then read and reported by the
GDB script.
---
gdb/testsuite/gdb.perf/jit-check.exp | 25 ++++++++
gdb/testsuite/gdb.perf/jit-check.py | 60 ++++++++++++++++++++
gdb/testsuite/gdb.perf/jit-solib.c | 28 +++++++++
gdb/testsuite/gdb.perf/jit.c | 85 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.perf/jit.exp | 37 ++++++++++++
5 files changed, 235 insertions(+)
create mode 100644 gdb/testsuite/gdb.perf/jit-check.exp
create mode 100644 gdb/testsuite/gdb.perf/jit-check.py
create mode 100644 gdb/testsuite/gdb.perf/jit-solib.c
create mode 100644 gdb/testsuite/gdb.perf/jit.c
create mode 100644 gdb/testsuite/gdb.perf/jit.exp
diff --git a/gdb/testsuite/gdb.perf/jit-check.exp b/gdb/testsuite/gdb.perf/jit-check.exp
new file mode 100644
index 00000000000..c124f294f7b
--- /dev/null
+++ b/gdb/testsuite/gdb.perf/jit-check.exp
@@ -0,0 +1,25 @@
+# Copyright (C) 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Benchmark registering and unregistering JITed code (run part).
+
+load_lib perftest.exp
+load_lib gen-perf-test.exp
+
+if [skip_perf_tests] {
+ return 0
+}
+
+GenPerfTest::standard_run_driver jit.exp make_testcase_config jit-check.py JitCheck
diff --git a/gdb/testsuite/gdb.perf/jit-check.py b/gdb/testsuite/gdb.perf/jit-check.py
new file mode 100644
index 00000000000..c5b46a23ffc
--- /dev/null
+++ b/gdb/testsuite/gdb.perf/jit-check.py
@@ -0,0 +1,60 @@
+# Copyright (C) 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Benchmark for registering and unregistering JITed code.
+
+import os
+import shlex
+
+from perftest import measure
+from perftest import perftest
+from perftest import testresult
+from perftest import utils
+
+
+class JitCheck(perftest.TestCaseWithBasicMeasurements):
+ def __init__(self, name, run_names, binfile):
+ super(JitCheck, self).__init__(name)
+ self.run_names = run_names
+ self.binfile = binfile
+ self.measurement = measure.MeasurementWallTime(
+ testresult.SingleStatisticTestResult()
+ )
+ self.measure.measurements.append(self.measurement)
+
+ def warm_up(self):
+ pass
+
+ def execute_test(self):
+ for run in self.run_names:
+ exe = "{}-{}".format(self.binfile, utils.convert_spaces(run))
+ utils.select_file(exe)
+
+ # Help the binary find the shared objects.
+ pieces = os.path.join(os.path.dirname(exe), "pieces")
+ gdb.execute("cd {}".format(shlex.quote(pieces)))
+
+ # Measure the total run time.
+ utils.runto_main()
+ gdb.execute("tbreak done_breakpoint")
+ self.measure.measure(lambda: gdb.execute("continue"), run)
+
+ # Extract and record processing times for individual functions.
+ for array_name in ("register_times", "unregister_times"):
+ array_value = gdb.parse_and_eval(array_name)
+ for i in range(*array_value.type.range()):
+ parameter = "{}:{}:{}".format(run, array_name, i + 1)
+ result = int(array_value[i]) / 1e6
+ self.measurement.result.record(parameter, result)
diff --git a/gdb/testsuite/gdb.perf/jit-solib.c b/gdb/testsuite/gdb.perf/jit-solib.c
new file mode 100644
index 00000000000..1171f1b86da
--- /dev/null
+++ b/gdb/testsuite/gdb.perf/jit-solib.c
@@ -0,0 +1,28 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright (C) 2022 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Shared object for benchmarking registering and unregistering JITed code.
+ It is simply copied into memory, so there should be no relocations.
+ Must be compiled with -DSHLIB=<shared object number>. */
+
+#define CAT_1(x, y) x##y
+#define CAT(x, y) CAT_1 (x, y)
+int
+CAT (jited_func_, SHLIB) (void)
+{
+ return SHLIB;
+}
diff --git a/gdb/testsuite/gdb.perf/jit.c b/gdb/testsuite/gdb.perf/jit.c
new file mode 100644
index 00000000000..7fc8b4a6339
--- /dev/null
+++ b/gdb/testsuite/gdb.perf/jit.c
@@ -0,0 +1,85 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright (C) 2022 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Benchmark for registering and unregistering JITed code.
+ Must be compiled with -DSHLIB=<total number of shared objects>. */
+
+#include "../gdb.base/jit-elf-util.h"
+#include "../gdb.base/jit-protocol-util.h"
+#include <stdint.h>
+#include <sys/time.h>
+
+static struct jit_code_entry entries[SHLIB];
+static uint64_t register_times[SHLIB];
+static uint64_t unregister_times[SHLIB];
+
+static uint64_t
+time_delta (struct timeval *start_time)
+{
+ struct timeval end_time;
+
+ gettimeofday (&end_time, NULL);
+ return (uint64_t)(end_time.tv_sec - start_time->tv_sec) * 1000000
+ + (end_time.tv_usec - start_time->tv_usec);
+}
+
+void __attribute__ ((noinline)) done_breakpoint (void) {}
+
+int
+main (void)
+{
+ struct timeval start_time;
+ int i;
+
+ /* Load and register shared libraries. */
+ for (i = 0; i < SHLIB; i++)
+ {
+ int (*jited_func) (void);
+ size_t obj_size;
+ char name[32];
+ void *addr;
+
+ sprintf (name, "jit-lib%d.so", i);
+ addr = load_elf (name, &obj_size, NULL);
+ sprintf (name, "jited_func_%d", i);
+ jited_func = addr + (uintptr_t)load_symbol (addr, name);
+
+ entries[i].symfile_addr = addr;
+ entries[i].symfile_size = obj_size;
+ gettimeofday (&start_time, NULL);
+ jit_push_back (&entries[i]);
+ register_times[i] = time_delta (&start_time);
+
+ if (jited_func () != i)
+ {
+ fprintf (stderr, "jited_func_%d () != %d\n", i, i);
+ return 1;
+ }
+ }
+
+ /* Now unregister them all in reverse order. */
+ for (i = SHLIB - 1; i >= 0; i--)
+ {
+ gettimeofday (&start_time, NULL);
+ jit_pop_back ();
+ unregister_times[i] = time_delta (&start_time);
+ }
+
+ done_breakpoint ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.perf/jit.exp b/gdb/testsuite/gdb.perf/jit.exp
new file mode 100644
index 00000000000..02923b290d9
--- /dev/null
+++ b/gdb/testsuite/gdb.perf/jit.exp
@@ -0,0 +1,37 @@
+# Copyright (C) 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Benchmark registering and unregistering JITed code (build part).
+
+load_lib perftest.exp
+load_lib gen-perf-test.exp
+
+if [skip_perf_tests] {
+ return 0
+}
+
+proc make_testcase_config { } {
+ set program_name "jit"
+ array set testcase [GenPerfTest::init_testcase $program_name]
+ set testcase(language) c
+ set testcase(binary_extra_sources) { { jit.c } }
+ set testcase(binary_link_with_shlibs) { 0 }
+ set testcase(gen_shlib_extra_sources) { { jit-solib.c } }
+ set testcase(run_names) { 500-sos }
+ set testcase(nr_gen_shlibs) { 500 }
+ return [array get testcase]
+}
+
+GenPerfTest::standard_compile_driver jit.exp make_testcase_config
--
2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 5/5] gdb.perf/: Add JIT performance test
2022-05-25 22:37 ` [PATCH 5/5] gdb.perf/: Add JIT performance test Ilya Leoshkevich
@ 2022-05-27 13:32 ` Andrew Burgess
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2022-05-27 13:32 UTC (permalink / raw)
To: Ilya Leoshkevich, Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Pedro Alves, gdb-patches,
Ilya Leoshkevich
Ilya Leoshkevich <iii@linux.ibm.com> writes:
> The test registers and unregisters 500 functions. The time it takes to
> process each function (the more functions there are, the slower it is)
> is written into a global array, which is then read and reported by the
> GDB script.
I'm basically happy with this, but I do have one suggestion...
> ---
> gdb/testsuite/gdb.perf/jit-check.exp | 25 ++++++++
> gdb/testsuite/gdb.perf/jit-check.py | 60 ++++++++++++++++++++
> gdb/testsuite/gdb.perf/jit-solib.c | 28 +++++++++
> gdb/testsuite/gdb.perf/jit.c | 85 ++++++++++++++++++++++++++++
> gdb/testsuite/gdb.perf/jit.exp | 37 ++++++++++++
> 5 files changed, 235 insertions(+)
> create mode 100644 gdb/testsuite/gdb.perf/jit-check.exp
> create mode 100644 gdb/testsuite/gdb.perf/jit-check.py
> create mode 100644 gdb/testsuite/gdb.perf/jit-solib.c
> create mode 100644 gdb/testsuite/gdb.perf/jit.c
> create mode 100644 gdb/testsuite/gdb.perf/jit.exp
>
> diff --git a/gdb/testsuite/gdb.perf/jit-check.exp b/gdb/testsuite/gdb.perf/jit-check.exp
> new file mode 100644
> index 00000000000..c124f294f7b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/jit-check.exp
> @@ -0,0 +1,25 @@
> +# Copyright (C) 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Benchmark registering and unregistering JITed code (run part).
> +
> +load_lib perftest.exp
> +load_lib gen-perf-test.exp
> +
> +if [skip_perf_tests] {
> + return 0
> +}
> +
> +GenPerfTest::standard_run_driver jit.exp make_testcase_config jit-check.py JitCheck
> diff --git a/gdb/testsuite/gdb.perf/jit-check.py b/gdb/testsuite/gdb.perf/jit-check.py
> new file mode 100644
> index 00000000000..c5b46a23ffc
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/jit-check.py
> @@ -0,0 +1,60 @@
> +# Copyright (C) 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Benchmark for registering and unregistering JITed code.
> +
> +import os
> +import shlex
> +
> +from perftest import measure
> +from perftest import perftest
> +from perftest import testresult
> +from perftest import utils
> +
> +
> +class JitCheck(perftest.TestCaseWithBasicMeasurements):
> + def __init__(self, name, run_names, binfile):
> + super(JitCheck, self).__init__(name)
> + self.run_names = run_names
> + self.binfile = binfile
> + self.measurement = measure.MeasurementWallTime(
> + testresult.SingleStatisticTestResult()
> + )
> + self.measure.measurements.append(self.measurement)
> +
> + def warm_up(self):
> + pass
> +
> + def execute_test(self):
> + for run in self.run_names:
> + exe = "{}-{}".format(self.binfile, utils.convert_spaces(run))
> + utils.select_file(exe)
> +
> + # Help the binary find the shared objects.
> + pieces = os.path.join(os.path.dirname(exe), "pieces")
> + gdb.execute("cd {}".format(shlex.quote(pieces)))
> +
> + # Measure the total run time.
> + utils.runto_main()
> + gdb.execute("tbreak done_breakpoint")
> + self.measure.measure(lambda: gdb.execute("continue"), run)
> +
> + # Extract and record processing times for individual functions.
> + for array_name in ("register_times", "unregister_times"):
> + array_value = gdb.parse_and_eval(array_name)
> + for i in range(*array_value.type.range()):
> + parameter = "{}:{}:{}".format(run, array_name, i + 1)
> + result = int(array_value[i]) / 1e6
> + self.measurement.result.record(parameter, result)
> diff --git a/gdb/testsuite/gdb.perf/jit-solib.c b/gdb/testsuite/gdb.perf/jit-solib.c
> new file mode 100644
> index 00000000000..1171f1b86da
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/jit-solib.c
> @@ -0,0 +1,28 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright (C) 2022 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +/* Shared object for benchmarking registering and unregistering JITed code.
> + It is simply copied into memory, so there should be no relocations.
> + Must be compiled with -DSHLIB=<shared object number>. */
> +
> +#define CAT_1(x, y) x##y
> +#define CAT(x, y) CAT_1 (x, y)
> +int
> +CAT (jited_func_, SHLIB) (void)
> +{
> + return SHLIB;
> +}
> diff --git a/gdb/testsuite/gdb.perf/jit.c b/gdb/testsuite/gdb.perf/jit.c
> new file mode 100644
> index 00000000000..7fc8b4a6339
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/jit.c
> @@ -0,0 +1,85 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright (C) 2022 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +/* Benchmark for registering and unregistering JITed code.
> + Must be compiled with -DSHLIB=<total number of shared objects>.
> */
Like I said on patch #3 I think we should change SHLIB to something
else, so this file would need updating.
> +
> +#include "../gdb.base/jit-elf-util.h"
> +#include "../gdb.base/jit-protocol-util.h"
> +#include <stdint.h>
> +#include <sys/time.h>
> +
> +static struct jit_code_entry entries[SHLIB];
> +static uint64_t register_times[SHLIB];
> +static uint64_t unregister_times[SHLIB];
> +
> +static uint64_t
> +time_delta (struct timeval *start_time)
> +{
> + struct timeval end_time;
> +
> + gettimeofday (&end_time, NULL);
> + return (uint64_t)(end_time.tv_sec - start_time->tv_sec) * 1000000
> + + (end_time.tv_usec - start_time->tv_usec);
> +}
> +
> +void __attribute__ ((noinline)) done_breakpoint (void) {}
> +
> +int
> +main (void)
> +{
> + struct timeval start_time;
> + int i;
> +
> + /* Load and register shared libraries. */
> + for (i = 0; i < SHLIB; i++)
> + {
> + int (*jited_func) (void);
> + size_t obj_size;
> + char name[32];
> + void *addr;
> +
> + sprintf (name, "jit-lib%d.so", i);
> + addr = load_elf (name, &obj_size, NULL);
> + sprintf (name, "jited_func_%d", i);
> + jited_func = addr + (uintptr_t)load_symbol (addr, name);
> +
> + entries[i].symfile_addr = addr;
> + entries[i].symfile_size = obj_size;
> + gettimeofday (&start_time, NULL);
> + jit_push_back (&entries[i]);
> + register_times[i] = time_delta (&start_time);
> +
> + if (jited_func () != i)
> + {
> + fprintf (stderr, "jited_func_%d () != %d\n", i, i);
> + return 1;
> + }
> + }
> +
> + /* Now unregister them all in reverse order. */
> + for (i = SHLIB - 1; i >= 0; i--)
> + {
> + gettimeofday (&start_time, NULL);
> + jit_pop_back ();
> + unregister_times[i] = time_delta (&start_time);
> + }
> +
> + done_breakpoint ();
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.perf/jit.exp b/gdb/testsuite/gdb.perf/jit.exp
> new file mode 100644
> index 00000000000..02923b290d9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/jit.exp
> @@ -0,0 +1,37 @@
> +# Copyright (C) 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Benchmark registering and unregistering JITed code (build part).
> +
> +load_lib perftest.exp
> +load_lib gen-perf-test.exp
> +
> +if [skip_perf_tests] {
> + return 0
> +}
> +
> +proc make_testcase_config { } {
> + set program_name "jit"
> + array set testcase [GenPerfTest::init_testcase $program_name]
> + set testcase(language) c
> + set testcase(binary_extra_sources) { { jit.c } }
> + set testcase(binary_link_with_shlibs) { 0 }
> + set testcase(gen_shlib_extra_sources) { { jit-solib.c } }
> + set testcase(run_names) { 500-sos }
> + set testcase(nr_gen_shlibs) { 500 }
I think it would be nice if the '500' could be user controllable. While
I was playing with this, I added this code before
'make_testcase_config':
# make build-perf RUNTESTFLAGS='jit.exp SOLIB_COUNT=100'
if ![info exists SOLIB_COUNT] {
set SOLIB_COUNT 500
}
And then made this change within make_testcase_config:
- set testcase(run_names) { 500-sos }
- set testcase(nr_gen_shlibs) { 500 }
+ set name "${::SOLIB_COUNT}-sos"
+ set testcase(run_names) [list $name]
+ set testcase(nr_gen_shlibs) ${::SOLIB_COUNT}
Then, like the comment says, I'd build with:
make build-perf RUNTESTFLAGS="jit.exp SOLIB_COUNT=100"
And test with:
make check-perf RUNTESTFLAGS="jit-check.exp SOLIB_COUNT=100" GDB_PERFTEST_MODE=run
I think we should include something like this, and make sure that the
new flag is documented, probably in the comment at the start of jit.exp.
However, before this is merged, I'd really like to understand why the
performance from gdb.base/jit-elf.exp is so different to the performance
we get from this test. I'm assuming that it's something to do with how
the test is compiled, but so far I've not been able to figure it out.
Thanks,
Andrew
> + return [array get testcase]
> +}
> +
> +GenPerfTest::standard_compile_driver jit.exp make_testcase_config
> --
> 2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/5] gdb.perf/: Add JIT performance test
2022-05-25 22:37 [PATCH 0/5] gdb.perf/: Add JIT performance test Ilya Leoshkevich
` (4 preceding siblings ...)
2022-05-25 22:37 ` [PATCH 5/5] gdb.perf/: Add JIT performance test Ilya Leoshkevich
@ 2022-05-27 9:43 ` Andrew Burgess
2022-05-30 12:32 ` Ilya Leoshkevich
5 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2022-05-27 9:43 UTC (permalink / raw)
To: Ilya Leoshkevich, Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Pedro Alves, gdb-patches,
Ilya Leoshkevich
Ilya Leoshkevich <iii@linux.ibm.com> writes:
> Hi,
>
> This series implements a JIT performance test as discussed in [1].
> Patch 1 is a small fix, patches 2-4 are preparations and patch 5 is
> the test itself.
>
> The test can be used as follows:
>
> $ make build-perf -j"$(nproc)" RUNTESTFLAGS=jit.exp
> $ make check-perf RUNTESTFLAGS=jit-check.exp GDB_PERFTEST_MODE=run
>
> The results can be converted to gnuplot-friendly format as follows:
>
> $ perl -ne 'if (/:register_times:(\d+) (.+)/) { print "$1 $2\n"; }' \
> <gdb.perf/outputs/jit/pieces/perftest.sum >jit.txt
>
> I've uploaded the results to [2]. They are somewhat different from
> those from Andrew, but they still show that [1] is an improvement.
I'm sure the reason for this difference is down to something I did
differently (wrong?) in my original benchmark, but I think it would be
really good to understand what that mistake was, as the performance
characteristics between my benchmark and yours are quite different.
I am taking a look at this myself, but it would be great if you could
run my original benchmark and let me know if you see result similar to
mine, or more similar to your benchmark.
If you can quickly spot why my benchmark does so much better (when
patched) then I'd love to hear why!
Thanks,
Andrew
> The third result shown on the chart is from [3] - with it I can
> pass the 10k JITed objects mark and finally debug CoreCLR JIT with
> GDB. It's still WIP though - the testsuite shows a couple of
> failures.
>
> Best regards,
> Ilya
>
> [1] https://sourceware.org/pipermail/gdb-patches/2022-May/189341.html
> [2] https://github.com/iii-i/binutils-gdb/blob/section-map-20220525-2/gdb/testsuite/jit.png
> [3] https://github.com/iii-i/binutils-gdb/commits/section-map-20220525-2
>
> Ilya Leoshkevich (5):
> gdb.perf/: Fix tcl_string_list_to_python_list {x}
> gdb.perf/: Add binary_link_with_shlibs setting to GenPerfTest
> gdb.perf/: Compile the binary with -DSHLIB
> gdb.base/: Introduce jit-protocol-util.h
> gdb.perf/: Add JIT performance test
>
> gdb/testsuite/gdb.base/jit-elf-fork-main.c | 37 +---------
> gdb/testsuite/gdb.base/jit-elf-main.c | 37 +---------
> gdb/testsuite/gdb.base/jit-protocol-util.h | 74 +++++++++++++++++++
> gdb/testsuite/gdb.base/jit-reader-host.c | 8 +-
> gdb/testsuite/gdb.perf/jit-check.exp | 25 +++++++
> gdb/testsuite/gdb.perf/jit-check.py | 60 +++++++++++++++
> gdb/testsuite/gdb.perf/jit-solib.c | 28 +++++++
> gdb/testsuite/gdb.perf/jit.c | 85 ++++++++++++++++++++++
> gdb/testsuite/gdb.perf/jit.exp | 37 ++++++++++
> gdb/testsuite/lib/gen-perf-test.exp | 25 ++++---
> gdb/testsuite/lib/perftest.exp | 2 +-
> 11 files changed, 336 insertions(+), 82 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/jit-protocol-util.h
> create mode 100644 gdb/testsuite/gdb.perf/jit-check.exp
> create mode 100644 gdb/testsuite/gdb.perf/jit-check.py
> create mode 100644 gdb/testsuite/gdb.perf/jit-solib.c
> create mode 100644 gdb/testsuite/gdb.perf/jit.c
> create mode 100644 gdb/testsuite/gdb.perf/jit.exp
>
> --
> 2.35.3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/5] gdb.perf/: Add JIT performance test
2022-05-27 9:43 ` [PATCH 0/5] " Andrew Burgess
@ 2022-05-30 12:32 ` Ilya Leoshkevich
0 siblings, 0 replies; 13+ messages in thread
From: Ilya Leoshkevich @ 2022-05-30 12:32 UTC (permalink / raw)
To: Andrew Burgess, Tom Tromey
Cc: Ulrich Weigand, Andreas Arnez, Pedro Alves, gdb-patches
On Fri, 2022-05-27 at 10:43 +0100, Andrew Burgess wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
>
> > Hi,
> >
> > This series implements a JIT performance test as discussed in [1].
> > Patch 1 is a small fix, patches 2-4 are preparations and patch 5 is
> > the test itself.
> >
> > The test can be used as follows:
> >
> > $ make build-perf -j"$(nproc)" RUNTESTFLAGS=jit.exp
> > $ make check-perf RUNTESTFLAGS=jit-check.exp GDB_PERFTEST_MODE=run
> >
> > The results can be converted to gnuplot-friendly format as follows:
> >
> > $ perl -ne 'if (/:register_times:(\d+) (.+)/) { print "$1 $2\n"; }'
> > \
> > <gdb.perf/outputs/jit/pieces/perftest.sum >jit.txt
> >
> > I've uploaded the results to [2]. They are somewhat different from
> > those from Andrew, but they still show that [1] is an improvement.
>
> I'm sure the reason for this difference is down to something I did
> differently (wrong?) in my original benchmark, but I think it would
> be
> really good to understand what that mistake was, as the performance
> characteristics between my benchmark and yours are quite different.
>
> I am taking a look at this myself, but it would be great if you could
> run my original benchmark and let me know if you see result similar
> to
> mine, or more similar to your benchmark.
>
> If you can quickly spot why my benchmark does so much better (when
> patched) then I'd love to hear why!
>
> Thanks,
> Andrew
Hi,
The problem is that I compile the shared objects with -fPIC, which
makes section addresses relative. However, the gdb JIT infrastructure
treats them as absolute, so a lot of overlaps arise. This makes the
unhappy
/* Sort on sequence number of the objfile in the chain. */
case in sort_cmp () occur extremely often, which then pollutes the
results. I'll fix this, measure again and send v2.
Best regards,
Ilya
^ permalink raw reply [flat|nested] 13+ messages in thread
