RFR [1/7]: 8197569: Refactor eager reclaim for concurrent remembered set rebuilding (original) (raw)
Thomas Schatzl thomas.schatzl at oracle.com
Tue Mar 6 13:17:53 UTC 2018
- Previous message (by thread): RFR [1/7]: 8197569: Refactor eager reclaim for concurrent remembered set rebuilding
- Next message (by thread): RFR [1/7]: 8197569: Refactor eager reclaim for concurrent remembered set rebuilding
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi Stefan,
thanks for looking into this.
On Mon, 2018-03-05 at 16:11 +0100, Stefan Johansson wrote:
Thanks for splitting this change up Thomas,
On 2018-03-05 15:18, Thomas Schatzl wrote: > Hi all, > > can I have some reviews for the following change that refactors > the > eager reclaim condition so that it can be later used for the policy > that determines for which regions G1 should rebuild the remembered > set? > (And it's a reasonable refactoring on its own btw). > > CR: > https://bugs.openjdk.java.net/browse/JDK-8197569 > Webrev: > http://cr.openjdk.java.net/~tschatzl/8197569/webrev/ Look good, just one thing: src/hotspot/share/gc/g1/g1CollectedHeap.cpp 2542 bool humongousregioniscandidate(G1CollectedHeap* heap, HeapRegion* region) const { ... 2594 return obj->istypeArray() && 2595 G1CollectedHeap::heap()- >ispotentialeagerreclaimcandidate(region); You could use the passed in heap parameter instead of using G1CollectedHeap::heap().
Done. Also renamed the parameter to "g1h" as used everywhere else.
Another, maybe even nicer, solution would be to move ispotentialeagerreclaimcandidate() to HeapRegion, and just call region->ispotentialeagerreclaimcandidate().
I did not do this for the following reasons:
- is_potential_eager_reclaim_candidate may depend on global state in the future. E.g. when trying to eagerly reclaim objArrays or similar the result depends on whether we are currently marking or not.
I do not like making HeapRegion dependent on e.g. G1CollectorState :) (any more than it already may be).
- (almost) all eager reclaim related code has until now been in G1CollectedHeap. Such a change would go against this, and run kind of contrary to my intent to move away from using HeapRegion as dumping ground for anything loosely related to it out of convenience, but only as a representation for that memory.
Either way is kind of fine for me right now, so let's see what others say?
New webrevs: http://cr.openjdk.java.net/~tschatzl/8197569/webrev.0_to_1/ (diff) http://cr.openjdk.java.net/~tschatzl/8197569/webrev.1/ (full)
Thanks, Thomas
- Previous message (by thread): RFR [1/7]: 8197569: Refactor eager reclaim for concurrent remembered set rebuilding
- Next message (by thread): RFR [1/7]: 8197569: Refactor eager reclaim for concurrent remembered set rebuilding
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]