RFR: JDK-8318444: Write details about compilation bailouts into crash reports [v4]
Christian Hagedorn
chagedorn at openjdk.org
Wed Nov 15 11:53:36 UTC 2023
On Tue, 14 Nov 2023 08:31:06 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> A little debugging aid to help analyze broken bailout chains, mainly in C2 (C1 is pretty clean).
>>
>> A broken bailout chain occurs when code marks a compilation as failed, but then either that function itself or any of its caller functions fails to abort the compilation. That may cause crashes, e.g. [JDK-8318183](https://bugs.openjdk.org/browse/JDK-8318183) or [JDK-8318445](https://bugs.openjdk.org/browse/JDK-8318445).
>>
>> Now, if the compiler initiates a bailout, it stores some context information - compile id, time, and call stack. That information is stored as part of `Compile` or `Compilation`, depending on the compiler.
>>
>> If we crash later during the same compilation, we print out that information as part of the crash report. That way, we have two call stacks, and it is easy to spot where the compiler failed to heed the bailout.
>>
>> ---------
>>
>> Looks like this (from https://github.com/openjdk/jdk/pull/16248). The first call stack is the crash point. The second call stack is the point where the compiler bailout was initiated.
>>
>>
>> Current CompileTask:
>> C2:2574 45 45 843 4 sun.nio.fs.UnixPath::resolve (17 bytes)
>>
>> Stack: [0x00007fa608cb3000,0x00007fa608db4000], sp=0x00007fa608daf310, free space=1008k
>> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
>> V [libjvm.so+0x631bb4] Unique_Node_List::push(Node*)+0x20 (node.hpp:1650)
>> V [libjvm.so+0xb8ea65] ConnectionGraph::verify_ram_nodes(Compile*, Node*)+0x87 (escape.cpp:743)
>> V [libjvm.so+0x960dda] Compile::Optimize()+0x956 (compile.cpp:2361)
>> V [libjvm.so+0x959d6c] Compile::Compile(ciEnv*, ciMethod*, int, Options, DirectiveSet*)+0x165e (compile.cpp:860)
>> V [libjvm.so+0x81bcd9] C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*)+0x203 (c2compiler.cpp:134)
>> V [libjvm.so+0x97bf63] CompileBroker::invoke_compiler_on_method(CompileTask*)+0xac5 (compileBroker.cpp:2290)
>> V [libjvm.so+0x97a981] CompileBroker::compiler_thread_loop()+0x411 (compileBroker.cpp:1951)
>> V [libjvm.so+0x99ebc0] CompilerThread::thread_entry(JavaThread*, JavaThread*)+0x84 (compilerThread.cpp:61)
>> V [libjvm.so+0xde0050] JavaThread::thread_main_inner()+0x15c (javaThread.cpp:720)
>> V [libjvm.so+0xddfeea] JavaThread::run()+0x258 (javaThread.cpp:705)
>> V [libjvm.so+0x15f5a04] Thread::call_run()+0x1a8 (thread.cpp:220)
>> V [libjvm.so+0x12de0a2] thread_native_entry(Thread*)+0x1c3 (os_linux.cpp:785)
>>
>> siginfo: si_signo: 11...
>
> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
>
> - Merge branch 'master' into JDK-8318444-Write-details-about-compilation-bailouts-into-crash-reports
> - Update src/hotspot/share/compiler/compilationFailureInfo.hpp
>
> Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
> - Update src/hotspot/share/utilities/vmError.cpp
>
> Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
> - reinstate elapsed time prefix in hs-err file
> - Merge branch 'openjdk:master' into JDK-8318444-Write-details-about-compilation-bailouts-into-crash-reports
> - wip
> - wip
> - wip
> - wip
> - JDK-8318444-Write-details-about-compilation-bailouts-into-crash-reports
That's a nice addition! I have some small comments but otherwise, looks good.
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?
src/hotspot/share/compiler/compilationFailureInfo.cpp line 90:
> 88:
> 89: const AbstractCompiler* const compiler = task->compiler();
> 90: if (compiler == nullptr) return false;
I suggest to use braces for the ifs.
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?
-------------
Marked as reviewed by chagedorn (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/16247#pullrequestreview-1731827073
PR Review Comment: https://git.openjdk.org/jdk/pull/16247#discussion_r1394085286
PR Review Comment: https://git.openjdk.org/jdk/pull/16247#discussion_r1394068291
PR Review Comment: https://git.openjdk.org/jdk/pull/16247#discussion_r1394092720
More information about the hotspot-runtime-dev
mailing list