[llvm] r212816 - raw_svector_ostream: grow and reserve atomically

[Alexander Kornienko]

On Fri, Jul 11, 2014 at 6:35 PM, Alp Toker <alp at nuanti.com> wrote:

>
> On 11/07/2014 19:26, Alexander Kornienko wrote:
>
>> 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?
>>
>
> 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.
>
>
Can you explain why changing growth from exponential to linear is fine
here? For large sizes it will lead to O(size) write time.


>
>
>>
>> On Fri, Jul 11, 2014 at 4:02 PM, Alp Toker <alp at nuanti.com <mailto:
>> alp at nuanti.com>> wrote:
>>
>>     Author: alp
>>     Date: Fri Jul 11 09:02:04 2014
>>     New Revision: 212816
>>
>>     URL: http://llvm.org/viewvc/llvm-project?rev=212816&view=rev
>>     Log:
>>     raw_svector_ostream: grow and reserve atomically
>>
>>     Including the scratch buffer size in the initial reservation
>>     eliminates the
>>     subsequent malloc+move operation and offers a healthier constant
>>     growth with
>>     less memory wastage.
>>
>>     When doing this, take care to avoid invalidating the source buffer.
>>
>>     Modified:
>>         llvm/trunk/lib/Support/raw_ostream.cpp
>>
>>     Modified: llvm/trunk/lib/Support/raw_ostream.cpp
>>     URL:
>>     http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
>> Support/raw_ostream.cpp?rev=212816&r1=212815&r2=212816&view=diff
>>     ============================================================
>> ==================
>>     --- llvm/trunk/lib/Support/raw_ostream.cpp (original)
>>     +++ llvm/trunk/lib/Support/raw_ostream.cpp Fri Jul 11 09:02:04 2014
>>     @@ -729,24 +729,26 @@ void raw_svector_ostream::resync() {
>>      }
>>
>>      void raw_svector_ostream::write_impl(const char *Ptr, size_t Size) {
>>     -  // If we're writing bytes from the end of the buffer into the
>>     smallvector, we
>>     -  // don't need to copy the bytes, just commit the bytes because
>>     they are
>>     -  // already in the right place.
>>     -  if (Ptr == OS.end()) {
>>     -    assert(OS.size() + Size <= OS.capacity() && "Invalid
>>     write_impl() call!");
>>     -    OS.set_size(OS.size() + Size);
>>     +  size_t NewSize = OS.size() + Size;
>>     +  size_t NewReservation = NewSize + 64;
>>     +
>>     +  bool NoOverlap = Ptr + Size < OS.begin() || Ptr > OS.begin() +
>>     OS.capacity();
>>     +
>>     +  if (NoOverlap) {
>>     +    assert(!GetNumBytesInBuffer());
>>     +    OS.reserve(NewReservation);
>>     +    memcpy(OS.end(), Ptr, Size);
>>     +    OS.set_size(NewSize);
>>     +  } else if (Ptr == OS.end()) {
>>     +    // Grow the buffer to include the scratch area without copying.
>>     +    assert(NewSize <= OS.capacity() && "Invalid write_impl() call!");
>>     +    OS.set_size(NewSize);
>>     +    OS.reserve(NewReservation);
>>        } else {
>>     -    assert(GetNumBytesInBuffer() == 0 &&
>>     -           "Should be writing from buffer if some bytes in it");
>>     -    // Otherwise, do copy the bytes.
>>     -    OS.append(Ptr, Ptr+Size);
>>     +    OS.append(Ptr, Ptr + Size);
>>     +    OS.reserve(NewReservation);
>>        }
>>
>>     -  // Grow the vector if necessary.
>>     -  if (OS.capacity() - OS.size() < 64)
>>     -    OS.reserve(OS.capacity() * 2);
>>     -
>>     -  // Update the buffer position.
>>        SetBuffer(OS.end(), OS.capacity() - OS.size());
>>      }
>>
>>
>>
>>     _______________________________________________
>>     llvm-commits mailing list
>>     llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>>
> --
> http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140711/cea8ab97/attachment.html>