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