[llvm] r212816 - raw_svector_ostream: grow and reserve atomically

[Alp Toker]

On 11/07/2014 19:41, Alexander Kornienko wrote:
> On Fri, Jul 11, 2014 at 6:35 PM, Alp Toker <alp at nuanti.com 
> <mailto: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.

There's no change to linear growth. See the implementation of reserve() 
which calls into SmallVector's grow():

   /// grow - Grow the allocated memory (without initializing new
   /// elements), doubling the size of the allocated memory.
   /// Guarantees space for at least one more element, or MinSize more
   /// elements if specified.
   void grow(size_t MinSize = 0);



>
>
>
>         On Fri, Jul 11, 2014 at 4:02 PM, Alp Toker <alp at nuanti.com
>         <mailto:alp at nuanti.com> <mailto: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>
>         <mailto: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
>
>

-- 
http://www.nuanti.com
the browser experts