RFR: 8333994: NMT: call stacks should show source information [v3]

[Thomas Stuefe]

On Fri, 21 Jun 2024 20:50:17 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
>> 
>>  - fix windows build
>>  - Merge branch 'master' into JDK-8333994-NMT-call-stacks-should-show-source-information
>>  - caching
>>  - Merge branch 'master' into JDK-8333994-NMT-call-stacks-should-show-source-information
>>  - exclude macos from testing source info
>>  - copyrights
>>  - test
>>  - JDK-8333994-NMT-call-stacks-should-show-source-information
>
> src/hotspot/share/utilities/nativeCallStack.cpp line 130:
> 
>> 128:       // cached?
>> 129:       bool created = false;
>> 130:       Entry* const cached_value = _cache.put_if_absent(pc, &created);
> 
> `cached_value` sounds generic, could we have `cached_stream_text` or something more descriptive?

Replaced it with cached_frame_text

> src/hotspot/share/utilities/nativeCallStack.hpp line 136:
> 
>> 134: // a NativeCallStackPrinter improves performance by caching printed frames by address.
>> 135: class NativeCallStackPrinter {
>> 136:   struct Entry { char text[1024]; };
> 
> 1024 is big enough? Would 512 suffice?
> 
> Not that important if we were creating/destroying it as needed, but right now it is always there...

Frame strings can get very lengthy. Think templatized function names, or long file names. In theory, 1024 can be not enough. 

I first wanted to do this via strdup, but that would mean that we use twice as many mallocs, copy the strings around unnecessarily, and need to manually deallocate after reporting. So a fixed-size entry is a compromise for simplicity.

Note that this is only used during detail reports. Those are memory-hungry anyway, since we snapshot the whole state.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19655#discussion_r1649572271
PR Review Comment: https://git.openjdk.org/jdk/pull/19655#discussion_r1649572192