RFR (S) 8078438: Interpreter should support conditional card marks (UseCondCardMark)

[Andrew Dinn]

On 28/04/15 16:10, Andrew Haley wrote:
>> If so then I believe for AArch64 that this means:
>>
>> On the writing side we need to implement StoreCM using either
>>
>>   dmb  ish
>>   strb zr, ...
>>
>> or, preferably
>>
>>   strbl zr, ...
>>
>> On the reading side we need the GC thread to execute an acquiring load
>> when it reads the card, either
>>
>>   ldrb ...
>>   dmb ishld
>>
>> or, preferably
>>
>>   ldarb ...
>>
>> How exactly does the GC code do the load -- using generated code or
>> using a C library routine?
>>
>> n.b. I am just completing an update to the AArch64 volatile write rules
>> which deal with volatile object puts. The latest patch moves form
>> generating a sequence like this
>>
>>   dmb	ish
>>   lsr	x10, x13, #9
>>   str	w14, [x13,#40]
>>   strb	wzr, [xmethod,x10,lsl #0]
>>   dmb	ish
>>
>> to producing something like this
>>
>>   add	x10, x13, #0x28
>>   stlr	w14, [x10]
>>   lsr	x10, x13, #9
>>   strb	wzr, [x11,x10,lsl #0]
>>
>> The strb here is the current translation of the StoreCM.
>>
>> From what you are saying it appears that we need to implement these
>> sequences as either:
>>
>>   dmb	ish
>>   lsr	x10, x13, #9
>>   str	w14, [x13,#40]
>>   dmb ish
>>   strb	wzr, [xmethod,x10,lsl #0]
>>   dmb	ish
> 
> Eek!  No!

Well, indeed. This is what we would have to do with if we were to stick
with using dmb and str everywhere but we really should not do that.

>> or, preferably
>>
>>   add	x10, x13, #0x28
>>   stlr	w14, [x10]
>>   lsr	x10, x13, #9
>>   stlrb	wzr, [x11,x10,lsl #0]
> 
> That's more like it.

Yes, that is indeed much preferred.

Mind you the above is all academic unless we identify where the GC
thread does the card table read and ensure it employs an acquiring load.
A quick perusal of the CMS collector code suggests that it is simply
relying on method BitMap.at()

concurrentMarkSweepGeneration.inline.hpp:156

inline bool CMSBitMap::isMarked(HeapWord* addr) const {
  assert_locked();
  assert(_bmStartWord <= addr && addr < (_bmStartWord + _bmWordSize),
         "outside underlying space?");
  return _bm.at(heapWordToOffset(addr));
}

where _bm is an instance of class BitMap

concurrentMarkSweepGeneration.hpp:82

class CMSBitMap VALUE_OBJ_CLASS_SPEC {
  friend class VMStructs;
  . . .
  BitMap    _bm;            // the bit map itself

and BitMap.at is defined at utilities/bitmap.hpp:89 as

  bool at(idx_t index) const {
    verify_index(index);
    return (*word_addr(index) & bit_mask(index)) != 0;
  }

So, I guess this code needs to be patched to use an ordered access load?

> We'd want to use conditional card marking, I think.  It is possible
> also to do the release conditionally on
> CMSCollectorCardTableModRefBSExt::_requires_release.

Hmm, am I missing something? That appears only to be mentioned in
comments in the ppc code.

For conditional card marking under CMS and G1 the C2 compiler inserts a
LoadB to read the card table feeding the loade value into a an if-then
test (BoolTest::ne) which branches around the StoreCM on test failure.

Do we need to make this an acquiring load?

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in UK and Wales under Company Registration No. 3798903
Directors: Michael Cunningham (USA), Matt Parson (USA), Charlie Peters
(USA), Michael O'Neill (Ireland)