RFR (M): 8219747: Remove g1_ prefix to g1_remset and g1_policy members in G1CollectedHeap
Thomas Schatzl
thomas.schatzl at oracle.com
Wed Feb 27 12:44:33 UTC 2019
Hi,
thanks for your review.
On Wed, 2019-02-27 at 11:41 +0100, Aleksey Shipilev wrote:
> On 2/27/19 11:09 AM, Thomas Schatzl wrote:
> > can I have reviews for this renaming change that removes some
> > "g1_" prefixes to some G1CollectedHeap members? They are simply
> > unnecessary within the context of G1.
> >
> > There has also been a "g1_collector_policy" method - after looking
> > at its uses I decided to remove it and add a specific whitebox
> > support method for it to G1CollectedHeap instead.
> >
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8219747
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8219747/webrev/
>
> Looks good. It is a bit confusing to see the rename is_hetero_heap ->
> is_heap_heterogeneous in this
> changeset that talks about g1_ prefix removal, though.
I removed this and will file separately.
> Nits:
>
> *) Indentation in g1CollectedHeap.cpp:
>
> 864 if (policy()->need_to_start_conc_mark("concurrent humongous
> allocation",
> 865 word_size)) {
>
> 4449 policy()->phase_times()-
> >record_fast_reclaim_humongous_time_ms((os::elapsedTime() -
> start_time) * 1000.0,
> 4450
> cl.humongous_objects_reclaimed());
>
> *) Not sure if you want to rename locals too, like in
> g1ConcurrentMarkThread.cpp:
>
> 246 G1Policy* g1_policy = g1h->policy();
>
Fixed (also local names).
http://cr.openjdk.java.net/~tschatzl/8219747/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8219747/webrev.1/ (full)
Still compiles locally.
Thanks,
Thomas
More information about the hotspot-gc-dev
mailing list