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