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 <nick.gas...@arm.com> >> --- >> 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; >> } >> } >> >> @@ -122,14 +107,12 @@ get_line_numbers(jvmtiEnv *jvmti, const void >> *compile_info, jvmti_line_info_t ** >> rec = (jvmtiCompiledMethodLoadInlineRecord *)hdr; >> for (i = 0; i < rec->numpcs; i++) { >> c = rec->pcinfo + i; >> - nr = 0; >> - ret = do_get_line_numbers(jvmti, c->pc, >> - c->methods[0], >> - c->bcis[0], >> - *tab + lines_total, >> - &nr); >> + ret = do_get_line_number(jvmti, c->pc, >> + c->methods[0], >> + c->bcis[0], >> + *tab + lines_total); >> if (ret == JVMTI_ERROR_NONE) >> - lines_total += nr; >> + lines_total++; >> } >> } >> } >> 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 >>
