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