RFR (M): 8087198: G1 card refinement: batching, sorting

Man Cao manc at google.com
Thu Nov 14 01:59:17 UTC 2019

Thanks for the reviews and testing, very helpful!

I have addressed all comments, new webrevs:
Incremental: https://cr.openjdk.java.net/~manc/8087198/webrev.00-01.inc/

Some tests involving G1AddMetaspaceDependency are crashing in
debug/fastdebug builds, due to
https://bugs.openjdk.java.net/browse/JDK-8234127 and G1UpdateBufferSize=1.
I'm working on fixing this bug for the hashtable.
I also see some Windows build failure on Submit repo, not sure if it is
related to JDK-8234127, could someone find the logs for this run?
Job: mach5-one-manc-JDK-8087198-1-20191113-2315-6689774

I also tested the performance for sorting order of the cards on
BigRamTester, below are the CPU time for refinement threads, averaged
across 34 trials with 95% confidence intervals in brackets:
base (no batching): 437656.676 [425374.295, 449939.057]
batching and decreasing order: 424714.529 [412841.728, 436587.330]
batching and increasing order: 459918.294 [448483.304, 471353.284]

Additional replies below.
> First of all, thanks for productizing my idea.
> The idea why I saved the MemRegions in my prototype, as far as I can
> remember, was so that I could coalesce consecutive ranges
You are welcome, Erik. Thanks for the explanation!

> collect_and_clean_cards could use two-finger compaction of the
> _node_buffer in place.  (Similar to the SATB buffer filtering.)
Great! I should have thought of this after removing the MemRegions.
Now it's doing the two-finger compaction, and no more memcpy and

> I think this function is poorly named.  We're not abandoning the
> cards.  We are instead abandoning the refinement of the cards in part
> of the buffer.  Something like keep_unrefined_cards might be better.
Renamed to redirty_unrefined_cards.

> This change re-reads top() after the fence.
> ...
> I *think* re-read is okay for humongous regions too, but haven't fully
> convinced myself of that yet.
I thought about this further and convinced myself it is safe.
I added some DEBUG_ONLY code to check the two reads of top()
should return the same value, by using a KVHashtable.
I'm not sure if such checking code is desirable. Please advise.
Maybe I should use ResourceHashtable as Ioi suggested in JDK-8234127.

> I already split this CR into two.
>  From a functional POV hs-tier1-5 pass with the change.
Thanks for the work! Let's finalize what to do with the DEBUG_ONLY
KVHashtable first,
then do another round of testing.

> Maybe it is used in follow-up patches in these places?
I haven't got to the follow-up patches yet.
I will create a CR for the epoch synchronization protocol, as a subtask for
Then I'll describe its implementation details and a few yet unresolved
corner cases, so we can think
about them together.


More information about the hotspot-gc-dev mailing list