On Mon, 23 Mar 2026 18:43:41 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. > > Andrew Haley has updated the pull request incrementally with one additional > commit since the last revision: > > Fix up any out-of-range offsets A few questions but looks ok. src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 3239: > 3237: starti; > \ > 3238: f(sf, 31), f(0b0011010110, 30, 21), f(0b010, 15, 13), f(c, 12); > \ > 3239: f(sz, 11, 10), zrf(Rm, 16), rf(Rn, 5), rf(Rd, 0); > \ Why introduce this as part of the current change? Also, why is Rm installed using zrf but Rn and Rd installed using rf? They are all documented as Xm/Wm etc in the ARM ARM. src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp line 2519: > 2517: void LIR_Assembler::increment_profile_ctr(LIR_Opr step, LIR_Opr > dest_opr, > 2518: LIR_Opr freq_opr, > 2519: LIR_Opr md_reg, LIR_Opr > md_opr, LIR_Opr md_offset_opr, nit: can you reformat so that md_reg is moved up to the preceding line? (alternatively move freq_opr up to the previous line) src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp line 1299: > 1297: int mdp_offset = md->byte_offset_of_slot(data, in_ByteSize(0)); > 1298: if (ProfileCaptureRatio > 1) { > 1299: __ profile_receiver_type nit: why the newline? src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp line 1353: > 1351: int profile_capture_ratio = ProfileCaptureRatio; > 1352: int ratio_shift = exact_log2(profile_capture_ratio); > 1353: auto threshold = (1ull << 32) >> ratio_shift; again why is this different to the explicit `UCONST64(1)` below? src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp line 1478: > 1476: ciProfileData* data = nullptr; > 1477: > 1478: __ block_comment("void > LIR_Assembler::emit_opTypeCheck(LIR_OpTypeCheck* op) {"); What's the value of providing this comment with a '{' but no matching '}'? src/hotspot/share/c1/c1_LIRGenerator.cpp line 971: > 969: LIR_Opr tmp = new_register(T_INT); > 970: LIR_Opr step = LIR_OprFact::intConst(DataLayout::counter_increment); > 971: LIR_Opr dummy = LIR_OprFact::intConst(0); What is dummy for? src/hotspot/share/c1/c1_LIRGenerator.cpp line 2416: > 2414: LIR_Opr md_reg = new_register(T_METADATA); > 2415: LIR_Opr tmp = new_register(T_INT); > 2416: LIR_Opr dummy = LIR_OprFact::intptrConst((intptr_t)0); Again what is dummy for? ------------- PR Review: https://git.openjdk.org/jdk/pull/28541#pullrequestreview-4036235441 PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3014872823 PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3020846522 PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3022456743 PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3022531415 PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3022515737 PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3022600517 PR Review Comment: https://git.openjdk.org/jdk/pull/28541#discussion_r3022601699
