<html><head><style>body{font-family:Helvetica,Arial;font-size:13px}</style></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div id="bloop_customfont" style="font-family:Helvetica,Arial;font-size:13px; color: rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Thomas,</div><div id="bloop_customfont" style="font-family:Helvetica,Arial;font-size:13px; color: rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br></div><div id="bloop_customfont" style="font-family:Helvetica,Arial;font-size:13px; color: rgba(0,0,0,1.0); margin: 0px; line-height: auto;">One nit and one question:</div><div id="bloop_customfont" style="font-family:Helvetica,Arial;font-size:13px; color: rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br></div><div id="bloop_customfont" style="font-family:Helvetica,Arial;font-size:13px; color: rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><pre><span class="new" style="color: blue; font-weight: bold;"> 144   // Fill the memory area from start to end with a dead object, and update the BOT</span>

<span class="new" style="color: blue; font-weight: bold;"> 145   // and the mark bitmap.</span>

<span class="new" style="color: blue; font-weight: bold;"> 146   void zap_dead_objects(HeapWord* start, HeapWord* end) {</span>

</pre><div><span class="new" style="color: blue; font-weight: bold;"><br></span></div></div> <div>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?).</div><div><br></div><div>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?</div><div><br></div><div>Tony</div><br><p class="airmail_on" style="color:#000;">On June 25, 2015 at 11:30:45 AM, Thomas Schatzl (<a href="mailto:thomas.schatzl@oracle.com">thomas.schatzl@oracle.com</a>) wrote:</p> <blockquote type="cite" class="clean_bq"><span><div><div></div><div>Hi Mikael,

<br>

thanks for the review...

<br>

<br>On Thu, 2015-06-25 at 15:20 +0200, Mikael Gerdin wrote:

<br>> Hi Thomas,

<br>>  

<br>> On 2015-06-25 10:07, Thomas Schatzl wrote:

<br>> > Hi all,

<br>> >

<br>> >    can I have reviews for the first change in a set of changes that speed

<br>> > up evacuation failure?

<br>> >[...]

<br>> > CR:

<br>> > https://bugs.openjdk.java.net/browse/JDK-8129558

<br>> >

<br>> > Webrev:

<br>> > http://cr.openjdk.java.net/~tschatzl/8129558/webrev

<br>>  

<br>> Can you make zap_dead_objects a member function of  

<br>> RemoveSelfForwardPtrObjClosure,

<br>> 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?

<br>> That way the zapping functionality is nicely encapsulated and

> last_object_end doesn't need to be exposed.

<br>>  

<br>> Otherwise I think the change looks good and I think it's a nice step  

> towards cleaning up this code.

<br>

<br>New webrevs at

<br>http://cr.openjdk.java.net/~tschatzl/8129558/webrev.1 (complete)

<br>http://cr.openjdk.java.net/~tschatzl/8129558/webrev.0_to_1 (diff)

<br>

<br>Fixed according to your suggestions I hope - it still needs an extra

method (zap_remainer()) to hide the last_object_end.

<br>

<br>Thanks,

<br>  Thomas

<br>

<br>

<br></div></div></span></blockquote> <div id="bloop_sign_1435248973918908160" class="bloop_sign"><div style="font-family:helvetica,arial;font-size:13px"><div>-----</div><div><br></div><div>Tony Printezis | JVM/GC Engineer / VM Team | Twitter</div><div><br></div><div>@TonyPrintezis</div><div><a href="mailto:tprintezis@twitter.com">tprintezis@twitter.com</a></div><div><br></div></div></div></body></html>