RFR: 8328361: Use 'memset' in method 'CardTable::dirty_MemRegion'

Guoxiong Li gli at openjdk.org
Mon Mar 18 17:17:27 UTC 2024


On Mon, 18 Mar 2024 16:05:08 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Hi all,
>> 
>> This patch uses `memset` instead of explicit loop in method `CardTable::dirty_MemRegion` to simplify the code. Thanks for taking the time to review.
>> 
>> Best Regards,
>> -- Guoxiong
>
> 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.)

So, in this patch, should I use the method `memset_with_concurrent_readers` in `dirty_MemRegion`, `clear_MemRegion` and `resize_covered_region`?

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

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


More information about the hotspot-gc-dev mailing list