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