On Thu, 16 Jul 2026 12:40:48 GMT, Andrew Haley <[email protected]> wrote:

>> Please use [this 
>> link](https://github.com/openjdk/jdk/pull/28541/changes?w=1) to view the 
>> files changed.
>> 
>> Profile counters scale very badly.
>> 
>> The overhead for profiled code isn't too bad with one thread, but as the 
>> thread count increases, things go wrong very quickly.
>> 
>> For example, here's a benchmark from the OpenJDK test suite, run at 
>> TieredLevel 3 with one thread, then three threads:
>> 
>> 
>> Benchmark (randomized) Mode Cnt Score Error Units
>> InterfaceCalls.test2ndInt5Types false avgt 4 27.468 ± 2.631 ns/op
>> InterfaceCalls.test2ndInt5Types false avgt 4 240.010 ± 6.329 ns/op
>> 
>> 
>> This slowdown is caused by high memory contention on the profile counters. 
>> Not only is this slow, but it can also lose profile counts.
>> 
>> This patch is for C1 only. It'd be easy to randomize C1 counters as well in 
>> another PR, if anyone thinks it's worth doing.
>> 
>> One other thing to note is that randomized profile counters degrade very 
>> badly with small decimation ratios. For example, using a ratio of 2 with 
>> `-XX:ProfileCaptureRatio=2` with a single thread results in
>> 
>> 
>> Benchmark                        (randomized)  Mode  Cnt   Score   Error  
>> Units
>> InterfaceCalls.test2ndInt5Types         false  avgt    4  80.147 ± 9.991  
>> ns/op
>> 
>> 
>> The problem is that the branch prediction rate drops away very badly, 
>> leading to many mispredictions. It only really makes sense to use higher 
>> decimation ratios, e.g. 64.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Andrew Haley has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Merge branch 'JDK-8134940' of https://github.com/theRealAph/jdk into 
> JDK-8134940
>  - x86 review fixes
>  - Review comments

More findings...

src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp line 397:

> 395:   }
> 396: 
> 397:   __ restore_profile_rng();

Am I blind, or x86/ARM versions do not have it in their 
`LIR_Assembler::emit_unwind_handler`-s?

src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp line 2608:

> 2606:         }
> 2607:         case T_LONG: {
> 2608:           __ increment(counter_address, inc, dest);

Missing `break`, so it falls through straight to `ShouldNotReachHere()`.

src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp line 2908:

> 2906: 
> 2907:   if (counter_stub != nullptr) {
> 2908:     __ cmpl(r_profile_rng, threshold);

`threshold` is `uint64_t`? So this should be `cmpq`? Or `check_cast<int32_t>`?

src/hotspot/share/c1/c1_LIRGenerator.cpp line 967:

> 965:              data_offset_reg, as_BasicType(if_instr->x()->type()));
> 966: 
> 967:     LIR_Opr tmp = new_register(T_INT);

Please cross-check if these temps really need to be `T_LONG` on `LP64`? There 
are other `new_register(T_INT)`-s in this patch as well. It looks like the 
BranchData cells are intptr_t, so this tmp may end up dealing with it? Our 
automatic tools have discovered the `ldr x3` -> `ldr w3` change is tier3 
assembly, but I have not checked myself.

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

PR Review: https://git.openjdk.org/jdk/pull/28541#pullrequestreview-4721755106
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3602462822
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3602400970
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3602469095
PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3602393971

Reply via email to