RFR (M): 8153503: Move remset scan iteration claim to remset local data structure
derek.white at oracle.com
Wed Apr 13 20:33:28 UTC 2016
On 4/13/16 2:06 PM, Thomas Schatzl wrote:
> On Wed, 2016-04-13 at 13:27 -0400, Derek White wrote:
>> Hi Thomas,
>> Small comment at the end...
>>>> I'll look at this more on Monday.
>>>> - Derek
>> Sorry for dripping this out. I spent a lot of time trying to
>> understand what G1ScanRSClosure::doHeapRegion() was really doing -
>> what it was iterating over, how it was filtering out the claimed
>> cards, etc. If that iteration were done anywhere else I'd suggest
>> separating it out into a "claimed" iterator class (but it's not done
>> anywhere else).
> Not sure what you mean, but probably something like SubTasksDone or so.
> The problem is that in this case, you want other threads to help each
> other when one region takes too long. So really every thread wants to
> iterate over all heap regions.
> Further, there is this matter that parts of the work should only ever
> done by a single thread if possible (adding the region to the dirty
> cards regions) or must be (code root scan).
> One can of course separate the tasks (update rs, scan rs and scan code
> roots), but I do not really want to address this here. There is the
> separate issue JDK-8153505 already.
Oh, I wasn't thinking of anything that grand, just a wrapper (another
iterator) around HeapRegionRemSetIterator::has_next() that only returns
claimed card_indexes (eg. factoring out all of the filtering). But it's
not worth it since this is only done in one place anyway.
>> One suggestion to existing code:
>> g1RemSet.cpp, Line 235 (etc):
>> Rename "jump_to_card" => "claimed_cards". The "jump" term pre-dates
>> current parallelization scheme and now isn't very meaningful.
> I think nothing changed conceptually in this part. I will see if I can
> find a better name for that variable though. :)
Sorry, I was referring to rev 1261 back in 2010 when they switched to a
block-based parallelization :-) The previous code was full of "jumps"
and "skips", which is why we're still talking about "jump_to_card". A
name that mentions "claims" and/or "blocks" would be more useful.
> There are much better ways to handle work distribution for scan rs, but
> in this CR (and probably for JDK 9 FC) I would prefer if we did not
> change how claiming work works.
Final review comment:
More information about the hotspot-gc-dev