RFR (M): 8129558: Coalesce dead objects during removal of self-forwarded pointers

[Tony Printezis]

Thomas,

One nit and one question:

 144   // Fill the memory area from start to end with a dead object, and update the BOT
 145   // and the mark bitmap.
 146   void zap_dead_objects(HeapWord* start, HeapWord* end) {

the nit: The comment should now be “with dead objects” (and I’d prefer "with filler objects”) instead of “with dead object” given that fill_with_objects(), called from zap_dead_objects(), might place up to two objects in the area (given your previous change, right?).

the (related) question: Do you set the BOT for [start, end) assuming one filler object but in reality [start, end) might have two objects?  I looked at the code for cross_threshold() / BOT::alloc_block() and it looks as if they set up the BOT as if [start, end) is covered by a single object (but I could have misread the code). This will result in some (most likely very rare) cases where BOT look-ups will do more work than they should. Should you do fill_with_objects() first, then iterate over the filler objects, and update the BOT for each?

Tony

On June 25, 2015 at 11:30:45 AM, Thomas Schatzl (thomas.schatzl at oracle.com) wrote:

Hi Mikael,  

thanks for the review...  

On Thu, 2015-06-25 at 15:20 +0200, Mikael Gerdin wrote:  
> Hi Thomas,  
>  
> On 2015-06-25 10:07, Thomas Schatzl wrote:  
> > Hi all,  
> >  
> > can I have reviews for the first change in a set of changes that speed  
> > up evacuation failure?  
> >[...]  
> > CR:  
> > https://bugs.openjdk.java.net/browse/JDK-8129558  
> >  
> > Webrev:  
> > http://cr.openjdk.java.net/~tschatzl/8129558/webrev  
>  
> Can you make zap_dead_objects a member function of  
> RemoveSelfForwardPtrObjClosure,  
> then it will already have access to _g1 and _cm  
> and then have remove_self_forward_ptr_by_...  
> call a function on rspc to clear the last part of the region?  
> That way the zapping functionality is nicely encapsulated and  
> last_object_end doesn't need to be exposed.  
>  
> Otherwise I think the change looks good and I think it's a nice step  
> towards cleaning up this code.  

New webrevs at  
http://cr.openjdk.java.net/~tschatzl/8129558/webrev.1 (complete)  
http://cr.openjdk.java.net/~tschatzl/8129558/webrev.0_to_1 (diff)  

Fixed according to your suggestions I hope - it still needs an extra  
method (zap_remainer()) to hide the last_object_end.  

Thanks,  
Thomas  


-----

Tony Printezis | JVM/GC Engineer / VM Team | Twitter

@TonyPrintezis
tprintezis at twitter.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150625/fd399edb/attachment.htm>