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

Kim Barrett kbarrett at openjdk.org
Mon Mar 18 16:08:27 UTC 2024


On Mon, 18 Mar 2024 14:17:18 GMT, Guoxiong Li <gli 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

Changes requested by kbarrett (Reviewer).

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.)

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

PR Review: https://git.openjdk.org/jdk/pull/18349#pullrequestreview-1943544985
PR Review Comment: https://git.openjdk.org/jdk/pull/18349#discussion_r1528846606


More information about the hotspot-gc-dev mailing list