Re: [EXTERNAL] Re: [PATCH v4] perf inject --jit: Remove //anon mmap events
On 06/01/20 16:53 PM, Ian Rogers wrote: > On Sun, May 31, 2020 at 11:17 PM Nick Gasson wrote: >> >> On 05/28/20 17:32 PM, Ian Rogers wrote: >> > >> > So on tip/perf/core with: >> > 1c0cd2dbb993 perf jvmti: Fix jitdump for methods without debug info >> > 3ce17c1e52f4 perf jvmti: remove redundant jitdump line table entries >> > >> > I've been trying variants of: >> > >> > Before: >> > /tmp/perf/perf record -k 1 -e cycles:u -o /tmp/perf.data java >> > -agentpath:/tmp/perf/libperf-jvmti.so -XX:+PreserveFramePointer >> > -XX:InitialCodeCacheSize=20M -XX:ReservedCodeCacheSize=1G -jar >> > dacapo-9.12-bach.jar jython >> > /tmp/perf/perf inject -i /tmp/perf.data -o /tmp/perf-jit.data -j >> > /tmp/perf/perf report -i /tmp/perf-jit.data |grep class\ |wc -l >> > 578 >> > /tmp/perf/perf report -i /tmp/perf-jit.data |grep unknown |wc -l >> > 6 >> > >> > After: >> > /tmp/perf/perf record -k 1 -e cycles:u -o /tmp/perf.data java >> > -agentpath:/tmp/perf/libperf-jvmti.so -XX:+PreserveFramePointer >> > -XX:InitialCodeCacheSize=20M -XX:ReservedCodeCacheSize=1G -jar >> > dacapo-9.12-bach.jar jython >> > /tmp/perf/perf inject -i /tmp/perf.data -o /tmp/perf-jit.data -j >> > /tmp/perf/perf report -i /tmp/perf-jit.data |grep class\ |wc -l >> > 589 >> > /tmp/perf/perf report -i /tmp/perf-jit.data |grep unknown |wc -l >> > 60 >> > >> > So maybe the jit cache isn't behaving the way that the patch cures, >> > the uptick in unknowns appears consistent in my testing though. I >> > expect user error, is there an obvious explanation I'm missing? >> > >> >> Hi Ian, >> >> I tried this as well with latest perf/core. The difference is that >> unresolved addresses currently look like: >> >> 0.00% java [JIT] tid 221782 [.] 0x451499a4 >> 0.00% java [JIT] tid 221782 [.] 0x4514f3e8 >> 0.00% java [JIT] tid 221782 [.] 0x45149394 >> >> But after Steve's patch this becomes: >> >> 0.00% java [unknown] [.] 0x58557d14 >> 0.00% java [unknown] [.] 0x785c03b4 >> 0.00% java [unknown] [.] 0x58386520 >> >> I couldn't see any events that were symbolised before but are no longer >> symbolised after this patch. > > I see this, thanks for digging into the explanation! Were you able to > get a test case where the unknowns went down? For example, by forcing > the code cache size to be small? This is the result I'd expect to see. I tried the same Dacapo benchmark as you with different values of InitialCodeCacheSize and grepped for -e '\[unknown\]' -e '\[JIT\]'. Base Patched 100M 338373 50M 333315 25M 323368 15M 1238 309 10M 2600 333 1M6035 337 This looks fairly convincing to me: the cliff at 15M is where the code cache starts needing to be enlarged. > >> I think most of these unknown events are caused by the asynchronous >> nature of the JVMTI event handling. After an nmethod is compiled the >> JVMTI event is posted to the Service Thread (*). So there can be a delay >> between the time the compiled code starts executing and the time the >> plugin receives the compiled code load event. >> >> Here's an edited down example: >> >> java 215881 750014.947873:1289634 cycles:u: >> 7856ad10 [unknown] ([unknown]) >> Service Thread 215895 750014.947971: PERF_RECORD_MMAP2 215879/215895: >> [0x785694c0(0x640) @ 0x40 fd:01 121010091 1]: >> java 215881 750014.948665:1295994 cycles:u: >> 7856ad10 org.python.core.__builtin__.range(org.python >> >> The plugin receives the event ~100us after the first sample inside that >> method. Ideally we would use the timestamp when the method was actually >> compiled, but I can't see any way to extract this information. > > Hmm.. this is a bit weird as the compile_info at one point was a > resource allocation and so would be cleared out when the resource mark > was passed on the compiler's return. Not something you'd want to do > asynchronously. Presumably this has changed to improve performance, > but doing the jvmti on a separate thread defeats jitdump - if we see > samples in code ahead of the code being loaded. Perhaps a profiler > like async-profiler > (https://github.com/jvm-profiling-tools/
Re: [EXTERNAL] Re: [PATCH v4] perf inject --jit: Remove //anon mmap events
On 05/28/20 17:32 PM, Ian Rogers wrote: > > So on tip/perf/core with: > 1c0cd2dbb993 perf jvmti: Fix jitdump for methods without debug info > 3ce17c1e52f4 perf jvmti: remove redundant jitdump line table entries > > I've been trying variants of: > > Before: > /tmp/perf/perf record -k 1 -e cycles:u -o /tmp/perf.data java > -agentpath:/tmp/perf/libperf-jvmti.so -XX:+PreserveFramePointer > -XX:InitialCodeCacheSize=20M -XX:ReservedCodeCacheSize=1G -jar > dacapo-9.12-bach.jar jython > /tmp/perf/perf inject -i /tmp/perf.data -o /tmp/perf-jit.data -j > /tmp/perf/perf report -i /tmp/perf-jit.data |grep class\ |wc -l > 578 > /tmp/perf/perf report -i /tmp/perf-jit.data |grep unknown |wc -l > 6 > > After: > /tmp/perf/perf record -k 1 -e cycles:u -o /tmp/perf.data java > -agentpath:/tmp/perf/libperf-jvmti.so -XX:+PreserveFramePointer > -XX:InitialCodeCacheSize=20M -XX:ReservedCodeCacheSize=1G -jar > dacapo-9.12-bach.jar jython > /tmp/perf/perf inject -i /tmp/perf.data -o /tmp/perf-jit.data -j > /tmp/perf/perf report -i /tmp/perf-jit.data |grep class\ |wc -l > 589 > /tmp/perf/perf report -i /tmp/perf-jit.data |grep unknown |wc -l > 60 > > So maybe the jit cache isn't behaving the way that the patch cures, > the uptick in unknowns appears consistent in my testing though. I > expect user error, is there an obvious explanation I'm missing? > Hi Ian, I tried this as well with latest perf/core. The difference is that unresolved addresses currently look like: 0.00% java [JIT] tid 221782 [.] 0x451499a4 0.00% java [JIT] tid 221782 [.] 0x4514f3e8 0.00% java [JIT] tid 221782 [.] 0x45149394 But after Steve's patch this becomes: 0.00% java [unknown] [.] 0x58557d14 0.00% java [unknown] [.] 0x785c03b4 0.00% java [unknown] [.] 0x58386520 I couldn't see any events that were symbolised before but are no longer symbolised after this patch. I think most of these unknown events are caused by the asynchronous nature of the JVMTI event handling. After an nmethod is compiled the JVMTI event is posted to the Service Thread (*). So there can be a delay between the time the compiled code starts executing and the time the plugin receives the compiled code load event. Here's an edited down example: java 215881 750014.947873:1289634 cycles:u: 7856ad10 [unknown] ([unknown]) Service Thread 215895 750014.947971: PERF_RECORD_MMAP2 215879/215895: [0x785694c0(0x640) @ 0x40 fd:01 121010091 1]: java 215881 750014.948665:1295994 cycles:u: 7856ad10 org.python.core.__builtin__.range(org.python The plugin receives the event ~100us after the first sample inside that method. Ideally we would use the timestamp when the method was actually compiled, but I can't see any way to extract this information. However I also saw a few recurring [unknown] addresses that never have a corresponding code load event. I'm not sure where these come from. (*) http://hg.openjdk.java.net/jdk/jdk/file/50fe8727ed79/src/hotspot/share/code/nmethod.cpp#l1591 -- Nick
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
[PATCH v2] perf jvmti: Remove redundant jitdump line table entries
For each PC/BCI pair in the JVMTI compiler inlining record table, the
jitdump plugin emits debug line table entries for every source line in
the method preceding that BCI. Instead only emit one source line per
PC/BCI pair. Reported by Ian Rogers. This reduces the .dump size for
SPECjbb from ~230MB to ~40MB.
Signed-off-by: Nick Gasson
---
Changes in v2:
- Split the unrelated DWARF debug fix into a separate patch
- Added a comment about the use of c->methods
tools/perf/jvmti/libjvmti.c | 78 -
1 file changed, 33 insertions(+), 45 deletions(-)
diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c
index c5d30834a64c..fcca275e5bf9 100644
--- a/tools/perf/jvmti/libjvmti.c
+++ b/tools/perf/jvmti/libjvmti.c
@@ -32,38 +32,41 @@ static void print_error(jvmtiEnv *jvmti, const char *msg,
jvmtiError ret)
#ifdef HAVE_JVMTI_CMLR
static jvmtiError
-do_get_line_numbers(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci,
- jvmti_line_info_t *tab, jint *nr)
+do_get_line_number(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci,
+ jvmti_line_info_t *tab)
{
- jint i, lines = 0;
- jint nr_lines = 0;
+ jint i, nr_lines = 0;
jvmtiLineNumberEntry *loc_tab = NULL;
jvmtiError ret;
+ jint src_line = -1;
ret = (*jvmti)->GetLineNumberTable(jvmti, m, &nr_lines, &loc_tab);
if (ret == JVMTI_ERROR_ABSENT_INFORMATION || ret ==
JVMTI_ERROR_NATIVE_METHOD) {
/* No debug information for this method */
- *nr = 0;
- return JVMTI_ERROR_NONE;
+ return ret;
} else if (ret != JVMTI_ERROR_NONE) {
print_error(jvmti, "GetLineNumberTable", ret);
return ret;
}
- for (i = 0; i < nr_lines; i++) {
- if (loc_tab[i].start_location < bci) {
- tab[lines].pc = (unsigned long)pc;
- tab[lines].line_number = loc_tab[i].line_number;
- tab[lines].discrim = 0; /* not yet used */
- tab[lines].methodID = m;
- lines++;
- } else {
- break;
- }
+ for (i = 0; i < nr_lines && loc_tab[i].start_location <= bci; i++) {
+ src_line = i;
+ }
+
+ if (src_line != -1) {
+ tab->pc = (unsigned long)pc;
+ tab->line_number = loc_tab[src_line].line_number;
+ tab->discrim = 0; /* not yet used */
+ tab->methodID = m;
+
+ ret = JVMTI_ERROR_NONE;
+ } else {
+ ret = JVMTI_ERROR_ABSENT_INFORMATION;
}
+
(*jvmti)->Deallocate(jvmti, (unsigned char *)loc_tab);
- *nr = lines;
- return JVMTI_ERROR_NONE;
+
+ return ret;
}
static jvmtiError
@@ -71,9 +74,8 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info,
jvmti_line_info_t **
{
const jvmtiCompiledMethodLoadRecordHeader *hdr;
jvmtiCompiledMethodLoadInlineRecord *rec;
- jvmtiLineNumberEntry *lne = NULL;
PCStackInfo *c;
- jint nr, ret;
+ jint ret;
int nr_total = 0;
int i, lines_total = 0;
@@ -86,24 +88,7 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info,
jvmti_line_info_t **
for (hdr = compile_info; hdr != NULL; hdr = hdr->next) {
if (hdr->kind == JVMTI_CMLR_INLINE_INFO) {
rec = (jvmtiCompiledMethodLoadInlineRecord *)hdr;
- for (i = 0; i < rec->numpcs; i++) {
- c = rec->pcinfo + i;
- nr = 0;
- /*
-* unfortunately, need a tab to get the number
of lines!
-*/
- ret = (*jvmti)->GetLineNumberTable(jvmti,
c->methods[0], &nr, &lne);
- if (ret == JVMTI_ERROR_NONE) {
- /* free what was allocated for nothing
*/
- (*jvmti)->Deallocate(jvmti, (unsigned
char *)lne);
- nr_total += (int)nr;
- } else if (ret ==
JVMTI_ERROR_ABSENT_INFORMATION ||
- ret == JVMTI_ERROR_NATIVE_METHOD) {
- /* No debug information for this method
*/
- } else {
- print_error(jvmti,
"GetLineNumberTable", ret);
- }
- }
+ nr_total += rec->numpcs;
}
}
@@ -122,14 +107,17 @@ get_line_numbers(jvmtiEnv *jvmti, const void
*compile_info, jvmti_line_info_t
[PATCH] perf jit: Fix inaccurate DWARF line table
Fix an issue where addresses in the DWARF line table are offset by -0x40 (GEN_ELF_TEXT_OFFSET). This can be seen with `objdump -S` on the ELF files after perf inject. Signed-off-by: Nick Gasson --- tools/perf/util/genelf_debug.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/genelf_debug.c b/tools/perf/util/genelf_debug.c index 30e9f618f6cd..dd40683bd4c0 100644 --- a/tools/perf/util/genelf_debug.c +++ b/tools/perf/util/genelf_debug.c @@ -342,7 +342,7 @@ static void emit_lineno_info(struct buffer_ext *be, */ /* start state of the state machine we take care of */ - unsigned long last_vma = code_addr; + unsigned long last_vma = 0; char const *cur_filename = NULL; unsigned long cur_file_idx = 0; int last_line = 1; @@ -473,7 +473,7 @@ jit_process_debug_info(uint64_t code_addr, ent = debug_entry_next(ent); } add_compilation_unit(di, buffer_ext_size(dl)); - add_debug_line(dl, debug, nr_debug_entries, 0); + add_debug_line(dl, debug, nr_debug_entries, GEN_ELF_TEXT_OFFSET); add_debug_abbrev(da); if (0) buffer_ext_dump(da, "abbrev"); -- 2.26.2
Re: [PATCH] perf jvmti: remove redundant jitdump line table entries
On 05/28/20 02:08 AM, Ian Rogers wrote:
>>
>> I noticed it loses information when the Hotspot code cache is
>> resized. I've been working around that by setting
>> -XX:InitialCodeCacheSize and -XX:ReservedCodeCacheSize to large
>> values. Does this help in your case?
>
> Thanks, I tried and also with Steve's patch:
> https://lore.kernel.org/lkml/1590544271-125795-1-git-send-email-steve.macl...@linux.microsoft.com/
Thanks for the reference! That patch fixes the problem I had with code
cache resizing so the workaround above is no longer necessary.
>
> Trying something very basic like just the -version command with compile only:
> /tmp/perf/perf record -k 1 -e cycles:u -F 6500 -o /tmp/perf.data java
> -agentpath:/tmp/perf/libperf-jvmti.so -XX:+PreserveFramePointer
> -XX:InitialCodeCacheSize=2G -XX:ReservedCodeCacheSize=2G
> -XX:CompileOnly=1 -version
> /tmp/perf/perf inject -i /tmp/perf.data -o /tmp/perf-jit.data -j
> /tmp/perf/perf report -i /tmp/perf-jit.data
>
> I don't see any of the JDK classes but 35 unknown symbols out of 272.
> The JDK classes are stripped to some degree iirc, but we should be
> able to give a symbol name as we don't care about local variables and
> like.
>
I tried this with latest perf/core and JDK 11 but I don't see any
[unknown] from jitted-*.so. All the events are in "Interpreter": I think
the options you want are -Xcomp -Xbatch rather than -XX:CompileOnly=1?
The latter restricts compilation to the named method/package.
There was a bug where no jitdump debug info was written for classes
compiled without line tables. That was fixed by d3ea46da3 ("perf jvmti:
Fix jitdump for methods without debug info").
--
Nick
Re: [PATCH] perf jvmti: remove redundant jitdump line table entries
On 05/27/20 13:03 PM, Ian Rogers wrote: > > Great result, thanks! I note there is a lack of symbolization when > benchmarking a few Java applications. I'll try to see if there's a > sensible resolution for those. > I noticed it loses information when the Hotspot code cache is resized. I've been working around that by setting -XX:InitialCodeCacheSize and -XX:ReservedCodeCacheSize to large values. Does this help in your case? > > It'd be better to make this into two patches. Also on acme's perf/core > branch if possible: > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=perf/core OK sure, I'll do that. -- Nick
Re: [PATCH] perf jvmti: remove redundant jitdump line table entries
On 05/26/20 19:55 PM, Jiri Olsa wrote:
> On Fri, May 22, 2020 at 02:53:30PM +0800, Nick Gasson wrote:
>> For each PC/BCI pair in the JVMTI compiler inlining record table, the
>> jitdump plugin emits debug line table entries for every source line in
>> the method preceding that BCI. Instead only emit one source line per
>> PC/BCI pair. Reported by Ian Rogers. This reduces the .dump size for
>> SPECjbb from ~230MB to ~40MB.
>>
>> Also fix an error in the DWARF line table state machine where addresses
>> are incorrectly offset by -0x40 (GEN_ELF_TEXT_OFFSET). This can be seen
>> with `objdump -S` on the ELF files after perf inject.
>
> hi,
> I can't apply this on latest Arnaldo's perf/core:
>
> patching file jvmti/libjvmti.c
> Hunk #1 FAILED at 32.
> Hunk #2 succeeded at 67 (offset -4 lines).
> Hunk #3 FAILED at 85.
> Hunk #4 succeeded at 114 (offset -7 lines).
>
Sorry I based this on my earlier patch series below but I didn't realise
that wasn't merged to perf/core yet. Could those patches be applied
first? I believe Ian added a Reviewed-by for all three.
https://lore.kernel.org/lkml/20200427061520.24905-3-nick.gas...@arm.com/T/
>
>>
>> Signed-off-by: Nick Gasson
>> ---
>> tools/perf/jvmti/libjvmti.c| 73 +-
>> tools/perf/util/genelf_debug.c | 4 +-
>> 2 files changed, 30 insertions(+), 47 deletions(-)
>>
>> diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c
>> index a9a056d68416..398e4ba6498d 100644
>> --- a/tools/perf/jvmti/libjvmti.c
>> +++ b/tools/perf/jvmti/libjvmti.c
>> @@ -32,38 +32,41 @@ static void print_error(jvmtiEnv *jvmti, const char
>> *msg, jvmtiError ret)
>>
>> #ifdef HAVE_JVMTI_CMLR
>> static jvmtiError
>> -do_get_line_numbers(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci,
>> -jvmti_line_info_t *tab, jint *nr)
>> +do_get_line_number(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci,
>> + jvmti_line_info_t *tab)
>> {
>> -jint i, lines = 0;
>> -jint nr_lines = 0;
>> +jint i, nr_lines = 0;
>> jvmtiLineNumberEntry *loc_tab = NULL;
>> jvmtiError ret;
>> +jint src_line = -1;
>>
>> ret = (*jvmti)->GetLineNumberTable(jvmti, m, &nr_lines, &loc_tab);
>> if (ret == JVMTI_ERROR_ABSENT_INFORMATION || ret ==
>> JVMTI_ERROR_NATIVE_METHOD) {
>> /* No debug information for this method */
>> -*nr = 0;
>> -return JVMTI_ERROR_NONE;
>> +return ret;
>> } else if (ret != JVMTI_ERROR_NONE) {
>> print_error(jvmti, "GetLineNumberTable", ret);
>> return ret;
>> }
>>
>> -for (i = 0; i < nr_lines; i++) {
>> -if (loc_tab[i].start_location < bci) {
>> -tab[lines].pc = (unsigned long)pc;
>> -tab[lines].line_number = loc_tab[i].line_number;
>> -tab[lines].discrim = 0; /* not yet used */
>> -tab[lines].methodID = m;
>> -lines++;
>> -} else {
>> -break;
>> -}
>> +for (i = 0; i < nr_lines && loc_tab[i].start_location <= bci; i++) {
>> +src_line = i;
>> +}
>> +
>> +if (src_line != -1) {
>> +tab->pc = (unsigned long)pc;
>> +tab->line_number = loc_tab[src_line].line_number;
>> +tab->discrim = 0; /* not yet used */
>> +tab->methodID = m;
>> +
>> +ret = JVMTI_ERROR_NONE;
>> +} else {
>> +ret = JVMTI_ERROR_ABSENT_INFORMATION;
>> }
>> +
>> (*jvmti)->Deallocate(jvmti, (unsigned char *)loc_tab);
>> -*nr = lines;
>> -return JVMTI_ERROR_NONE;
>> +
>> +return ret;
>> }
>>
>> static jvmtiError
>> @@ -71,9 +74,8 @@ get_line_numbers(jvmtiEnv *jvmti, const void
>> *compile_info, jvmti_line_info_t **
>> {
>> const jvmtiCompiledMethodLoadRecordHeader *hdr;
>> jvmtiCompiledMethodLoadInlineRecord *rec;
>> -jvmtiLineNumberEntry *lne = NULL;
>> PCStackInfo *c;
>> -jint nr, ret;
>> +jint ret;
>> int nr_total = 0;
>> int i, lines_total = 0;
>>
>> @@ -86,24 +88,7 @@ get_line_numbers(jvmtiEnv *jvmti, const void
>> *compile_info, jvmti_line_info_t **
>> for (hdr = compile_info; hdr != NULL; hdr = hdr->next) {
[PATCH] perf jvmti: remove redundant jitdump line table entries
For each PC/BCI pair in the JVMTI compiler inlining record table, the
jitdump plugin emits debug line table entries for every source line in
the method preceding that BCI. Instead only emit one source line per
PC/BCI pair. Reported by Ian Rogers. This reduces the .dump size for
SPECjbb from ~230MB to ~40MB.
Also fix an error in the DWARF line table state machine where addresses
are incorrectly offset by -0x40 (GEN_ELF_TEXT_OFFSET). This can be seen
with `objdump -S` on the ELF files after perf inject.
Signed-off-by: Nick Gasson
---
tools/perf/jvmti/libjvmti.c| 73 +-
tools/perf/util/genelf_debug.c | 4 +-
2 files changed, 30 insertions(+), 47 deletions(-)
diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c
index a9a056d68416..398e4ba6498d 100644
--- a/tools/perf/jvmti/libjvmti.c
+++ b/tools/perf/jvmti/libjvmti.c
@@ -32,38 +32,41 @@ static void print_error(jvmtiEnv *jvmti, const char *msg,
jvmtiError ret)
#ifdef HAVE_JVMTI_CMLR
static jvmtiError
-do_get_line_numbers(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci,
- jvmti_line_info_t *tab, jint *nr)
+do_get_line_number(jvmtiEnv *jvmti, void *pc, jmethodID m, jint bci,
+ jvmti_line_info_t *tab)
{
- jint i, lines = 0;
- jint nr_lines = 0;
+ jint i, nr_lines = 0;
jvmtiLineNumberEntry *loc_tab = NULL;
jvmtiError ret;
+ jint src_line = -1;
ret = (*jvmti)->GetLineNumberTable(jvmti, m, &nr_lines, &loc_tab);
if (ret == JVMTI_ERROR_ABSENT_INFORMATION || ret ==
JVMTI_ERROR_NATIVE_METHOD) {
/* No debug information for this method */
- *nr = 0;
- return JVMTI_ERROR_NONE;
+ return ret;
} else if (ret != JVMTI_ERROR_NONE) {
print_error(jvmti, "GetLineNumberTable", ret);
return ret;
}
- for (i = 0; i < nr_lines; i++) {
- if (loc_tab[i].start_location < bci) {
- tab[lines].pc = (unsigned long)pc;
- tab[lines].line_number = loc_tab[i].line_number;
- tab[lines].discrim = 0; /* not yet used */
- tab[lines].methodID = m;
- lines++;
- } else {
- break;
- }
+ for (i = 0; i < nr_lines && loc_tab[i].start_location <= bci; i++) {
+ src_line = i;
+ }
+
+ if (src_line != -1) {
+ tab->pc = (unsigned long)pc;
+ tab->line_number = loc_tab[src_line].line_number;
+ tab->discrim = 0; /* not yet used */
+ tab->methodID = m;
+
+ ret = JVMTI_ERROR_NONE;
+ } else {
+ ret = JVMTI_ERROR_ABSENT_INFORMATION;
}
+
(*jvmti)->Deallocate(jvmti, (unsigned char *)loc_tab);
- *nr = lines;
- return JVMTI_ERROR_NONE;
+
+ return ret;
}
static jvmtiError
@@ -71,9 +74,8 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info,
jvmti_line_info_t **
{
const jvmtiCompiledMethodLoadRecordHeader *hdr;
jvmtiCompiledMethodLoadInlineRecord *rec;
- jvmtiLineNumberEntry *lne = NULL;
PCStackInfo *c;
- jint nr, ret;
+ jint ret;
int nr_total = 0;
int i, lines_total = 0;
@@ -86,24 +88,7 @@ get_line_numbers(jvmtiEnv *jvmti, const void *compile_info,
jvmti_line_info_t **
for (hdr = compile_info; hdr != NULL; hdr = hdr->next) {
if (hdr->kind == JVMTI_CMLR_INLINE_INFO) {
rec = (jvmtiCompiledMethodLoadInlineRecord *)hdr;
- for (i = 0; i < rec->numpcs; i++) {
- c = rec->pcinfo + i;
- nr = 0;
- /*
-* unfortunately, need a tab to get the number
of lines!
-*/
- ret = (*jvmti)->GetLineNumberTable(jvmti,
c->methods[0], &nr, &lne);
- if (ret == JVMTI_ERROR_NONE) {
- /* free what was allocated for nothing
*/
- (*jvmti)->Deallocate(jvmti, (unsigned
char *)lne);
- nr_total += (int)nr;
- } else if (ret == JVMTI_ERROR_ABSENT_INFORMATION
- || ret == JVMTI_ERROR_NATIVE_METHOD)
{
- /* No debug information for this method
*/
- } else {
- print_error(jvmti,
"GetLineNumberTable", ret);
- }
- }
+ nr_total += rec->numpcs;
Re: [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent
On 05/15/20 06:41 am, Ian Rogers wrote:
>
> If you are looking at this code I believe there is a bug in that the
> loop handling jvmtiCompiledMethodLoadInlineRecord is writing out the
> entire line number table before a pc and not just the line number
> table at the pc. This loop in do_get_line_numbers:
>
> if (loc_tab[i].start_location < bci) {
> tab[lines].pc = (unsigned long)pc;
> tab[lines].line_number = loc_tab[i].line_number;
> tab[lines].discrim = 0; /* not yet used */
> tab[lines].methodID = m;
> lines++;
> } else {
>
> It could possibly make sense if it were iterating over the inline data
> in the jvmtiCompiledMethodLoadInlineRecord rather than the line number
> table.
> Fixing this is toward the end of a list of things I need to look at.
>
OK sure, I'll have a look at this. Thanks for the reviews.
--
Thanks,
Nick
Re: [PATCH 0/3] perf jvmti: Various fixes to JVMTI agent
On 04/27/20 18:35 pm, Jiri Olsa wrote: > > adding Stephane to the loop > > jirka > >> >> These three patches fix a couple of issues I ran into while using the >> jitdump JVMTI agent to profile the SPECjbb benchmark. >> Hi, any feedback on these patches? Thanks, Nick >> >> >> Nick Gasson (3): >> perf jvmti: Fix jitdump for methods without debug info >> perf jvmti: Do not report error when missing debug information >> perf jvmti: Fix demangling Java symbols >> >> tools/perf/jvmti/libjvmti.c | 24 +++ >> 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 + >> 6 files changed, 66 insertions(+), 19 deletions(-) >> create mode 100644 tools/perf/tests/demangle-java-test.c >> >> -- >> 2.26.1 >>
