RFR: 8328361: Use memset() in method CardTable::dirty_MemRegion()

Kim Barrett kbarrett at openjdk.org
Fri Mar 29 04:57:32 UTC 2024


On Mon, 25 Mar 2024 03:15:53 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> src/hotspot/share/gc/shared/cardTable.cpp line 210:
>> 
>>> 208:   CardValue* cur  = byte_for(mr.start());
>>> 209:   CardValue* last = byte_after(mr.last());
>>> 210:   memset(cur, dirty_card, pointer_delta(last, cur, sizeof(CardValue)));
>> 
>> The "official" HotSpot usage is to use functions from the Copy utility class.
>> Though that actually doesn't get used as much as it could, with the mem*
>> functions being more frequently used. I think that's partly familiarity for
>> the mem* functions, and partly because the API for the Copy class leaves a
>> fair amount to be desired.
>> 
>> Also, I think that for G1 this probably ought to instead be
>> memset_with_concurrent_readers. It may be that the reason for using that
>> function is no longer be relevant. (Primarly, memset on Solaris/SPARC can
>> introduce spurious intermediate values. Support for that platform was dropped
>> from OpenJDK a while ago.) It may be the explicit loop used here (instead of
>> memset as in the nearby clear_MemRegion) was to intentionally avoid that
>> memset issue, back before memset_with_concurrent_readers was introduced.
>> (Unfortunately, the dirtying loop and clearing memset date back to before
>> Mercurial, so hard to dredge up any history.) (Though the explicit loop isn't
>> a guarantee - I've seen compilers sometimes turn such loops into mem* calls.)
>
> @kimbarrett What about this patch? Kindly ping.

@albertnetymk  pointed out to me that G1 doesn't (and shouldn't) use dirty_MemRegion.
(The class hierarchy would permit it, but that's a design issue of the class hierarchy.)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18349#discussion_r1544059272


More information about the hotspot-gc-dev mailing list