RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

classic Classic list List threaded Threaded
55 messages Options
123
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

Thomas Schatzl
Hi all,

  I would like you to ask for reviews of a set of related changes that
optimize the way G1 updates/scans/refines cards of the remembered set.

As these CRs are highly dependent on each other I would like to have
them reviewed and pushed together. Another reason is that all of them
are self-contained, which means that they take one or the other detour
I want to avoid questions about.

The basic premise is that refinement and Scan RS (and Update RS as it
is mostly the same) phases are quite slow with G1. There are a lot of
low-hanging fruits to be picked here, ranging from micro-optimizations,
fixing "wrong" code sharing, re-using (slow) CMS code, doing stuff in
non-straightforward ways etc.

These changes try to make it better (tm) ;)

Results are good, around 10%+ faster scan rs time/card, 10%+ faster
update rs/card, refinement also faster (although it is too hard to
measure), 8 LOC less (there is more to delete, but that one needs more
care. Also these changes add some logging :))

Here we go:

8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful() for
use during concurrent refinement and updating the rset

CR: https://bugs.openjdk.java.net/browse/JDK-8071280
Webrev: http://cr.openjdk.java.net/~tschatzl/8071280/webrev

As the title indicates, this change allows specializations of
HeapRegion::oops_on_card_seq_iterate_careful() according to current GC
phase (concurrent or during gc) to allow to remove some checks.
This is mostly a preparatory change for the next ones.

8162928: Micro-optimizations in scanning the remembered sets

CR: https://bugs.openjdk.java.net/browse/JDK-8162928
Webrev: http://cr.openjdk.java.net/~tschatzl/8162928/webrev/

This change removes the use of HeapRegionDCTOC during scanning
remembered sets, and uses the faster
HeapRegion::oops_on_card_seq_iterate_careful() (measured).
HeapRegionDCTOC also depends CMS code which is just bad.

8177707: Specialize G1RemSet::refine_card for concurrent/during
safepoint refinement

CR: https://bugs.openjdk.java.net/browse/JDK-8177707
Webrev: http://cr.openjdk.java.net/~tschatzl/8177707/webrev/

This change temporarily duplicates the code in G1RemSet::refine_card
for during refinement/gc to allow (initial) specialization of these
methods for their purpose.

8177044: Remove _scan_top from HeapRegion

CR: https://bugs.openjdk.java.net/browse/JDK-8177044
Webrev: http://cr.openjdk.java.net/~tschatzl/8177044/

Removes _scan_top and all its awkward handling by explicitly creating a
snapshot of the top values of the heap regions. This allows significant
specialization of the refine_during_gc() method by subsuming all the
various checks at the beginning into a single one.

This one is responsible for a large part of the improvement of the
update rs phase.

8178148: Log more detailed information about scan rs phase

CR: https://bugs.openjdk.java.net/browse/JDK-8178148
Webrev: http://cr.openjdk.java.net/~tschatzl/8178148/webrev/

Adds some more statistics gathering and logging about what the scan rs
phase is actually doing, allowing better performance analysis. This
information will also be used further in the future.

8175554: Improve G1UpdateRSOrPushRefClosure

CR: https://bugs.openjdk.java.net/browse/JDK-8175554
Webrev: http://cr.openjdk.java.net/~tschatzl/8175554/webrev/

