RFR (L): 8076454, 8076289 and 8076452: removing SharedHeap

Bengt Rutisson bengt.rutisson at oracle.com
Wed Apr 1 12:30:50 UTC 2015


Hi everyone,

Could I have a couple of reviews for these three patches to remove 
SharedHeap?

I'm sending all three patches in the same email because I think they are 
closely related. But I am planning to push them at three separate changes.

JDK-8076454: Clean up/move things out of SharedHeap
https://bugs.openjdk.java.net/browse/JDK-8076454
http://cr.openjdk.java.net/~brutisso/8076454/webrev.00/


JDK-8076289: Move the StrongRootsScope out of SharedHeap
https://bugs.openjdk.java.net/browse/JDK-8076289
http://cr.openjdk.java.net/~brutisso/8076289/webrev.00/


JDK-8076452: Remove SharedHeap
https://bugs.openjdk.java.net/browse/JDK-8076452
http://cr.openjdk.java.net/~brutisso/8076452/webrev.00/



Some more details about the changes:

JDK-8076454 contains many small changes. Here's the steps I took to 
complete the patch. It may be easier to review each step of the way:

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-remove_SharedHeap_gcprologue/
This removes the pure virtual implementations of 
SharedHeap::gc_prologue() and SharedHeap::gc_epilogue() since there are 
no callers of these methods. All calls are directly to the concrete 
subclasses.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-remove_SharedHeap_set_barrier_set/
This removes SharedHeap::set_barrier() and instead inlines the code in 
G1CollectedHeap and GenCollectedHeap similarly to what 
ParallelScavengeHeap already does.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-move_post_initialized_and_ref_init/
SharedHeap::post_initialize() was just forwarding to 
CollectedHeap::post_initialize() and adding a call to 
ref_processing_init(). Moved this down to the concrete sub classes.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-remove_SharedHeap_no_gc_in_progress/
no_gc_in_progress() is only used in GenCollectedHeap so it is enough 
that it is declared there.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-remove_SharedHeap_space_iterate/
space_iterate() was dead code.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-remove_SharedHeap_oop_iterate/
oop_iterate() is already declared pure virtual in CollectedHeap, so no 
need to have it in SharedHeap too.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-remove_SharedHeap_space_containing/
There were no callers of SharedHeap::space_containing(). No need to have 
a virtual method in SharedHeap. Enough to declare the methods in the 
concrete sub classes.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-cleanup_includes_and_friends/
Just some cleanups of includes and friend declarations. Not really 
relevant as the file will be completely removed by JDK-8076452.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-remove_SharedHeap_set_par_threads/
SharedHeap::set_par_threads() only introduced an assert compared to what 
CollectedHeap::set_par_threads() already does. The assert is only 
relevant in the GenCollectedHeap case. So I moved the assert down to 
that class.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-move_workers/
Move the instance variable to hold the FlexibleWorkGang (_workers) down 
to the concrete sub classes. Only two lines of common code, so hardly 
worth sharing.

http://cr.openjdk.java.net/~brutisso/8076454/webrev.00-product-fix/
I had a mistake for product builds regarding set_heap_lock_held_for_gc().



JDK-8076289 moves the StrongRootsScope class out to its own file.

http://cr.openjdk.java.net/~brutisso/8076289/webrev.00-remove_StrongRootsScope_sh/
The StrongRootsScope no longer makes use of its SharedHeap instance 
variable, so it can be removed and the constructor does not have to take 
it as a parameter.

http://cr.openjdk.java.net/~brutisso/8076289/webrev.00-move_StrongRootsScope/
Move the StrongRootsScope out to its own files and to the global name space.

http://cr.openjdk.java.net/~brutisso/8076289/webrev.00-move_MarkScope/
Move the super class of StrongRootsScope, MarkScope, into the same files.

I would like to follow this change up with a couple of things. I would 
like to rename StrongRootsScope to something like RootProcessingScope 
since we no longer use it purely for strong roots. I would like to 
rename MarkScope to something like NMethodScope. And I think 
ParallelScavenge could use MarkScope directly instead of introducing 
ParStrongRootsScope. But I would like to do these changes as a separate 
patch.


JDK-8076452 is pretty much just one patch to get rid of all reference to 
SharedHeap now that it is empty after the above changes.

Thanks,
Bengt









More information about the hotspot-gc-dev mailing list