Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols
On 05/28/20 06:34 AM, Arnaldo Carvalho de Melo wrote: >> >> This is in my tmp.perf/core branch pending a round of testing, after >> that it'll move to perf/core on its way to 5.8, thanks. > > All tests passed, moved to perf/core. > Great, thank you! -- Nick
Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols
Em Wed, May 27, 2020 at 01:23:00PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Wed, May 27, 2020 at 11:20:57AM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Mon, Apr 27, 2020 at 02:15:16PM +0800, Nick Gasson escreveu: > > > For a Java method signature like: > > > > > > Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V > > > > > > The demangler produces: > > > > > > void class java.lang.AbstractStringBuilder.appendChars(class > > > java.lang., shorttring., int, int) > > > > > > The arguments should be (java.lang.String, int, int) but the demangler > > > interprets the "S" in String as the type code for "short". Correct this > > > and two other minor things: > > > > > > - There is no "bool" type in Java, should be "boolean". > > > > > > - The demangler prepends "class" to every Java class name. This is not > > > standard Java syntax and it wastes a lot of horizontal space if the > > > signature is long. Remove this as there isn't any ambiguity between > > > class names and primitives. > > > > > > Also added a test case. > > > > So, I took this and split into a patch for the new 'perf test java' and > > then the fix, so that we can see the problem being detected and then > > apply the fix and see it fixed, the last patch in this series thus > > became: > > This is in my tmp.perf/core branch pending a round of testing, after > that it'll move to perf/core on its way to 5.8, thanks. All tests passed, moved to perf/core. - Arnaldo
Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols
Em Wed, May 27, 2020 at 11:20:57AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Apr 27, 2020 at 02:15:16PM +0800, Nick Gasson escreveu:
> > For a Java method signature like:
> >
> > Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V
> >
> > The demangler produces:
> >
> > void class java.lang.AbstractStringBuilder.appendChars(class
> > java.lang., shorttring., int, int)
> >
> > The arguments should be (java.lang.String, int, int) but the demangler
> > interprets the "S" in String as the type code for "short". Correct this
> > and two other minor things:
> >
> > - There is no "bool" type in Java, should be "boolean".
> >
> > - The demangler prepends "class" to every Java class name. This is not
> > standard Java syntax and it wastes a lot of horizontal space if the
> > signature is long. Remove this as there isn't any ambiguity between
> > class names and primitives.
> >
> > Also added a test case.
>
> So, I took this and split into a patch for the new 'perf test java' and
> then the fix, so that we can see the problem being detected and then
> apply the fix and see it fixed, the last patch in this series thus
> became:
This is in my tmp.perf/core branch pending a round of testing, after
that it'll move to perf/core on its way to 5.8, thanks.
- Arnaldo
>
> commit 341e11c1d44532da3f5d626c9fe096949ae3
> Author: Nick Gasson
> Date: Mon Apr 27 14:15:16 2020 +0800
>
> perf jvmti: Fix demangling Java symbols
>
> For a Java method signature like:
>
> Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V
>
> The demangler produces:
>
> void class java.lang.AbstractStringBuilder.appendChars(class
> java.lang., shorttring., int, int)
>
> The arguments should be (java.lang.String, int, int) but the demangler
> interprets the "S" in String as the type code for "short". Correct this
> and two other minor things:
>
> - There is no "bool" type in Java, should be "boolean".
>
> - The demangler prepends "class" to every Java class name. This is not
> standard Java syntax and it wastes a lot of horizontal space if the
> signature is long. Remove this as there isn't any ambiguity between
> class names and primitives.
>
> Committer notes:
>
> This was split from a larger patch that also added a java demangler
> 'perf test' entry, that, before this patch shows the error being fixed
> by it:
>
> $ perf test java
> 65: Demangle Java : FAILED!
> $ perf test -v java
> Couldn't bump rlimit(MEMLOCK), failures may take place when creating
> BPF maps, etc
> 65: Demangle Java :
> --- start ---
> test child forked, pid 307264
> FAILED: Ljava/lang/StringLatin1;equals([B[B)Z: bool class
> java.lang.StringLatin1.equals(byte[], byte[]) != boolean
> java.lang.StringLatin1.equals(byte[], byte[])
> FAILED: Ljava/util/zip/ZipUtils;CENSIZ([BI)J: long class
> java.util.zip.ZipUtils.CENSIZ(byte[], int) != long
> java.util.zip.ZipUtils.CENSIZ(byte[], int)
> FAILED:
> Ljava/util/regex/Pattern$BmpCharProperty;match(Ljava/util/regex/Matcher;ILjava/lang/CharSequence;)Z:
> bool class java.util.regex.Pattern$BmpCharProperty.match(class
> java.util.regex.Matcher., int, class java.lang., charhar, shortequence) !=
> boolean
> java.util.regex.Pattern$BmpCharProperty.match(java.util.regex.Matcher, int,
> java.lang.CharSequence)
> FAILED:
> Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V: void
> class java.lang.AbstractStringBuilder.appendChars(class java.lang.,
> shorttring., int, int) != void
> java.lang.AbstractStringBuilder.appendChars(java.lang.String, int, int)
> FAILED: Ljava/lang/Object;()V: void class
> java.lang.Object() != void java.lang.Object()
> test child finished with -1
> end
> Demangle Java: FAILED!
> $
>
> After applying this patch:
>
> $ perf test java
> 65: Demangle Java : Ok
> $
>
> Signed-off-by: Nick Gasson
> Reviewed-by: Ian Rogers
> Tested-by: Arnaldo Carvalho de Melo
> Tested-by: Ian Rogers
> Cc: Alexander Shishkin
> Cc: Jiri Olsa
> Cc: Mark Rutland
> Cc: Namhyung Kim
> Cc: Peter Zijlstra
> Link:
> http://lore.kernel.org/lkml/20200427061520.24905-4-nick.gas...@arm.com
> Signed-off-by: Arnaldo Carvalho de Melo
>
> diff --git a/tools/perf/util/demangle-java.c b/tools/perf/util/demangle-java.c
> index 6fb7f34c0814..39c05200ed65 100644
> --- a/tools/perf/util/demangle-java.c
> +++ b/tools/perf/util/demangle-java.c
> @@ -15,7 +15,7 @@ enum {
> MODE_CLASS = 1,
> MODE_FUNC = 2,
> MODE_TYPE = 3,
> - MODE_CTYPE = 3, /* class arg */
> + MODE_CTYPE = 4, /*
Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols
Em Mon, Apr 27, 2020 at 02:15:16PM +0800, Nick Gasson escreveu:
> For a Java method signature like:
>
> Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V
>
> The demangler produces:
>
> void class java.lang.AbstractStringBuilder.appendChars(class java.lang.,
> shorttring., int, int)
>
> The arguments should be (java.lang.String, int, int) but the demangler
> interprets the "S" in String as the type code for "short". Correct this
> and two other minor things:
>
> - There is no "bool" type in Java, should be "boolean".
>
> - The demangler prepends "class" to every Java class name. This is not
> standard Java syntax and it wastes a lot of horizontal space if the
> signature is long. Remove this as there isn't any ambiguity between
> class names and primitives.
>
> Also added a test case.
So, I took this and split into a patch for the new 'perf test java' and
then the fix, so that we can see the problem being detected and then
apply the fix and see it fixed, the last patch in this series thus
became:
commit 341e11c1d44532da3f5d626c9fe096949ae3
Author: Nick Gasson
Date: Mon Apr 27 14:15:16 2020 +0800
perf jvmti: Fix demangling Java symbols
For a Java method signature like:
Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V
The demangler produces:
void class java.lang.AbstractStringBuilder.appendChars(class
java.lang., shorttring., int, int)
The arguments should be (java.lang.String, int, int) but the demangler
interprets the "S" in String as the type code for "short". Correct this
and two other minor things:
- There is no "bool" type in Java, should be "boolean".
- The demangler prepends "class" to every Java class name. This is not
standard Java syntax and it wastes a lot of horizontal space if the
signature is long. Remove this as there isn't any ambiguity between
class names and primitives.
Committer notes:
This was split from a larger patch that also added a java demangler
'perf test' entry, that, before this patch shows the error being fixed
by it:
$ perf test java
65: Demangle Java : FAILED!
$ perf test -v java
Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF
maps, etc
65: Demangle Java :
--- start ---
test child forked, pid 307264
FAILED: Ljava/lang/StringLatin1;equals([B[B)Z: bool class
java.lang.StringLatin1.equals(byte[], byte[]) != boolean
java.lang.StringLatin1.equals(byte[], byte[])
FAILED: Ljava/util/zip/ZipUtils;CENSIZ([BI)J: long class
java.util.zip.ZipUtils.CENSIZ(byte[], int) != long
java.util.zip.ZipUtils.CENSIZ(byte[], int)
FAILED:
Ljava/util/regex/Pattern$BmpCharProperty;match(Ljava/util/regex/Matcher;ILjava/lang/CharSequence;)Z:
bool class java.util.regex.Pattern$BmpCharProperty.match(class
java.util.regex.Matcher., int, class java.lang., charhar, shortequence) !=
boolean java.util.regex.Pattern$BmpCharProperty.match(java.util.regex.Matcher,
int, java.lang.CharSequence)
FAILED:
Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V: void class
java.lang.AbstractStringBuilder.appendChars(class java.lang., shorttring., int,
int) != void java.lang.AbstractStringBuilder.appendChars(java.lang.String, int,
int)
FAILED: Ljava/lang/Object;()V: void class java.lang.Object()
!= void java.lang.Object()
test child finished with -1
end
Demangle Java: FAILED!
$
After applying this patch:
$ perf test java
65: Demangle Java : Ok
$
Signed-off-by: Nick Gasson
Reviewed-by: Ian Rogers
Tested-by: Arnaldo Carvalho de Melo
Tested-by: Ian Rogers
Cc: Alexander Shishkin
Cc: Jiri Olsa
Cc: Mark Rutland
Cc: Namhyung Kim
Cc: Peter Zijlstra
Link: http://lore.kernel.org/lkml/20200427061520.24905-4-nick.gas...@arm.com
Signed-off-by: Arnaldo Carvalho de Melo
diff --git a/tools/perf/util/demangle-java.c b/tools/perf/util/demangle-java.c
index 6fb7f34c0814..39c05200ed65 100644
--- a/tools/perf/util/demangle-java.c
+++ b/tools/perf/util/demangle-java.c
@@ -15,7 +15,7 @@ enum {
MODE_CLASS = 1,
MODE_FUNC = 2,
MODE_TYPE = 3,
- MODE_CTYPE = 3, /* class arg */
+ MODE_CTYPE = 4, /* class arg */
};
#define BASE_ENT(c, n) [c - 'A']=n
@@ -27,7 +27,7 @@ static const char *base_types['Z' - 'A' + 1] = {
BASE_ENT('I', "int" ),
BASE_ENT('J', "long" ),
BASE_ENT('S', "short" ),
- BASE_ENT('Z', "bool" ),
+ BASE_ENT('Z', "boolean" ),
};
/*
@@ -59,15 +59,16 @@ __demangle_java_sym(const char *str, const char *end, char
*buf, int maxlen, int
switch (*q) {
case 'L':
Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols
Em Mon, Apr 27, 2020 at 02:15:16PM +0800, Nick Gasson escreveu:
> For a Java method signature like:
>
> Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V
>
> The demangler produces:
>
> void class java.lang.AbstractStringBuilder.appendChars(class java.lang.,
> shorttring., int, int)
Cool, one more 'perf test' entry, thanks a lot!
Applied,
- Arnaldo
> The arguments should be (java.lang.String, int, int) but the demangler
> interprets the "S" in String as the type code for "short". Correct this
> and two other minor things:
>
> - There is no "bool" type in Java, should be "boolean".
>
> - The demangler prepends "class" to every Java class name. This is not
> standard Java syntax and it wastes a lot of horizontal space if the
> signature is long. Remove this as there isn't any ambiguity between
> class names and primitives.
>
> Also added a test case.
>
> Signed-off-by: Nick Gasson
> ---
> tools/perf/tests/Build| 1 +
> tools/perf/tests/builtin-test.c | 4 +++
> tools/perf/tests/demangle-java-test.c | 42 +++
> tools/perf/tests/tests.h | 1 +
> tools/perf/util/demangle-java.c | 13 +
> 5 files changed, 55 insertions(+), 6 deletions(-)
> create mode 100644 tools/perf/tests/demangle-java-test.c
>
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 1692529639b0..2c45ac4a9581 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -55,6 +55,7 @@ perf-y += mem2node.o
> perf-y += maps.o
> perf-y += time-utils-test.o
> perf-y += genelf.o
> +perf-y += demangle-java-test.o
>
> $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
> $(call rule_mkdir)
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 54d9516c9839..03b362b37f97 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -309,6 +309,10 @@ static struct test generic_tests[] = {
> .desc = "maps__merge_in",
> .func = test__maps__merge_in,
> },
> + {
> + .desc = "Demangle Java",
> + .func = test__demangle_java,
> + },
> {
> .func = NULL,
> },
> diff --git a/tools/perf/tests/demangle-java-test.c
> b/tools/perf/tests/demangle-java-test.c
> new file mode 100644
> index ..8f3b90832fb0
> --- /dev/null
> +++ b/tools/perf/tests/demangle-java-test.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include
> +#include
> +#include
> +#include "tests.h"
> +#include "session.h"
> +#include "debug.h"
> +#include "demangle-java.h"
> +
> +int test__demangle_java(struct test *test __maybe_unused, int subtest
> __maybe_unused)
> +{
> + int ret = TEST_OK;
> + char *buf = NULL;
> + size_t i;
> +
> + struct {
> + const char *mangled, *demangled;
> + } test_cases[] = {
> + { "Ljava/lang/StringLatin1;equals([B[B)Z",
> + "boolean java.lang.StringLatin1.equals(byte[], byte[])" },
> + { "Ljava/util/zip/ZipUtils;CENSIZ([BI)J",
> + "long java.util.zip.ZipUtils.CENSIZ(byte[], int)" },
> + {
> "Ljava/util/regex/Pattern$BmpCharProperty;match(Ljava/util/regex/Matcher;ILjava/lang/CharSequence;)Z",
> + "boolean
> java.util.regex.Pattern$BmpCharProperty.match(java.util.regex.Matcher, int,
> java.lang.CharSequence)" },
> + {
> "Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V",
> + "void
> java.lang.AbstractStringBuilder.appendChars(java.lang.String, int, int)" },
> + { "Ljava/lang/Object;()V",
> + "void java.lang.Object()" },
> + };
> +
> + for (i = 0; i < sizeof(test_cases) / sizeof(test_cases[0]); i++) {
> + buf = java_demangle_sym(test_cases[i].mangled, 0);
> + if (strcmp(buf, test_cases[i].demangled)) {
> + pr_debug("FAILED: %s: %s != %s\n",
> test_cases[i].mangled,
> + buf, test_cases[i].demangled);
> + ret = TEST_FAIL;
> + }
> + free(buf);
> + }
> +
> + return ret;
> +}
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index 9a160fef47c9..49b791d978f6 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -111,6 +111,7 @@ int test__mem2node(struct test *t, int subtest);
> int test__maps__merge_in(struct test *t, int subtest);
> int test__time_utils(struct test *t, int subtest);
> int test__jit_write_elf(struct test *test, int subtest);
> +int test__demangle_java(struct test *test, int subtest);
>
> bool test__bp_signal_is_supported(void);
> bool test__bp_account_is_supported(void);
> diff --git a/tools/perf/util/demangle-java.c b/tools/perf/util/demangle-java.c
> index 6fb7f34c0814..39c05200ed65 100644
> --- a/tools/perf/util/demangle-java.c
>
Re: [PATCH 3/3] perf jvmti: Fix demangling Java symbols
On Sun, Apr 26, 2020 at 11:16 PM Nick Gasson wrote:
>
> For a Java method signature like:
>
> Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V
>
> The demangler produces:
>
> void class java.lang.AbstractStringBuilder.appendChars(class java.lang.,
> shorttring., int, int)
>
> The arguments should be (java.lang.String, int, int) but the demangler
> interprets the "S" in String as the type code for "short". Correct this
> and two other minor things:
>
> - There is no "bool" type in Java, should be "boolean".
>
> - The demangler prepends "class" to every Java class name. This is not
> standard Java syntax and it wastes a lot of horizontal space if the
> signature is long. Remove this as there isn't any ambiguity between
> class names and primitives.
>
> Also added a test case.
>
> Signed-off-by: Nick Gasson
Reviewed-by: Ian Rogers
Tested-by: Ian Rogers
Thanks!
Ian
> ---
> tools/perf/tests/Build| 1 +
> tools/perf/tests/builtin-test.c | 4 +++
> tools/perf/tests/demangle-java-test.c | 42 +++
> tools/perf/tests/tests.h | 1 +
> tools/perf/util/demangle-java.c | 13 +
> 5 files changed, 55 insertions(+), 6 deletions(-)
> create mode 100644 tools/perf/tests/demangle-java-test.c
>
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 1692529639b0..2c45ac4a9581 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -55,6 +55,7 @@ perf-y += mem2node.o
> perf-y += maps.o
> perf-y += time-utils-test.o
> perf-y += genelf.o
> +perf-y += demangle-java-test.o
>
> $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
> $(call rule_mkdir)
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 54d9516c9839..03b362b37f97 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -309,6 +309,10 @@ static struct test generic_tests[] = {
> .desc = "maps__merge_in",
> .func = test__maps__merge_in,
> },
> + {
> + .desc = "Demangle Java",
> + .func = test__demangle_java,
> + },
> {
> .func = NULL,
> },
> diff --git a/tools/perf/tests/demangle-java-test.c
> b/tools/perf/tests/demangle-java-test.c
> new file mode 100644
> index ..8f3b90832fb0
> --- /dev/null
> +++ b/tools/perf/tests/demangle-java-test.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include
> +#include
> +#include
> +#include "tests.h"
> +#include "session.h"
> +#include "debug.h"
> +#include "demangle-java.h"
> +
> +int test__demangle_java(struct test *test __maybe_unused, int subtest
> __maybe_unused)
> +{
> + int ret = TEST_OK;
> + char *buf = NULL;
> + size_t i;
> +
> + struct {
> + const char *mangled, *demangled;
> + } test_cases[] = {
> + { "Ljava/lang/StringLatin1;equals([B[B)Z",
> + "boolean java.lang.StringLatin1.equals(byte[], byte[])" },
> + { "Ljava/util/zip/ZipUtils;CENSIZ([BI)J",
> + "long java.util.zip.ZipUtils.CENSIZ(byte[], int)" },
> + {
> "Ljava/util/regex/Pattern$BmpCharProperty;match(Ljava/util/regex/Matcher;ILjava/lang/CharSequence;)Z",
> + "boolean
> java.util.regex.Pattern$BmpCharProperty.match(java.util.regex.Matcher, int,
> java.lang.CharSequence)" },
> + {
> "Ljava/lang/AbstractStringBuilder;appendChars(Ljava/lang/String;II)V",
> + "void
> java.lang.AbstractStringBuilder.appendChars(java.lang.String, int, int)" },
> + { "Ljava/lang/Object;()V",
> + "void java.lang.Object()" },
> + };
> +
> + for (i = 0; i < sizeof(test_cases) / sizeof(test_cases[0]); i++) {
> + buf = java_demangle_sym(test_cases[i].mangled, 0);
> + if (strcmp(buf, test_cases[i].demangled)) {
> + pr_debug("FAILED: %s: %s != %s\n",
> test_cases[i].mangled,
> +buf, test_cases[i].demangled);
> + ret = TEST_FAIL;
> + }
> + free(buf);
> + }
> +
> + return ret;
> +}
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index 9a160fef47c9..49b791d978f6 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -111,6 +111,7 @@ int test__mem2node(struct test *t, int subtest);
> int test__maps__merge_in(struct test *t, int subtest);
> int test__time_utils(struct test *t, int subtest);
> int test__jit_write_elf(struct test *test, int subtest);
> +int test__demangle_java(struct test *test, int subtest);
>
> bool test__bp_signal_is_supported(void);
> bool test__bp_account_is_supported(void);
> diff --git a/tools/perf/util/demangle-java.c b/tools/perf/util/demangle-java.c
> index 6fb7f34c0814..39c05200ed65
