<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 11, 2014 at 6:49 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
On 11/07/2014 19:41, Alexander Kornienko wrote:<div class=""><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Fri, Jul 11, 2014 at 6:35 PM, Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a> <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>> wrote:<br>
<br>
<br>
  On 11/07/2014 19:26, Alexander Kornienko wrote:<br>
<br>
    I wonder whether I missed the review thread for this patch or<br>
    this kind of patch is generally accepted to be committed<br>
    without any review?<br>
<br>
<br>
  I'm OK with this change, but any review is welcome. This is a<br>
  supporting commit towards fixing the really old FIXMEs related to<br>
  scratch buffer overallocation.<br>
<br>
<br>
Can you explain why changing growth from exponential to linear is fine here? For large sizes it will lead to O(size) write time.<br>
</blockquote>
<br></div>
There's no change to linear growth. See the implementation of reserve() which calls into SmallVector's grow():<br>
<br>
 /// grow - Grow the allocated memory (without initializing new<br>
 /// elements), doubling the size of the allocated memory.<br>
 /// Guarantees space for at least one more element, or MinSize more<br>
 /// elements if specified.<br>
 void grow(size_t MinSize = 0);<br>
<br></blockquote><div><br></div><div>Thank you for the explanation. This probably deserves a short note in the patch description, as it may be not immediately obvious.</div><div>Â </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><br>
<br>
    On Fri, Jul 11, 2014 at 4:02 PM, Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><br></div>
    <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>> <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><div><div class="h5"><br>
    <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>>> wrote:<br>
<br>
      Author: alp<br>
      Date: Fri Jul 11 09:02:04 2014<br>
      New Revision: 212816<br>
<br>
      URL: <a href="http://llvm.org/viewvc/llvm-project?rev=212816&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=212816&view=rev</a><br>
      Log:<br>
      raw_svector_ostream: grow and reserve atomically<br>
<br>
      Including the scratch buffer size in the initial reservation<br>
      eliminates the<br>
      subsequent malloc+move operation and offers a healthier<br>
    constant<br>
      growth with<br>
      less memory wastage.<br>
<br>
      When doing this, take care to avoid invalidating the<br>
    source buffer.<br>
<br>
      Modified:<br>
        llvm/trunk/lib/Support/raw_<u></u>ostream.cpp<br>
<br>
      Modified: llvm/trunk/lib/Support/raw_<u></u>ostream.cpp<br>
      URL:<br>
    <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/raw_ostream.cpp?rev=212816&r1=212815&r2=212816&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>Support/raw_ostream.cpp?rev=<u></u>212816&r1=212815&r2=212816&<u></u>view=diff</a><br>
          ==============================<u></u>==============================<u></u>==================<br>
      --- llvm/trunk/lib/Support/raw_<u></u>ostream.cpp (original)<br>
      +++ llvm/trunk/lib/Support/raw_<u></u>ostream.cpp Fri Jul 11<br>
    09:02:04 2014<br>
      @@ -729,24 +729,26 @@ void raw_svector_ostream::resync() {<br>
       }<br>
<br>
       void raw_svector_ostream::write_<u></u>impl(const char *Ptr,<br>
    size_t Size) {<br>
      -  // If we're writing bytes from the end of the buffer<br>
    into the<br>
      smallvector, we<br>
      -  // don't need to copy the bytes, just commit the bytes<br>
    because<br>
      they are<br>
      -  // already in the right place.<br>
      -  if (Ptr == OS.end()) {<br>
      -   assert(OS.size() + Size <= OS.capacity() && "Invalid<br>
      write_impl() call!");<br>
      -   OS.set_size(OS.size() + Size);<br>
      +  size_t NewSize = OS.size() + Size;<br>
      +  size_t NewReservation = NewSize + 64;<br>
      +<br>
      +  bool NoOverlap = Ptr + Size < OS.begin() || Ptr ><br>
    OS.begin() +<br>
      OS.capacity();<br>
      +<br>
      +  if (NoOverlap) {<br>
      +   assert(!GetNumBytesInBuffer())<u></u>;<br>
      +   OS.reserve(NewReservation);<br>
      +   memcpy(OS.end(), Ptr, Size);<br>
      +   OS.set_size(NewSize);<br>
      +  } else if (Ptr == OS.end()) {<br>
      +   // Grow the buffer to include the scratch area<br>
    without copying.<br>
      +   assert(NewSize <= OS.capacity() && "Invalid<br>
    write_impl() call!");<br>
      +   OS.set_size(NewSize);<br>
      +   OS.reserve(NewReservation);<br>
        } else {<br>
      -   assert(GetNumBytesInBuffer() == 0 &&<br>
      -      "Should be writing from buffer if some bytes<br>
    in it");<br>
      -   // Otherwise, do copy the bytes.<br>
      -   OS.append(Ptr, Ptr+Size);<br>
      +   OS.append(Ptr, Ptr + Size);<br>
      +   OS.reserve(NewReservation);<br>
        }<br>
<br>
      -  // Grow the vector if necessary.<br>
      -  if (OS.capacity() - OS.size() < 64)<br>
      -   OS.reserve(OS.capacity() * 2);<br>
      -<br>
      -  // Update the buffer position.<br>
        SetBuffer(OS.end(), OS.capacity() - OS.size());<br>
       }<br>
<br>
<br>
<br>
      ______________________________<u></u>_________________<br>
      llvm-commits mailing list<br>
    <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.<u></u>edu</a>><br></div></div>
    <mailto:<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.<u></u>edu</a><div class=""><br>
    <mailto:<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.<u></u>edu</a>>><br>
    <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
<br>
<br>
  --   <a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
  the browser experts<br>
<br>
<br>
</div></blockquote><div class="HOEnZb"><div class="h5">
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><div><div><font color="#666666"><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(213,15,37);border-right-color:rgb(213,15,37);border-bottom-color:rgb(213,15,37);border-left-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Alexander Kornienko |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(51,105,232);border-right-color:rgb(51,105,232);border-bottom-color:rgb(51,105,232);border-left-color:rgb(51,105,232);padding-top:2px;margin-top:2px">Â Software Engineer |</span></font><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(0,153,57);border-right-color:rgb(0,153,57);border-bottom-color:rgb(0,153,57);border-left-color:rgb(0,153,57);padding-top:2px;margin-top:2px"><font color="#666666">Â </font><a href="mailto:alexfh@google.com" style="color:rgb(17,85,204)" target="_blank">alexfh@google.com</a>Â |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(238,178,17);border-right-color:rgb(238,178,17);border-bottom-color:rgb(238,178,17);border-left-color:rgb(238,178,17);padding-top:2px;margin-top:2px">Â Google Germany, Munich</span></div>
</div></div>
</div></div>