On 2018-05-03 15:19, Erik Helin wrote:
On 05/02/2018 04:37 PM, Stefan Karlsson wrote:
> Hi all,
Hi Stefan,
thanks for taking on this work, much appreciated!
On 05/02/2018 04:37 PM, Stefan Karlsson wrote:
> The first patch simply cleans up some INCLUDE_ALL_GCS usages in
platform-specific files. Some of these changes are already being cleaned
up by other RFEs:
>
>
http://cr.openjdk.java.net/~stefank/8200729/webrev.04/00.removeUnneededIncludeAllGCs/
Looks good, Reviewed.
On 05/02/2018 04:37 PM, Stefan Karlsson wrote:
> The second patch pre-cleans some include files:
>
> http://cr.openjdk.java.net/~stefank/8200729/webrev.04/01.fixIncludes/
Also looks good, Reviewed.
On 05/02/2018 04:37 PM, Stefan Karlsson wrote:
> The following is the main patch, which include all relevant HotSpot
changes. For a while now, we have been actively working on abstracting
away GC specific code from non-GC directories, but as can be seen in
this patch, there are still a few places left. Hopefully, we will get
rid of most of these in the near future.
>
> http://cr.openjdk.java.net/~stefank/8200729/webrev.04/02.mainPatch/
Very nice, just one small nit:
- barrierSetConfig.hpp:
could you move "+ G1GC_ONLY(f(G1BarrierSet))" into
FOR_EACH_CONCRETE_BARRIER_SET_DO ? As in
// Do something for each concrete barrier set part of the build.
#define FOR_EACH_CONCRETE_BARRIER_SET_DO(f) \
f(CardTableBarrierSet) \
G1GC_ONLY(f(G1BarrierSet))
Yes. That's better.
As a note for people following along this thread (and to a future
version of myself), the following work is ongoing to further clean up
the GC specific bits and pieces sprinkled throughout the rest of the VM:
- 8202377: Modularize C2 GC barriers
- 8202547: Move G1 runtime calls used by generated code to
G1BarrierSetRuntime
- 8201436: Replace oop_ps_push_contents with oop_iterate and closure
In addition to the above work, this patch highlights a few more places
that needs to be taken care of:
- get rid of #if INCLUDE_CMS in defNewGeneration.cpp
- split collector specific parts of gcTrace.hpp into collector
specific tracers
- rework Threads::create_thread_roots_tasks into something more generic
that parallel then can use to implement its own
create_thread_roots_task
- remove marksweep_init() and set up MarkSweep::_gc_timer and
MarkSweep::_gc_tracer in SerialHeap and ParallelScavengeHeap
- benchmark (and measure file sizes) to see if it is still worthwhile to
keep the INCLUDE_OOP_OOP_ITERATE_BACKWARDS guard (i.e. can we always
compile the oop_iterate_backwards methods?)
- need to do some work to encapsulate the G1 and CDS code
(see e.g. oop.cpp, metaspaceShared.cpp and file.cpp)
- move VM_CollectForMetadataAllocation::initiate_concurrent_GC to
something like
CollectedHeap::initiate_concurrent_GC_for_metadata_allocation
(preferably with a shorter name
(the above list is _not_ complete, there will always be a need for
cleanups :D)
I agree.
> The last patch adds the make file support to turn on and off the
different GCs. The content of this patch has evolved from versions
written by myself, Per, and Magnus Ihse Bursie. Magnus last proposed
version used the names gc-cms, gc-g1, gc-parallel, and gc-serial, but
I've changed them to cmsgc, g1gc, parallelgc, and serialgc, so that they
match the INCLUDE_<GC> defines.
>
>
http://cr.openjdk.java.net/~stefank/8200729/webrev.04/03.selectIndivudualGCsMakePatch/
My Makefile knowledge is limited to smaller hacks, I will leave this
patch for Magnus to review (which I see he already has done).
Overall, very nice work Stefan, thanks!
Thanks Erik!
StefanK
Erik
