RFR: 8311542: Consolidate the native stack printing code

David Holmes dholmes at openjdk.org
Fri Dec 6 04:25:42 UTC 2024


On Fri, 6 Dec 2024 02:53:28 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> We now print native stacks in a number of contexts, not just VMError reporting, and we have to try `os::platform_print_native_stack` else fall back to `VMError::print_native stack`. 
>> 
>> The refactoring adds a new `NativeStackPrinter` (NSP) helper class which can be constructed with some of the context information for the printing that will follow. This avoids the need for the print functions to have a large number of parameters. We have to expose both the top-level printing functionality and the "lower-level" frame-based stack walk as the error reporter needs access to that directly. To maintain the exact same format of output the NSP has to be aware of some error reporter usage requirements.
>> 
>> I also had to expose a direct `frame` taking print function for the Debug.cpp `pns` case.
>> 
>> Testing:
>>  - tiers 1 - 4
>> 
>> Some frame management code was also moved from `VMError` to the `frame` class.
>
> Looks good, except for some const issues.  I'm approving as is if you decide not to
> address those in this PR.

Thanks for the review @kimbarrett !

> src/hotspot/share/utilities/nativeStackPrinter.cpp line 44:
> 
>> 42: }
>> 43: 
>> 44: void NativeStackPrinter::print_stack_from_frame(outputStream* st, frame& fr,
> 
> Why is `frame` passed by non-const reference?  This doesn't seem like a function that should be modifying
> it?  Though changing this might also run afoul of const-correctness issues when accessing the frame, so
> feel free to defer to a followup if that's a problem.

I passed by reference here just to avoid adding in an additional level of copying in the new code as I was unsure if an additional copy might have an adverse impact. I never considered applying const and I also suspect it will have a significant flow-on effect if I try.

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

PR Comment: https://git.openjdk.org/jdk/pull/22472#issuecomment-2522093608
PR Review Comment: https://git.openjdk.org/jdk/pull/22472#discussion_r1872551599


More information about the hotspot-runtime-dev mailing list