RFR: JDK-8318444: Write details about compilation bailouts into crash reports [v4]

[Christian Hagedorn]

On Wed, 15 Nov 2023 12:23:00 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/compiler/compilationFailureInfo.cpp line 45:
>> 
>>> 43: #include "utilities/nativeCallStack.hpp"
>>> 44: 
>>> 45: static constexpr int skip_frames = 2;
>> 
>> Is there a specific reason to define this separately and not directly use `_stack(2)` below?
>
> None but that the argument usually goes the other way around, "why do you use raw numbers, please name that constant" :-)
> 
> No problem, I can pass 2 directly.

Hehe ok. I guess with the IDE parameter hints, it's probably fine to pass a 2 directly. But either way is fine :-)

>> src/hotspot/share/opto/compile.cpp line 995:
>> 
>>> 993: Compile::~Compile() {
>>> 994:   delete _print_inlining_stream;
>>> 995:   delete _first_failure_details;
>> 
>> Should we first check for `nullptr` before deleting here?
>
> delete, like free, accepts nullptr and does nothing. We delete/os::free null in many places.

Okay, then this should be fine as is. Was just wondering because I sometimes see that we do the following (sometimes dropping the explicit `nullptr` assignment and sometimes not):

if (x != nullptr) {
  delete x;
  x = nullptr;
}

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16247#discussion_r1394140175
PR Review Comment: https://git.openjdk.org/jdk/pull/16247#discussion_r1394140236