RFR: 8335334: Stress mode to randomly execute unstable if traps [v3]

[Vladimir Kozlov]

On Wed, 18 Sep 2024 11:10:47 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Unstable if traps are supposed to be taken rarely. This patch introduces a `StressUnstableIfTraps` flag that forces unstable if traps to be taken randomly and thus potentially triggering intermittent bugs such as incorrect debug information. It works by adding another if before the unstable if that checks a "random" condition at runtime (a simple shared counter) and then either takes the trap or executes the original, unstable if.
>> 
>> This stress option also has the nice side effect of triggering re-compilation of methods that would otherwise not be re-compiled (see [JDK-8335843](https://bugs.openjdk.org/browse/JDK-8335843)).
>> 
>> I had to adjust a few tests that rely on methods being compiled / deoptimized because with the stress option enabled, deoptimization might unexpectedly (not) happen.
>> 
>> It reliably triggers [JDK-8335977](https://bugs.openjdk.org/browse/JDK-8335977), [JDK-8320308](https://bugs.openjdk.org/browse/JDK-8320308) and [JDK-8335843](https://bugs.openjdk.org/browse/JDK-8335843) in our testing.
>> 
>> Tested with multiple runs up to tier6. I'll integrate it into our (Oracle internal) testing and CTW [(JDK-8340302)](https://bugs.openjdk.org/browse/JDK-8340302) once all the bugs that it currently triggers are fixed.
>> 
>> Thanks,
>> Tobias
>
> Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Moved declaration

More comments.

src/hotspot/share/opto/parse2.cpp line 1375:

> 1373: 
> 1374: // Used by StressUnstableIfTraps
> 1375: static volatile int _trap_stress_counter = 0;

Please, check that all accesses to it use ExternalAddress (external_word_type relocation).

src/hotspot/share/opto/parse2.cpp line 1377:

> 1375: static volatile int _trap_stress_counter = 0;
> 1376: 
> 1377: void Parse::load_trap_stress_counter(Node*& counter, Node*& incr_store) {

`load_trap_` -> `increment_trap_`

src/hotspot/share/opto/parse2.cpp line 1594:

> 1592:     trap = (CallStaticJavaNode*)orig_iff->raw_out(i)->find_out_with(Op_CallStaticJava);
> 1593:     if (trap != nullptr && trap->is_uncommon_trap() && trap->jvms()->should_reexecute() &&
> 1594:         Deoptimization::trap_request_reason(trap->uncommon_trap_request()) == Deoptimization::Reason_unstable_if) {

Can we use `ProjNode::is_uncommon_trap_if_pattern()` here?

src/hotspot/share/opto/parse2.cpp line 1627:

> 1625:   trap_region->set_req(1, trap_proj);
> 1626:   trap_region->set_req(2, if_true);
> 1627:   trap->set_req(0, _gvn.transform(trap_region));

Can you use `IdealKit::if_then()` here to simplify code?

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

PR Review: https://git.openjdk.org/jdk/pull/21037#pullrequestreview-2313403156
PR Review Comment: https://git.openjdk.org/jdk/pull/21037#discussion_r1765497470
PR Review Comment: https://git.openjdk.org/jdk/pull/21037#discussion_r1765488657
PR Review Comment: https://git.openjdk.org/jdk/pull/21037#discussion_r1765493396
PR Review Comment: https://git.openjdk.org/jdk/pull/21037#discussion_r1765503522