RFR: JDK-8299229: [JVMCI] add support for UseZGC [v7]
Stefan Karlsson
stefank at openjdk.org
Thu May 4 18:30:31 UTC 2023
On Thu, 4 May 2023 17:36:26 GMT, Tom Rodriguez <never at openjdk.org> wrote:
>> This exposes the required ZGC values to JVMCI and adds support for nmethod entry barriers. The ZGC support is straightforward but the nmethod entry barrier required some reworking to fit better into JVMCI usage. I also removed the epoch based barrier since it was no longer used with simplified the assumptions on the JVMCI side. There is also a minor loom related fix to support post call nops included. I could move that into a separate PR if that would be preferred.
>
> Tom Rodriguez has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits:
>
> - Merge branch 'master' into tkr-zgc
> - Fix mdo iteration and riscv code
> - Fix handling of extra data
> - Merge branch 'master' into tkr-zgc
> - Require nmethod entry barrier emission
> - Merge branch 'master' into tkr-zgc
> - Use reloc for guard location and read internal fields using HotSpot accessors
> - Merge branch 'master' into tkr-zgc
> - Remove access to extra data section from Java code
> - Handle concurrent unloading
> - ... and 6 more: https://git.openjdk.org/jdk/compare/fc76687c...a0dae2be
This adds support for the non-generational ZGC. We are currently in the process of reviewing Generational ZGC. Have @fisk talked to you about that? It's unclear to me if Generational ZGC will immediately break these changes. Erik could you shed some light w.r.t that?
I took the opportunity to look at the patch to see if we would get any major merge conflicts. While looking through the patch I found a few nits that I think would be nice to get fixed.
src/hotspot/cpu/aarch64/gc/shared/barrierSetNMethod_aarch64.cpp line 39:
> 37: #include "utilities/align.hpp"
> 38: #include "utilities/formatBuffer.hpp"
> 39: #include "utilities/debug.hpp"
Sort includes.
src/hotspot/share/code/nmethod.cpp line 861:
> 859: _speculations_offset = _nul_chk_table_offset + align_up(nul_chk_table->size_in_bytes(), oopSize);
> 860: _jvmci_data_offset = _speculations_offset + align_up(speculations_len, oopSize);
> 861: int jvmci_data_size = compiler->is_jvmci() ? jvmci_data->size() : 0;
Indentation seems off.
src/hotspot/share/gc/shared/barrierSetNMethod.hpp line 33:
> 31: #if INCLUDE_JVMCI
> 32: #include "utilities/formatBuffer.hpp"
> 33: #endif
Given that this doesn't include any JVMCI files I think we can skip the INCLUDE_JVMCI guard (and sort in the added include). Or maybe better, forward declare FormatBuffer?
src/hotspot/share/jvmci/jvmciCodeInstaller.cpp line 783:
> 781: }
> 782: }
> 783: }
We have this typedef:
typedef FormatBuffer<> err_msg;
So, it looks weird to have code that mixes FormatBuffer<> and err_msg.
src/hotspot/share/jvmci/jvmciCompilerToVM.hpp line 28:
> 26:
> 27: #include "gc/shared/cardTable.hpp"
> 28: #include "gc/shared/barrierSetAssembler.hpp"
Sort includes.
src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp line 34:
> 32: #include "gc/shared/gc_globals.hpp"
> 33: #include "gc/shared/tlab_globals.hpp"
> 34: #include "gc/shared/barrierSetNMethod.hpp"
Sort includes.
src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp line 36:
> 34: #include "gc/shared/barrierSetNMethod.hpp"
> 35: #include "gc/z/zThreadLocalData.hpp"
> 36: #include "gc/z/zBarrierSetRuntime.hpp"
Should this be guarded with an INCLUDE_ZGC check? This also needs to be sorted.
src/hotspot/share/jvmci/jvmciEnv.hpp line 30:
> 28: #include "classfile/javaClasses.hpp"
> 29: #include "jvmci/jvmciJavaClasses.hpp"
> 30: #include "oops/klass.inline.hpp"
Don't include .inline.hpp file in .hpp files.
-------------
PR Review: https://git.openjdk.org/jdk/pull/11996#pullrequestreview-1413619985
PR Review Comment: https://git.openjdk.org/jdk/pull/11996#discussion_r1185337757
PR Review Comment: https://git.openjdk.org/jdk/pull/11996#discussion_r1185347190
PR Review Comment: https://git.openjdk.org/jdk/pull/11996#discussion_r1185348575
PR Review Comment: https://git.openjdk.org/jdk/pull/11996#discussion_r1185353494
PR Review Comment: https://git.openjdk.org/jdk/pull/11996#discussion_r1185340442
PR Review Comment: https://git.openjdk.org/jdk/pull/11996#discussion_r1185341203
PR Review Comment: https://git.openjdk.org/jdk/pull/11996#discussion_r1185341688
PR Review Comment: https://git.openjdk.org/jdk/pull/11996#discussion_r1185342470
More information about the hotspot-dev
mailing list