RFR: 8200759: Move GC entries in vmStructs.cpp to GC specific files
Aleksey Shipilev
shade at redhat.com
Thu Apr 5 11:00:23 UTC 2018
On 04/04/2018 09:37 PM, Stefan Karlsson wrote:
> Hi all,
>
> Please review this patch to move GC specific entries in vmStructs.cpp out to GC specific files.
>
> http://cr.openjdk.java.net/~stefank/8200759/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8200759
*) It feels awkward to move JavaThread::_satb_mark_queue and JavaThread::_dirty_card_queue, to
VM_STRUCTS_G1GC, since it is guarded by INCLUDE_ALL_GCS. VM_STRUCTS_GC seems more fitting. (Trivia:
Shenandoah uses those things too, so they are "shared", even though the implementation is indeed in
vm/gc/g1).
*) I'd do:
#ifdef INCLUDE_ALL_GCS
declare_constant_with_value("satbMarkQueueBufferOffset", ...
declare_constant_with_value("satbMarkQueueIndexOffset", ...
declare_constant_with_value("satbMarkQueueActiveOffset", ...
#endif
...instead of:
ALL_GCS_ONLY(declare_constant_with_value("satbMarkQueueBufferOffset", ...)
ALL_GCS_ONLY(declare_constant_with_value("satbMarkQueueIndexOffset", ...)
ALL_GCS_ONLY(declare_constant_with_value("satbMarkQueueActiveOffset", ...)
It would be easier with multiple GCs wanting to have a piece of it. For example, it would be easier
for Shenandoah to do:
#ifdef (INCLUDE_G1 || INCLUDE_SHENANDOAH)
declare_constant_with_value("satbMarkQueueBufferOffset", ...
declare_constant_with_value("satbMarkQueueIndexOffset", ...
declare_constant_with_value("satbMarkQueueActiveOffset", ...
#endif
In other words, ease off on ALL_GCS_ONLY macro, because it is not easily composable.
Ditto for e.g.:
154 ALL_GCS_ONLY(VM_TYPES_CMSGC(declare_type, \
155 declare_toplevel_type, \
156 declare_integer_type)) \
157 ALL_GCS_ONLY(VM_TYPES_G1GC(declare_type, \
158 declare_toplevel_type, \
159 declare_integer_type)) \
160 ALL_GCS_ONLY(VM_TYPES_PARALLELGC(declare_type, \
161 declare_toplevel_type, \
162 declare_integer_type)) \
...because you would later like to do:
#ifdef INCLUDE_CMS
VM_TYPES_CMSGC(declare_type, \
declare_toplevel_type, \
integer_type)) \
#endif
#ifdef INCLUDE_G1
VM_TYPES_G1GC(declare_type, \
declare_toplevel_type, \
integer_type)) \
#endif
Thanks,
-Aleksey
More information about the hotspot-dev
mailing list