RFR: 8200759: Move GC entries in vmStructs.cpp to GC specific files (original) (raw)

Stefan Karlsson stefan.karlsson at oracle.com
Thu Apr 5 11:47:57 UTC 2018

On 2018-04-05 13:00, Aleksey Shipilev wrote:

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::satbmarkqueue and JavaThread::dirtycardqueue, to VMSTRUCTSG1GC, since it is guarded by INCLUDEALLGCS. VMSTRUCTSGC seems more fitting. (Trivia: Shenandoah uses those things too, so they are "shared", even though the implementation is indeed in vm/gc/g1).

My follow-up changes change this to INCLUDE_G1GC, so in that sense this I don't think it's that awkward.

However, if Shenandoah needs to use this then I agree that it needs to be moved. I don't think it needs to be done for this change though.

*) I'd do: #ifdef INCLUDEALLGCS declareconstantwithvalue("satbMarkQueueBufferOffset", ... declareconstantwithvalue("satbMarkQueueIndexOffset", ... declareconstantwithvalue("satbMarkQueueActiveOffset", ... #endif ...instead of: ALLGCSONLY(declareconstantwithvalue("satbMarkQueueBufferOffset", ...) ALLGCSONLY(declareconstantwithvalue("satbMarkQueueIndexOffset", ...) ALLGCSONLY(declareconstantwithvalue("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 (INCLUDEG1 || INCLUDESHENANDOAH) declareconstantwithvalue("satbMarkQueueBufferOffset", ... declareconstantwithvalue("satbMarkQueueIndexOffset", ... declareconstantwithvalue("satbMarkQueueActiveOffset", ... #endif In other words, ease off on ALLGCSONLY macro, because it is not easily composable. Ditto for e.g.: _154 ALLGCSONLY(VMTYPESCMSGC(declaretype, _ _155 declaretopleveltype, _ _156 declareintegertype)) _ _157 ALLGCSONLY(VMTYPESG1GC(declaretype, _ _158 declaretopleveltype, _ _159 declareintegertype)) _ _160 ALLGCSONLY(VMTYPESPARALLELGC(declaretype, _ _161 declaretopleveltype, _ _162 declareintegertype)) _ ...because you would later like to do: #ifdef INCLUDECMS _VMTYPESCMSGC(declaretype, _ _declaretopleveltype, _ _integertype)) _ #endif #ifdef INCLUDEG1 _VMTYPESG1GC(declaretype, _ _declaretopleveltype, _ _integertype)) _ #endif

I can't really use your suggested constructs, because it won't compile.

StefanK

Thanks, -Aleksey

More information about the hotspot-dev mailing list