RFR (M) 8138762: Refactor setup of evacuation closures in G1
Erik Helin
erik.helin at oracle.com
Tue Oct 6 13:36:54 UTC 2015
Hi Mikael,
first of all, thanks for a really nice patch! I think the patch
essentially is good to go, just a few comments inline.
On 2015-10-02, Mikael Gerdin wrote:
> I've split the change into two different webrevs.
> The first one moves a bunch of code out from g1CollectedHeap.cpp to become
> externally visible.
> I also noticed that g1CodeBlobClosure.hpp is missing include guards.
> To make room for the closure set in the par scan state I remove the unused
> g1 rem set pointer in this first webrev.
>
> http://cr.openjdk.java.net/~mgerdin/8138762_1/webrev.0/
Looks good, Reviewed.
> The second webrev contains the actual change.
> I decided to separate the interface headers and the actual implementations
> of the closure sets in two separate files.
> g1RootClosureSet.hpp contains the interface and g1EvacuationClosures.hpp
> contains the implementations used by a G1 evacuation pause.
>
> http://cr.openjdk.java.net/~mgerdin/8138762_2/webrev.0/
- g1CollectedHeap.cpp:
The variable trace_metadata can be initialized with the
expression:
`bool trace_metadata =
_g1h->collector_state()->during_initial_mark_pause() &&
ClassUnloadingWithConcurrentMark;`
- g1RootProcessor.cpp:
Could you update the assert comments for:
+ assert(closures->second_pass_weak_clds() != NULL, "should be
null");
+ assert(closures->second_pass_weak_clds() == NULL, "should not
null");
- g1RootClosureSet.hpp:
Could you rename the interface (and the file) to g1RootClosures?
Correspondingly, rename G1EvacuationRootClosureSet to
G1RootEvacuationClosures?
- g1EvacuationClosures.hpp:
Rename G1ClosureSet to G1SharedClosures?
I'm not too fond of having comments before bool arguments,
like /* process_only_dirty_klasses */. Personally I would remove them,
but if you want to keep them, that is fine by me as well.
Thanks,
Erik
> The full webrev is available at:
> http://cr.openjdk.java.net/~mgerdin/8138762_full/webrev.0/
>
> Testing:
> JPRT
> Performance (aurora.se G1 workloads on Solaris x64)
> Kitchensink,runThese,vm.gc.testlist,vm.g1classunload.testlist
>
> I appreciate any feedback, if you have suggestions for different class names
> I'm happy to change them, I had some trouble coming up with good ones.
>
> Thanks
> /Mikael
More information about the hotspot-gc-dev
mailing list