(This mostly deletes code, so it's still "S" ;))

Finally remove the G1ParPushHeapRSClosure and merge it with
G1UpdateOrScanRSClosure. You may now notice that
G1UpdateOrScanRSClosure is pretty similar to G1ParScanClosure - that is
another issue.
Also make sure that G1UpdateOrScanRSClosure properly does the humongous
and "ext" region handling (which is benign so far).

8178151: Clean up G1RemSet files

CR: https://bugs.openjdk.java.net/browse/JDK-8178151
Webrev: http://cr.openjdk.java.net/~tschatzl/8178151/webrev/

Clean up after all these changes a little in the G1Remset* files,
removing obsolete stuff, some renamings, etc. 

Thanks,
  Thomas

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

Thomas Schatzl
Hi all,

On Tue, 2017-04-11 at 13:30 +0200, Thomas Schatzl wrote:
> Hi all,



>   I would like you to ask for reviews of a set of related changes
> that optimize the way G1 updates/scans/refines cards of the
> remembered set.
  I forgot to add comments about testing:

- jprt for all changes, stacked in the order given
- rbt common and gc for tier 2-5 for everything
- perf benchmarks for almost all of the phases, and all together
- some additional large heap benchmarks

Thanks,
  Thomas

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

Kim Barrett
In reply to this post by Thomas Schatzl
> On Apr 11, 2017, at 7:30 AM, Thomas Schatzl <[hidden email]> wrote:
> 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful() for
> use during concurrent refinement and updating the rset
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8071280
> Webrev: http://cr.openjdk.java.net/~tschatzl/8071280/webrev
>
> As the title indicates, this change allows specializations of
> HeapRegion::oops_on_card_seq_iterate_careful() according to current GC
> phase (concurrent or during gc) to allow to remove some checks.
> This is mostly a preparatory change for the next ones.

Only this changeset.  I’ll be sending out comments on others later, separate messages for each.

------------------------------------------------------------------------------
src/share/vm/gc/g1/heapRegion.inline.hpp
 149   assert(ClassUnloadingWithConcurrentMark,

Is this assertion really correct?  There is a similar assertion in
block_size, but it is protected by a call to block_is_obj.  I'm not
seeing similar protection for this one.  Maybe I'm missing someting?

------------------------------------------------------------------------------
src/share/vm/gc/g1/g1RemSet.cpp
 728   }

Close brace is now mis-indented.

------------------------------------------------------------------------------
src/share/vm/gc/g1/heapRegion.hpp
 329   template <class Closure, bool is_gc_active>
 330   inline bool do_oops_on_card_in_humongous(MemRegion mr,

and

 682   template <class Closure, bool is_gc_active>
 683   inline bool oops_on_card_seq_iterate_careful(MemRegion mr, Closure* cl);

It would be better to have the is_gc_active template parameter first.
That would allow the Closure parameter to be deduced in calls, e.g.
instead of

    card_processed = r->oops_on_card_seq_iterate_careful<G1UpdateRSOrPushRefOopClosure, true>(dirty_region, &update_rs_oop_cl);

one would use

    card_processed = r->oops_on_card_seq_iterate_careful<true>(dirty_region, &update_rs_oop_cl);

with Closure being deduced from the type of the argument in the call.

------------------------------------------------------------------------------

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

Kim Barrett
In reply to this post by Thomas Schatzl
> On Apr 11, 2017, at 7:30 AM, Thomas Schatzl <[hidden email]> wrote:
>
> Hi all,
>
>   I would like you to ask for reviews of a set of related changes that
> optimize the way G1 updates/scans/refines cards of the remembered set.
>
> […]
> Here we go:
>
> […]
> 8162928: Micro-optimizations in scanning the remembered sets
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8162928
> Webrev: http://cr.openjdk.java.net/~tschatzl/8162928/webrev/
>
> This change removes the use of HeapRegionDCTOC during scanning
> remembered sets, and uses the faster
> HeapRegion::oops_on_card_seq_iterate_careful() (measured).
> HeapRegionDCTOC also depends CMS code which is just bad.

Looks good.

------------------------------------------------------------------------------
src/share/vm/gc/g1/g1RemSet.cpp
 318     r->oops_on_card_seq_iterate_careful<G1ParPushHeapRSClosure, true>(mr, _push_heap_cl);

This will of course need to be updated for my earlier suggested change
to reorder the template parameters.  I don't need a new webrev for that.

------------------------------------------------------------------------------


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

Kim Barrett
In reply to this post by Thomas Schatzl
> On Apr 11, 2017, at 7:30 AM, Thomas Schatzl <[hidden email]> wrote:
>
> Hi all,
>
>   I would like you to ask for reviews of a set of related changes that
> optimize the way G1 updates/scans/refines cards of the remembered set.
>
> […]
> 8177707: Specialize G1RemSet::refine_card for concurrent/during
> safepoint refinement
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8177707
> Webrev: http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
>
> This change temporarily duplicates the code in G1RemSet::refine_card
> for during refinement/gc to allow (initial) specialization of these
> methods for their purpose.

------------------------------------------------------------------------------
src/share/vm/gc/g1/g1RemSet.cpp
 588   // While we are processing RSet buffers during the collection, we
...
 598   assert(!r->in_collection_set(), "Concurrent refinement should not encounter cards in the collection set.");

The comment starting at line 588 seems like it needs updating to
account for line 598 being changed from an "if" to an "assert".

However, I'm not sure this change change to an assert is correct,
because of the possibility of stale cards.

------------------------------------------------------------------------------
src/share/vm/gc/g1/g1RemSet.cpp
 636   HeapWord* scan_limit = r->top();

I think the commentary about this initialization is important and
ought not be deleted.

Similarly for the use of scan_top() in the gc_active case.

------------------------------------------------------------------------------
src/share/vm/gc/g1/g1RemSet.cpp
 688 bool G1RemSet::refine_card_during_gc(jbyte* card_ptr,

I sure hope a lot of the near duplication with the concurrent version
is going to go away in one of the later changes in this stack.

------------------------------------------------------------------------------  
src/share/vm/gc/g1/g1RemSet.cpp
 777   _conc_refine_cards++;

Since this is not in "concurrent" refinement, is it still correct to
include it in this count?  Maybe the variable name is not right?  If
so, I'd be okay with a variable rename in a later change if that makes
management of this stack of changes easier.

------------------------------------------------------------------------------  
src/share/vm/gc/g1/g1OopClosures.inline.hpp
 114 inline static void check_obj_during_refinement(T* p, oop const obj) {

Just "inline" is sufficient, no "static" needed.

------------------------------------------------------------------------------

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

sangheon.kim@oracle.com
In reply to this post by Thomas Schatzl
Hi Thomas,

On 04/11/2017 04:30 AM, Thomas Schatzl wrote:

> Hi all,
>
>    I would like you to ask for reviews of a set of related changes that
> optimize the way G1 updates/scans/refines cards of the remembered set.
>
> As these CRs are highly dependent on each other I would like to have
> them reviewed and pushed together. Another reason is that all of them
> are self-contained, which means that they take one or the other detour
> I want to avoid questions about.
>
> The basic premise is that refinement and Scan RS (and Update RS as it
> is mostly the same) phases are quite slow with G1. There are a lot of
> low-hanging fruits to be picked here, ranging from micro-optimizations,
> fixing "wrong" code sharing, re-using (slow) CMS code, doing stuff in
> non-straightforward ways etc.
>
> These changes try to make it better (tm) ;)
>
> Results are good, around 10%+ faster scan rs time/card, 10%+ faster
> update rs/card, refinement also faster (although it is too hard to
> measure), 8 LOC less (there is more to delete, but that one needs more
> care. Also these changes add some logging :))
>
> Here we go:
>
> 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful() for
> use during concurrent refinement and updating the rset
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8071280
> Webrev: http://cr.openjdk.java.net/~tschatzl/8071280/webrev
>
> As the title indicates, this change allows specializations of
> HeapRegion::oops_on_card_seq_iterate_careful() according to current GC
> phase (concurrent or during gc) to allow to remove some checks.
> This is mostly a preparatory change for the next ones.
8071280, seems good to me.

src/share/vm/gc/g1/heapRegion.inline.hpp
  149   assert(ClassUnloadingWithConcurrentMark,

I have same question as Kim about this assertion.
In addition, block_size_during_gc() and block_size() are very similar
except block_size() could return earlier.
Could you give me some explanation for this?

Thanks,
Sangheon


>
> 8162928: Micro-optimizations in scanning the remembered sets
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8162928
> Webrev: http://cr.openjdk.java.net/~tschatzl/8162928/webrev/
>
> This change removes the use of HeapRegionDCTOC during scanning
> remembered sets, and uses the faster
> HeapRegion::oops_on_card_seq_iterate_careful() (measured).
> HeapRegionDCTOC also depends CMS code which is just bad.
>
> 8177707: Specialize G1RemSet::refine_card for concurrent/during
> safepoint refinement
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8177707
> Webrev: http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
>
> This change temporarily duplicates the code in G1RemSet::refine_card
> for during refinement/gc to allow (initial) specialization of these
> methods for their purpose.
>
> 8177044: Remove _scan_top from HeapRegion
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8177044
> Webrev: http://cr.openjdk.java.net/~tschatzl/8177044/
>
> Removes _scan_top and all its awkward handling by explicitly creating a
> snapshot of the top values of the heap regions. This allows significant
> specialization of the refine_during_gc() method by subsuming all the
> various checks at the beginning into a single one.
>
> This one is responsible for a large part of the improvement of the
> update rs phase.
>
> 8178148: Log more detailed information about scan rs phase
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8178148
> Webrev: http://cr.openjdk.java.net/~tschatzl/8178148/webrev/
>
> Adds some more statistics gathering and logging about what the scan rs
> phase is actually doing, allowing better performance analysis. This
> information will also be used further in the future.
>
> 8175554: Improve G1UpdateRSOrPushRefClosure
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8175554
> Webrev: http://cr.openjdk.java.net/~tschatzl/8175554/webrev/
>
> (This mostly deletes code, so it's still "S" ;))
>
> Finally remove the G1ParPushHeapRSClosure and merge it with
> G1UpdateOrScanRSClosure. You may now notice that
> G1UpdateOrScanRSClosure is pretty similar to G1ParScanClosure - that is
> another issue.
> Also make sure that G1UpdateOrScanRSClosure properly does the humongous
> and "ext" region handling (which is benign so far).
>
> 8178151: Clean up G1RemSet files
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8178151
> Webrev: http://cr.openjdk.java.net/~tschatzl/8178151/webrev/
>
> Clean up after all these changes a little in the G1Remset* files,
> removing obsolete stuff, some renamings, etc.
>
> Thanks,
>    Thomas
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

Kim Barrett
In reply to this post by Kim Barrett
> On Apr 20, 2017, at 11:29 PM, Kim Barrett <[hidden email]> wrote:
>
>> On Apr 11, 2017, at 7:30 AM, Thomas Schatzl <[hidden email]> wrote:
>>
>> Hi all,
>>
>>  I would like you to ask for reviews of a set of related changes that
>> optimize the way G1 updates/scans/refines cards of the remembered set.
>>
>> […]
>> 8177707: Specialize G1RemSet::refine_card for concurrent/during
>> safepoint refinement
>>
>> CR: https://bugs.openjdk.java.net/browse/JDK-8177707
>> Webrev: http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
>>
>> This change temporarily duplicates the code in G1RemSet::refine_card
>> for during refinement/gc to allow (initial) specialization of these
>> methods for their purpose.
>
> src/share/vm/gc/g1/g1RemSet.cpp
> 777   _conc_refine_cards++;
>
> Since this is not in "concurrent" refinement, is it still correct to
> include it in this count?  Maybe the variable name is not right?  If
> so, I'd be okay with a variable rename in a later change if that makes
> management of this stack of changes easier.

And since you are in the neighborhood, consider adding a fix for
https://bugs.openjdk.java.net/browse/JDK-8043505
to this stack of changes.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

Mikael Gerdin
In reply to this post by Kim Barrett
Hi,

On 2017-04-21 03:47, Kim Barrett wrote:

>> On Apr 11, 2017, at 7:30 AM, Thomas Schatzl <[hidden email]> wrote:
>>
>> Hi all,
>>
>>   I would like you to ask for reviews of a set of related changes that
>> optimize the way G1 updates/scans/refines cards of the remembered set.
>>
>> […]
>> Here we go:
>>
>> […]
>> 8162928: Micro-optimizations in scanning the remembered sets
>>
>> CR: https://bugs.openjdk.java.net/browse/JDK-8162928
>> Webrev: http://cr.openjdk.java.net/~tschatzl/8162928/webrev/
>>
>> This change removes the use of HeapRegionDCTOC during scanning
>> remembered sets, and uses the faster
>> HeapRegion::oops_on_card_seq_iterate_careful() (measured).
>> HeapRegionDCTOC also depends CMS code which is just bad.
>
> Looks good.

+1

/Mikael

>
> ------------------------------------------------------------------------------
> src/share/vm/gc/g1/g1RemSet.cpp
>  318     r->oops_on_card_seq_iterate_careful<G1ParPushHeapRSClosure, true>(mr, _push_heap_cl);
>
> This will of course need to be updated for my earlier suggested change
> to reorder the template parameters.  I don't need a new webrev for that.
>
> ------------------------------------------------------------------------------
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

sangheon.kim@oracle.com
In reply to this post by Thomas Schatzl
Review for 8177707.

On 04/11/2017 04:30 AM, Thomas Schatzl wrote:

> Hi all,
>
>    I would like you to ask for reviews of a set of related changes that
> optimize the way G1 updates/scans/refines cards of the remembered set.
>
> As these CRs are highly dependent on each other I would like to have
> them reviewed and pushed together. Another reason is that all of them
> are self-contained, which means that they take one or the other detour
> I want to avoid questions about.
>
> The basic premise is that refinement and Scan RS (and Update RS as it
> is mostly the same) phases are quite slow with G1. There are a lot of
> low-hanging fruits to be picked here, ranging from micro-optimizations,
> fixing "wrong" code sharing, re-using (slow) CMS code, doing stuff in
> non-straightforward ways etc.
>
> These changes try to make it better (tm) ;)
>
> Results are good, around 10%+ faster scan rs time/card, 10%+ faster
> update rs/card, refinement also faster (although it is too hard to
> measure), 8 LOC less (there is more to delete, but that one needs more
> care. Also these changes add some logging :))
>
> Here we go:
>
> 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful() for
> use during concurrent refinement and updating the rset
>
> CR:https://bugs.openjdk.java.net/browse/JDK-8071280
> Webrev:http://cr.openjdk.java.net/~tschatzl/8071280/webrev
>
> As the title indicates, this change allows specializations of
> HeapRegion::oops_on_card_seq_iterate_careful() according to current GC
> phase (concurrent or during gc) to allow to remove some checks.
> This is mostly a preparatory change for the next ones.
>
> 8162928: Micro-optimizations in scanning the remembered sets
>
> CR:https://bugs.openjdk.java.net/browse/JDK-8162928
> Webrev:http://cr.openjdk.java.net/~tschatzl/8162928/webrev/
>
> This change removes the use of HeapRegionDCTOC during scanning
> remembered sets, and uses the faster
> HeapRegion::oops_on_card_seq_iterate_careful() (measured).
> HeapRegionDCTOC also depends CMS code which is just bad.
>
> 8177707: Specialize G1RemSet::refine_card for concurrent/during
> safepoint refinement
>
> CR:https://bugs.openjdk.java.net/browse/JDK-8177707
> Webrev:http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
>
> This change temporarily duplicates the code in G1RemSet::refine_card
> for during refinement/gc to allow (initial) specialization of these
> methods for their purpose.
Looks good, just minor comments.

/src/share/vm/gc/g1/g1RemSet.hpp
152   // Refine the card corresponding to "card_ptr". Returns if the
given card contains
Second sentence seems incomplete. Returns "true" if the given card contains?

  154   bool refine_card_during_gc(jbyte* card_ptr,
Do we really need to rename refine_card()? Couldn't be refine_card() and
refine_card_concurrently()?

--------------------------------
src/share/vm/gc/g1/g1RemSet.cpp
  549 void G1RemSet::refine_card_concurrently(jbyte* card_ptr,
  550                            uint worker_i) {
line 550 needs alignment update.

Thanks,
Sangheon


> 8177044: Remove _scan_top from HeapRegion
>
> CR:https://bugs.openjdk.java.net/browse/JDK-8177044
> Webrev:http://cr.openjdk.java.net/~tschatzl/8177044/
>
> Removes _scan_top and all its awkward handling by explicitly creating a
> snapshot of the top values of the heap regions. This allows significant
> specialization of the refine_during_gc() method by subsuming all the
> various checks at the beginning into a single one.
>
> This one is responsible for a large part of the improvement of the
> update rs phase.
>
> 8178148: Log more detailed information about scan rs phase
>
> CR:https://bugs.openjdk.java.net/browse/JDK-8178148
> Webrev:http://cr.openjdk.java.net/~tschatzl/8178148/webrev/
>
> Adds some more statistics gathering and logging about what the scan rs
> phase is actually doing, allowing better performance analysis. This
> information will also be used further in the future.
>
> 8175554: Improve G1UpdateRSOrPushRefClosure
>
> CR:https://bugs.openjdk.java.net/browse/JDK-8175554
> Webrev:http://cr.openjdk.java.net/~tschatzl/8175554/webrev/
>
> (This mostly deletes code, so it's still "S" ;))
>
> Finally remove the G1ParPushHeapRSClosure and merge it with
> G1UpdateOrScanRSClosure. You may now notice that
> G1UpdateOrScanRSClosure is pretty similar to G1ParScanClosure - that is
> another issue.
> Also make sure that G1UpdateOrScanRSClosure properly does the humongous
> and "ext" region handling (which is benign so far).
>
> 8178151: Clean up G1RemSet files
>
> CR:https://bugs.openjdk.java.net/browse/JDK-8178151
> Webrev:http://cr.openjdk.java.net/~tschatzl/8178151/webrev/
>
> Clean up after all these changes a little in the G1Remset* files,
> removing obsolete stuff, some renamings, etc.
>
> Thanks,
>    Thomas
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful for use during concurrent refinement and updating the rset

Thomas Schatzl
In reply to this post by Kim Barrett
Hi Kim,

  thanks for your review.

On Wed, 2017-04-19 at 20:06 -0400, Kim Barrett wrote:

> >
> > On Apr 11, 2017, at 7:30 AM, Thomas Schatzl <thomas.schatzl@oracle.
> > com> wrote:
> > 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful()
> > for
> > use during concurrent refinement and updating the rset
> >
> > CR: https://bugs.openjdk.java.net/browse/JDK-8071280
> > Webrev: http://cr.openjdk.java.net/~tschatzl/8071280/webrev
> >
> > As the title indicates, this change allows specializations of
> > HeapRegion::oops_on_card_seq_iterate_careful() according to current
> > GC
> > phase (concurrent or during gc) to allow to remove some checks.
> > This is mostly a preparatory change for the next ones.
> Only this changeset.  I’ll be sending out comments on others later,
> separate messages for each.
>
> -------------------------------------------------------------------
> ----------- 
> src/share/vm/gc/g1/heapRegion.inline.hpp
>  149   assert(ClassUnloadingWithConcurrentMark,
>
> Is this assertion really correct?  There is a similar assertion in
> block_size, but it is protected by a call to block_is_obj.  I'm not
> seeing similar protection for this one.  Maybe I'm missing someting?

If ClassUnloadingWithConcurrentMark is false, this method will not be
called, and this is what the assert checks.
HeapRegion::is_obj_dead_with_size() then always uses the obj->size()
path.

> -------------------------------------------------------------------
> -----------
> src/share/vm/gc/g1/g1RemSet.cpp
>  728   }
>
> Close brace is now mis-indented.

Fixed.

> -------------------------------------------------------------------
> ----------- 
> src/share/vm/gc/g1/heapRegion.hpp
>  329   template <class Closure, bool is_gc_active>
>  330   inline bool do_oops_on_card_in_humongous(MemRegion mr,
>
> and
>
>  682   template <class Closure, bool is_gc_active>
>  683   inline bool oops_on_card_seq_iterate_careful(MemRegion mr,
> Closure* cl);
>
> It would be better to have the is_gc_active template parameter first.
> That would allow the Closure parameter to be deduced in calls, e.g.
> instead of
>
>     card_processed = r-
> >oops_on_card_seq_iterate_careful<G1UpdateRSOrPushRefOopClosure,
> true>(dirty_region, &update_rs_oop_cl);
>
> one would use
>
>     card_processed = r-
> >oops_on_card_seq_iterate_careful<true>(dirty_region,
> &update_rs_oop_cl);
>
> with Closure being deduced from the type of the argument in the call.

Fixed.

New webrevs:
http://cr.openjdk.java.net/~tschatzl/8071280/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8071280/webrev.1/ (full)

Thanks,
  Thomas

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8162928: Micro-optimizations in scanning the remembered sets

Thomas Schatzl
In reply to this post by Mikael Gerdin
Hi Kim, Mikael,

On Wed, 2017-05-03 at 15:45 +0200, Mikael Gerdin wrote:

> Hi,
>
> On 2017-04-21 03:47, Kim Barrett wrote:
> >
> > >
> > > On Apr 11, 2017, at 7:30 AM, Thomas Schatzl <thomas.schatzl@oracl
> > > e.com> wrote:
> > >
> > > Hi all,
> > >
> > >   I would like you to ask for reviews of a set of related changes
> > > that
> > > optimize the way G1 updates/scans/refines cards of the remembered
> > > set.
> > >
> > > […]
> > > Here we go:
> > >
> > > […]
> > > 8162928: Micro-optimizations in scanning the remembered sets
> > >
> > > CR: https://bugs.openjdk.java.net/browse/JDK-8162928
> > > Webrev: http://cr.openjdk.java.net/~tschatzl/8162928/webrev/
> > >
> > > This change removes the use of HeapRegionDCTOC during scanning
> > > remembered sets, and uses the faster
> > > HeapRegion::oops_on_card_seq_iterate_careful() (measured).
> > > HeapRegionDCTOC also depends CMS code which is just bad.
> > Looks good.
> +1

 thanks for your reviews.

For reference, the webrev I am going to push is as follows, as a change
in the previous CR change the signature of
oops_on_card_seq_iterate_careful:
http://cr.openjdk.java.net/~tschatzl/8162928/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8162928/webrev.1/ (full)

Thanks,
  Thomas
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8177707: Specialize G1RemSet::refine_card for concurrent/during safepoint refinement

Thomas Schatzl
In reply to this post by sangheon.kim@oracle.com
Hi Sangheon,

  thanks for your review.

On Thu, 2017-05-04 at 15:09 -0700, sangheon wrote:
> Review for 8177707.
>
> On 04/11/2017 04:30 AM, Thomas Schatzl wrote:
[...]

> >
> > 8177707: Specialize G1RemSet::refine_card for concurrent/during
> > safepoint refinement
> >
> > CR:https://bugs.openjdk.java.net/browse/JDK-8177707
> > Webrev:http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
> >
> > This change temporarily duplicates the code in
> > G1RemSet::refine_card
> > for during refinement/gc to allow (initial) specialization of these
> > methods for their purpose.
> Looks good, just minor comments.
>
> /src/share/vm/gc/g1/g1RemSet.hpp
> 152   // Refine the card corresponding to "card_ptr". Returns if the 
> given card contains Second sentence seems incomplete. Returns "true"
> if the given card contains?

Fixed.

>
>   154   bool refine_card_during_gc(jbyte* card_ptr,
> Do we really need to rename refine_card()? Couldn't be refine_card()
> and refine_card_concurrently()?

I prefer to have the verbose names.

> --------------------------------
> src/share/vm/gc/g1/g1RemSet.cpp
>   549 void G1RemSet::refine_card_concurrently(jbyte* card_ptr,
>   550                            uint worker_i) {
> line 550 needs alignment update.
>

Fixed.

New webrevs:
http://cr.openjdk.java.net/~tschatzl/8177707/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8177707/webrev.1/ (full)

Thanks,
  Thomas

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8177707: Specialize G1RemSet::refine_card for concurrent/during safepoint refinement

Thomas Schatzl
In reply to this post by Kim Barrett
Hi Kim,

  thanks for your review.

On Thu, 2017-04-20 at 23:29 -0400, Kim Barrett wrote:

> >
> > On Apr 11, 2017, at 7:30 AM, Thomas Schatzl <thomas.schatzl@oracle.
> > com> wrote:
> >
> > Hi all,
> >
> >   I would like you to ask for reviews of a set of related changes
> > that
> > optimize the way G1 updates/scans/refines cards of the remembered
> > set.
> >
> > […]
> > 8177707: Specialize G1RemSet::refine_card for concurrent/during
> > safepoint refinement
> >
> > CR: https://bugs.openjdk.java.net/browse/JDK-8177707
> > Webrev: http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
> >
> > This change temporarily duplicates the code in
> > G1RemSet::refine_card
> > for during refinement/gc to allow (initial) specialization of these
> > methods for their purpose.
> -------------------------------------------------------------------
> ----------- 
> src/share/vm/gc/g1/g1RemSet.cpp
>  588   // While we are processing RSet buffers during the collection,
> we
> ...
>  598   assert(!r->in_collection_set(), "Concurrent refinement should
> not encounter cards in the collection set.");
>
> The comment starting at line 588 seems like it needs updating to
> account for line 598 being changed from an "if" to an "assert".
>
> However, I'm not sure this change change to an assert is correct,
> because of the possibility of stale cards.

Fixed. Thanks for pointing this out, you are right.

>
> -------------------------------------------------------------------
> ----------- 
> src/share/vm/gc/g1/g1RemSet.cpp 
>  636   HeapWord* scan_limit = r->top();
>
> I think the commentary about this initialization is important and
> ought not be deleted.
>
> Similarly for the use of scan_top() in the gc_active case.

Fixed.

>
> -------------------------------------------------------------------
> -----------
> src/share/vm/gc/g1/g1RemSet.cpp 
>  688 bool G1RemSet::refine_card_during_gc(jbyte* card_ptr,
>
> I sure hope a lot of the near duplication with the concurrent version
> is going to go away in one of the later changes in this stack.

Actually the next change in this stack cleans this up to a large degree
(8177044: Remove _scan_top from HeapRegion), but also others are going
to reduce the code and code duplication in refine_card_during_gc().

> -------------------------------------------------------------------
> -----------  
> src/share/vm/gc/g1/g1RemSet.cpp 
>  777   _conc_refine_cards++;
>
> Since this is not in "concurrent" refinement, is it still correct to
> include it in this count?  Maybe the variable name is not right?  If
> so, I'd be okay with a variable rename in a later change if that
> makes management of this stack of changes easier.

The variable name has actually never been "right". I created JDK-
8179677 to avoid disturbing the change stack. I will post an RFR for
that later.

>
> -------------------------------------------------------------------
> -----------  
> src/share/vm/gc/g1/g1OopClosures.inline.hpp
>  114 inline static void check_obj_during_refinement(T* p, oop const
> obj) {
>
> Just "inline" is sufficient, no "static" needed.
>
> -------------------------------------------------------------------
> ----------- 

Fixed.

New webrevs:
http://cr.openjdk.java.net/~tschatzl/8177707/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8177707/webrev.1/ (full)

Thanks,
  Thomas

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful() for use during concurrent refinement and updating the rset

Thomas Schatzl
In reply to this post by sangheon.kim@oracle.com
Hi Sangheon,

  thanks for your review.

On Tue, 2017-05-02 at 15:07 -0700, sangheon wrote:

> Hi Thomas,
>
> On 04/11/2017 04:30 AM, Thomas Schatzl wrote:
> >
> > Hi all,
> >
> >    I would like you to ask for reviews of a set of related changes
> > that optimize the way G1 updates/scans/refines cards of the
> > remembered set.
> >
> >
> > [...]
> > Here we go:
> >
> > 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful()
> > for use during concurrent refinement and updating the rset
> >
> > CR: https://bugs.openjdk.java.net/browse/JDK-8071280
> > Webrev: http://cr.openjdk.java.net/~tschatzl/8071280/webrev
> >
> > As the title indicates, this change allows specializations of
> > HeapRegion::oops_on_card_seq_iterate_careful() according to current
> > GC
> > phase (concurrent or during gc) to allow to remove some checks.
> > This is mostly a preparatory change for the next ones.
> 8071280, seems good to me.
>
> src/share/vm/gc/g1/heapRegion.inline.hpp
>   149   assert(ClassUnloadingWithConcurrentMark,
>
> I have same question as Kim about this assertion.
> In addition, block_size_during_gc() and block_size() are very similar
> except block_size() could return earlier.
> Could you give me some explanation for this?

As mentioned in Kim's reply: "If ClassUnloadingWithConcurrentMark is
false, this method will not be called, and this is what the assert
checks. HeapRegion::is_obj_dead_with_size() then always uses the obj-
>size() path."

Block_size_during_gc() is a very aggressive specialization of
block_size(), so of course they ultimately do the same thing. The
difference is that during GC a lot of cases that block_size() needs to
check for always evaluate to a particular value.

The other important optimization that block_size_during_gc() does is
that we pass the bitmap to look at, as the triple indirection (load
G1CollectedHeap*, load G1ConcurrentMark*, load G1CMBitMapR0*) to find
it has shown to be very expensive.

The compiler is unable to see all this.

The reason why the two methods look similar is that
block_size_during_gc() changes all ifs into asserts checking that it is
called only in the right circumstances. I.e. that all assumptions that
we can make during GC are actually true.

Looking at it now (it has been a long way to get to this version),
maybe block_size() could just call block_size_during_gc() at the end?

Thanks,
  Thomas

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280, 8162928, 8177707, 8177044, 8178148, 8175554, 8178151: Remembered set update/scan improvements

Thomas Schatzl
In reply to this post by Thomas Schatzl
Hi all,

  recent reviews have made changes necessary to parts of the changeset
chain.

Here is a list of links to updated webrevs. Since they have apparently
not been reviewed yet, I simply overwrote the old webrevs.

JDK-8177044: Remove _scan_top from HeapRegion
http://cr.openjdk.java.net/~tschatzl/8177044/webrev/

JDK-8178148: Log more detailed information about scan rs phase
http://cr.openjdk.java.net/~tschatzl/8178148/webrev/

JDK-8175554: Improve G1UpdateRSOrPushRefClosure
http://cr.openjdk.java.net/~tschatzl/8175554/webrev/

JDK-8178151: Clean up G1RemSet files
http://cr.openjdk.java.net/~tschatzl/8178151/webrev/

Please consider changing the subject of the email if you are reviewing
one specific webrev, as this seems helpful to quickly find the context
but keep the reviews together in the email thread.

Thanks,
  Thomas

On Tue, 2017-04-11 at 13:30 +0200, Thomas Schatzl wrote:

> Hi all,
>
>   I would like you to ask for reviews of a set of related changes
> that optimize the way G1 updates/scans/refines cards of the
> remembered set.
>
> As these CRs are highly dependent on each other I would like to have
> them reviewed and pushed together. Another reason is that all of them
> are self-contained, which means that they take one or the other
> detour I want to avoid questions about.
>
> The basic premise is that refinement and Scan RS (and Update RS as it
> is mostly the same) phases are quite slow with G1. There are a lot of
> low-hanging fruits to be picked here, ranging from micro-
> optimizations, fixing "wrong" code sharing, re-using (slow) CMS code,
> doing stuff in
> non-straightforward ways etc.
>
> These changes try to make it better (tm) ;)
>
> Results are good, around 10%+ faster scan rs time/card, 10%+ faster
> update rs/card, refinement also faster (although it is too hard to
> measure), 8 LOC less (there is more to delete, but that one needs
> more care. Also these changes add some logging :))
>
> Here we go:
>
> 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful()
> for
> use during concurrent refinement and updating the rset
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8071280
> Webrev: http://cr.openjdk.java.net/~tschatzl/8071280/webrev
>
> As the title indicates, this change allows specializations of
> HeapRegion::oops_on_card_seq_iterate_careful() according to current
> GC
> phase (concurrent or during gc) to allow to remove some checks.
> This is mostly a preparatory change for the next ones.
>
> 8162928: Micro-optimizations in scanning the remembered sets
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8162928
> Webrev: http://cr.openjdk.java.net/~tschatzl/8162928/webrev/
>
> This change removes the use of HeapRegionDCTOC during scanning
> remembered sets, and uses the faster
> HeapRegion::oops_on_card_seq_iterate_careful() (measured).
> HeapRegionDCTOC also depends CMS code which is just bad.
>
> 8177707: Specialize G1RemSet::refine_card for concurrent/during
> safepoint refinement
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8177707
> Webrev: http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
>
> This change temporarily duplicates the code in G1RemSet::refine_card
> for during refinement/gc to allow (initial) specialization of these
> methods for their purpose.
>
> 8177044: Remove _scan_top from HeapRegion
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8177044
> Webrev: http://cr.openjdk.java.net/~tschatzl/8177044/
>
> Removes _scan_top and all its awkward handling by explicitly creating
> a
> snapshot of the top values of the heap regions. This allows
> significant
> specialization of the refine_during_gc() method by subsuming all the
> various checks at the beginning into a single one.
>
> This one is responsible for a large part of the improvement of the
> update rs phase.
>
> 8178148: Log more detailed information about scan rs phase
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8178148
> Webrev: http://cr.openjdk.java.net/~tschatzl/8178148/webrev/
>
> Adds some more statistics gathering and logging about what the scan
> rs
> phase is actually doing, allowing better performance analysis. This
> information will also be used further in the future.
>
> 8175554: Improve G1UpdateRSOrPushRefClosure
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8175554
> Webrev: http://cr.openjdk.java.net/~tschatzl/8175554/webrev/
>
> (This mostly deletes code, so it's still "S" ;))
>
> Finally remove the G1ParPushHeapRSClosure and merge it with
> G1UpdateOrScanRSClosure. You may now notice that
> G1UpdateOrScanRSClosure is pretty similar to G1ParScanClosure - that
> is
> another issue.
> Also make sure that G1UpdateOrScanRSClosure properly does the
> humongous
> and "ext" region handling (which is benign so far).
>
> 8178151: Clean up G1RemSet files
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8178151
> Webrev: http://cr.openjdk.java.net/~tschatzl/8178151/webrev/
>
> Clean up after all these changes a little in the G1Remset* files,
> removing obsolete stuff, some renamings, etc. 
>
> Thanks,
>   Thomas
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8177707: Specialize G1RemSet::refine_card for concurrent/during safepoint refinement

sangheon.kim@oracle.com
In reply to this post by Thomas Schatzl


On 05/05/2017 04:49 AM, Thomas Schatzl wrote:

> Hi Sangheon,
>
>    thanks for your review.
>
> On Thu, 2017-05-04 at 15:09 -0700, sangheon wrote:
>> Review for 8177707.
>>
>> On 04/11/2017 04:30 AM, Thomas Schatzl wrote:
> [...]
>>> 8177707: Specialize G1RemSet::refine_card for concurrent/during
>>> safepoint refinement
>>>
>>> CR:https://bugs.openjdk.java.net/browse/JDK-8177707
>>> Webrev:http://cr.openjdk.java.net/~tschatzl/8177707/webrev/
>>>
>>> This change temporarily duplicates the code in
>>> G1RemSet::refine_card
>>> for during refinement/gc to allow (initial) specialization of these
>>> methods for their purpose.
>> Looks good, just minor comments.
>>
>> /src/share/vm/gc/g1/g1RemSet.hpp
>> 152   // Refine the card corresponding to "card_ptr". Returns if the
>> given card contains Second sentence seems incomplete. Returns "true"
>> if the given card contains?
> Fixed.
>
>>    154   bool refine_card_during_gc(jbyte* card_ptr,
>> Do we really need to rename refine_card()? Couldn't be refine_card()
>> and refine_card_concurrently()?
> I prefer to have the verbose names.
OK.

>
>> --------------------------------
>> src/share/vm/gc/g1/g1RemSet.cpp
>>    549 void G1RemSet::refine_card_concurrently(jbyte* card_ptr,
>>    550                            uint worker_i) {
>> line 550 needs alignment update.
>>
> Fixed.
>
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8177707/webrev.0_to_1/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8177707/webrev.1/ (full)
webrev.1 seems good to me.

Thanks,
Sangheon


>
> Thanks,
>    Thomas
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful() for use during concurrent refinement and updating the rset

sangheon.kim@oracle.com
In reply to this post by Thomas Schatzl
Hi Thomas,

On 05/05/2017 05:03 AM, Thomas Schatzl wrote:

> Hi Sangheon,
>
>    thanks for your review.
>
> On Tue, 2017-05-02 at 15:07 -0700, sangheon wrote:
>> Hi Thomas,
>>
>> On 04/11/2017 04:30 AM, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>     I would like you to ask for reviews of a set of related changes
>>> that optimize the way G1 updates/scans/refines cards of the
>>> remembered set.
>>>
>>>
>>> [...]
>>> Here we go:
>>>
>>> 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful()
>>> for use during concurrent refinement and updating the rset
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8071280
>>> Webrev: http://cr.openjdk.java.net/~tschatzl/8071280/webrev
>>>
>>> As the title indicates, this change allows specializations of
>>> HeapRegion::oops_on_card_seq_iterate_careful() according to current
>>> GC
>>> phase (concurrent or during gc) to allow to remove some checks.
>>> This is mostly a preparatory change for the next ones.
>> 8071280, seems good to me.
>>
>> src/share/vm/gc/g1/heapRegion.inline.hpp
>>    149   assert(ClassUnloadingWithConcurrentMark,
>>
>> I have same question as Kim about this assertion.
>> In addition, block_size_during_gc() and block_size() are very similar
>> except block_size() could return earlier.
>> Could you give me some explanation for this?
> As mentioned in Kim's reply: "If ClassUnloadingWithConcurrentMark is
> false, this method will not be called, and this is what the assert
> checks. HeapRegion::is_obj_dead_with_size() then always uses the obj-
>> size() path."
> Block_size_during_gc() is a very aggressive specialization of
> block_size(), so of course they ultimately do the same thing. The
> difference is that during GC a lot of cases that block_size() needs to
> check for always evaluate to a particular value.
>
> The other important optimization that block_size_during_gc() does is
> that we pass the bitmap to look at, as the triple indirection (load
> G1CollectedHeap*, load G1ConcurrentMark*, load G1CMBitMapR0*) to find
> it has shown to be very expensive.
>
> The compiler is unable to see all this.
>
> The reason why the two methods look similar is that
> block_size_during_gc() changes all ifs into asserts checking that it is
> called only in the right circumstances. I.e. that all assumptions that
> we can make during GC are actually true.
Thank you very much for the explanation!

>
> Looking at it now (it has been a long way to get to this version),
> maybe block_size() could just call block_size_during_gc() at the end?
Doesn't hurt your goal with above idea?
Current version seems good to me but if you are considering a change
adding inline function with common stuffs could be one of idea.

Thanks,
Sangheon


>
> Thanks,
>    Thomas
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8071280: Specialize HeapRegion::oops_on_card_seq_iterate_careful for use during concurrent refinement and updating the rset

Kim Barrett
In reply to this post by Thomas Schatzl
> On May 5, 2017, at 7:30 AM, Thomas Schatzl <[hidden email]> wrote:
>
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8071280/webrev.0_to_1/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8071280/webrev.1/ (full)

Looks good.

Do the tails of block_size and block_size_during_gc remain similar through
all of these cleanups?  If so, merging them into a common helper would be
nice.  That can be a further cleanup if they do remain similar.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8162928: Micro-optimizations in scanning the remembered sets

Kim Barrett
In reply to this post by Thomas Schatzl
> On May 5, 2017, at 7:39 AM, Thomas Schatzl <[hidden email]> wrote:
>
>  thanks for your reviews.
>
> For reference, the webrev I am going to push is as follows, as a change
> in the previous CR change the signature of
> oops_on_card_seq_iterate_careful:
> http://cr.openjdk.java.net/~tschatzl/8162928/webrev.0_to_1/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8162928/webrev.1/ (full)
>
> Thanks,
>   Thomas

Still looks good.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR (7xS): 8177707: Specialize G1RemSet::refine_card for concurrent/during safepoint refinement

Kim Barrett
In reply to this post by Thomas Schatzl
> On May 5, 2017, at 7:52 AM, Thomas Schatzl <[hidden email]> wrote:
>
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8177707/webrev.0_to_1/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8177707/webrev.1/ (full)
>
> Thanks,
>   Thomas

Looks good.

123
Loading...