<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <br>
    <div class="moz-cite-prefix">On 3/18/14 6:30 AM, Bengt Rutisson
      wrote:<br>
    </div>
    <blockquote cite="mid:53284A7E.9060106@oracle.com" type="cite">
      <br>
      Hi Jon,
      <br>
      <br>
      On 2014-03-17 05:40, Jon Masamitsu wrote:
      <br>
      <blockquote type="cite">8031323: Optionally align objects copied
        to survivor spaces
        <br>
        <br>
        Add the option to specify an alignment for allocations into the
        <br>
        survivor spaces.  Survivor space allocations will overflow if
        the
        <br>
        survivor spaces have not been increased.   The expected use
        <br>
        is to align survivor space allocations to cache line sizes.
        <br>
        <br>
        <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/8031323/webrev.00/">http://cr.openjdk.java.net/~jmasa/8031323/webrev.00/</a>
        <br>
      </blockquote>
      <br>
      For G1 it looks to me like we are aligning objects in the old
      regions as well. I thought the intent was to only align in the
      survivor regions. Am I missing something?
      <br>
      <br>
      Note that it is not enough to check the alloc purpose for G1 since
      G1CollectedHeap::par_allocate_during_gc() only takes the purpose
      as a hint and may still return buffers from other regions than
      expected for a purpose. But I guess that in this case it is not
      too important to get it exactly right, so it may be enough to
      check the alloc purpose value.
      <br>
      <br>
      <br>
      The code in PSYoungPromotionLAB::allcoate(),
      ParGCAllocBuffer::allocate_aligned() and
      ContiguousSpace::allocate_aligned() is very similar. Is there a
      way to consolidate these methods?
      <br>
    </blockquote>
    <br>
    To consolidate this code I've added
    CollectedHeap::allocate_inc_top()<br>
    <br>
<a class="moz-txt-link-freetext" href="http://javaweb.us.oracle.com/net/arches-sunw.us.oracle.com/export/home1/java/jdk9-gc-8031323/webrev_inc_top/webrev/src/share/vm/gc_interface/collectedHeap.inline.hpp.frames.html">http://javaweb.us.oracle.com/net/arches-sunw.us.oracle.com/export/home1/java/jdk9-gc-8031323/webrev_inc_top/webrev/src/share/vm/gc_interface/collectedHeap.inline.hpp.frames.html</a><br>
    <br>
    Note "top" passed by reference.<br>
    <br>
    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
    <pre><span class="new"> 278 inline HeapWord* CollectedHeap::allocate_inc_top(HeapWord* &top,</span>
<span class="new"> 279                                                 HeapWord* end,</span>
<span class="new"> 280                                                 size_t word_size) {</span>
<span class="new"> 281   HeapWord* res = top;</span>
<span class="new"> 282   if (pointer_delta(end, top) >= word_size) {</span>
<span class="new"> 283     top = top + word_size;</span>
<span class="new"> 284     assert(is_ptr_aligned(res, HeapWordSize) &&</span>
<span class="new"> 285       is_ptr_aligned(top, HeapWordSize), "checking alignment");</span>
<span class="new"> 286     return res;</span>
<span class="new"> 287   } else {</span>
<span class="new"> 288     return NULL;</span>
 289   }</pre>
    and use it in places such as<br>
    <br>
<a class="moz-txt-link-freetext" href="http://javaweb.us.oracle.com/net/arches-sunw.us.oracle.com/export/home1/java/jdk9-gc-8031323/webrev_inc_top/webrev/src/share/vm/gc_implementation/parallelScavenge/psPromotionLAB.hpp.udiff.html">http://javaweb.us.oracle.com/net/arches-sunw.us.oracle.com/export/home1/java/jdk9-gc-8031323/webrev_inc_top/webrev/src/share/vm/gc_implementation/parallelScavenge/psPromotionLAB.hpp.udiff.html</a><br>
    <br>
    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
    <pre><span class="removed">-    HeapWord* new_top = obj + size;</span>
<span class="removed">-    // The 'new_top>obj' check is needed to detect overflow of obj+size.</span>
<span class="removed">-    if (new_top > obj && new_top <= end()) {</span>
<span class="removed">-      set_top(new_top);</span>
<span class="removed">-      assert(is_object_aligned((intptr_t)obj) && is_object_aligned((intptr_t)new_top),</span>
<span class="removed">-             "checking alignment");</span>
<span class="removed">-      return obj;</span>
<span class="removed">-    }</span>
     set_top(obj);
<span class="removed">-    return NULL;</span>
<span class="new">+    return CollectedHeap::allocate_inc_top(_top, end(), size);


</span></pre>
    Does this do what you want in an OK way?<br>
    <br>
    Jon<br>
    <br>
    <blockquote cite="mid:53284A7E.9060106@oracle.com" type="cite">
      <br>
      <br>
      The code in CollectedHeap::align_allocation_or_fail() looks quite
      different from other places in the code base where we do padding.
      What do you think about doing something like this instead?
      <br>
      <br>
      inline HeapWord* CollectedHeap::align_allocation_or_fail(HeapWord*
      addr, HeapWord* end, unsigned short alignment_in_bytes) {
      <br>
        if (alignment_in_bytes <= ObjectAlignmentInBytes) {
      <br>
          return addr;
      <br>
        }
      <br>
      <br>
        assert(is_ptr_aligned(addr, HeapWordSize), "");
      <br>
        assert(is_size_aligned(alignment_in_bytes, HeapWordSize), "");
      <br>
      <br>
        HeapWord* new_addr = (HeapWord*) align_pointer_up(addr,
      alignment_in_bytes);
      <br>
        size_t padding = pointer_delta(new_addr, addr);
      <br>
      <br>
        if (padding == 0) {
      <br>
          return addr;
      <br>
        }
      <br>
      <br>
        if (padding < CollectedHeap::min_fill_size()) {
      <br>
          padding += alignment_in_bytes / HeapWordSize;
      <br>
          assert(padding >= CollectedHeap::min_fill_size(), "");
      <br>
          new_addr = addr + padding;
      <br>
        }
      <br>
      <br>
        if(new_addr > addr && new_addr < end) {
      <br>
          CollectedHeap::fill_with_object(addr, padding);
      <br>
          return new_addr;
      <br>
        } else {
      <br>
          return NULL;
      <br>
        }
      <br>
      }
      <br>
      <br>
      Thanks,
      <br>
      Bengt
      <br>
      <br>
      <blockquote type="cite">
        <br>
        Jon
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>