RFR: 8202082: Remove explicit CMS checks in CardTableBarrierSetAssembler (original) (raw)
Erik Österlund erik.osterlund at oracle.com
Tue Apr 24 06:57:48 UTC 2018
- Previous message: RFR: 8202082: Remove explicit CMS checks in CardTableBarrierSetAssembler
- Next message: RFR: 8202082: Remove explicit CMS checks in CardTableBarrierSetAssembler
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi Kim,
On 2018-04-24 04:15, Kim Barrett wrote:
On Apr 20, 2018, at 10:43 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
Hi, In CardTableBarrierSetAssembler, we sometimes need fencing due to the card table being scanned concurrently when using CMS with pre-cleaning. Rather than explicitly checking for those CMS flags in the generic CardTableBarrierSetAssembler class, it is preferrable to check the CardTable scannedconcurrently() property which already precisely reflects that. Bug: https://bugs.openjdk.java.net/browse/JDK-8202082 Webrev: http://cr.openjdk.java.net/~eosterlund/8202082/webrev.00/ Thanks, /Erik ------------------------------------------------------------------------------ src/hotspot/cpu/x86/gc/shared/cardTableBarrierSetAssemblerx86.cpp 121 if (ct->scannedconcurrently()) { I think the correct test here is cardmarkmustfollowstore. See my review of 8202083 for more details. Similarly for the other platforms.
I disagree. Let me explain why, and where I am trying to get with this.
We used to have a whole bunch of different ways of making decisions based on whether the card table is scanned concurrently or not, queried in various ways, depending on whether we were talking to the CollectedHeap interface, the CardTableModRefBS interface, or whether we were inside of the CardTableModRefBS class. I am trying to remove those one by one, until only one is left, the configuration point in CardTable which describes precisely that: that the card table is scanned concurrently. Here are a few examples:
- The ReduceInitialCardMarks optimization either defers card marks or issues them immediately on slowpath allocations, depending on whether the card table is scanned concurrently or not (this used to ask CollectedHeap about these queries).
- Inside of the post barriers, StoreStore fencing between the reference store and card mark depends on whether the card table is scanned concurrently or not. (checked with explicit CMS + precleaning and sometimes conservatively just CMS checks)
- When using UseCondCardMark, there is a StoreLoad fence to make sure the store of the reference happens-before the load of the card value (not the store of the card value) for filtering. Again, this is only an issue when the card table is scanned concurrently, and not very well described by whether the card mark must follow the store IMO.
The reason there is not yet a single property is that some code has previously asked CollectedHeap about these questions from outside of GC code (ReduceInitialCardMarks), while other code lived in the card table barrier set (CMS + pre-cleaning checks). Where I want to go is that no code outside of the CardTableBarrierSet classes should need to ask these questions. And code inside of the CardTableBarrierSet classes should ask its CardTable, which acts as a configuration, whether it is scanned concurrently or not.
So basically, I would prefer to now remove the card_mark_must_follow_store() method, and replace it with direct calls to whether the card table is scanned concurrently.
Do you agree?
------------------------------------------------------------------------------ src/hotspot/cpu/x86/gc/shared/cardTableBarrierSetAssemblerx86.cpp 113 AddressLiteral cardtable((address)ctbs->cardtable()->bytemapbase(), relocInfo::none);
[pre-existing] Why isn't this using (address)disp rather than refetching the value. ------------------------------------------------------------------------------
That seems unrelated to my changes.
Thanks, /Erik
- Previous message: RFR: 8202082: Remove explicit CMS checks in CardTableBarrierSetAssembler
- Next message: RFR: 8202082: Remove explicit CMS checks in CardTableBarrierSetAssembler
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]