<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 11, 2014 at 6:35 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">
<div class=""><br>
On 11/07/2014 19:26, Alexander Kornienko wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I wonder whether I missed the review thread for this patch or this kind of patch is generally accepted to be committed without any review?<br>
</blockquote>
<br></div>
I'm OK with this change, but any review is welcome. This is a supporting commit towards fixing the really old FIXMEs related to scratch buffer overallocation.<br>
<br></blockquote><div><br></div><div>Can you explain why changing growth from exponential to linear is fine here? For large sizes it will lead to O(size) write time.</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><div class="h5">
<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> <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 constant<br>
growth with<br>
less memory wastage.<br>
<br>
When doing this, take care to avoid invalidating the 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 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, size_t Size) {<br>
- // If we're writing bytes from the end of the buffer into the<br>
smallvector, we<br>
- // don't need to copy the bytes, just commit the bytes 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 > 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 without copying.<br>
+ assert(NewSize <= OS.capacity() && "Invalid 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 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></div></div>
<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>
<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><span class="HOEnZb"><font color="#888888">
</font></span></blockquote><span class="HOEnZb"><font color="#888888">
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</font></span></blockquote></div><br>
</div></div>