[PATCH] Make RewriteBuffer write in chunks

[Alp Toker]

We agreed to go with this patch for now.

Landed in r196119 along with a comment explaining the technique used.

Alp.


On 12/11/2013 18:19, Benjamin Kramer wrote:
> On 09.11.2013, at 00:31, Alp Toker <alp at nuanti.com> wrote:
>
>> On 08/11/2013 20:26, Manuel Klimek wrote:
>>> raw_ostream &RewriteBuffer::write(raw_ostream &os) const {
>>> -  // FIXME: eliminate the copy by writing out each chunk at a time
>>> -  os << std::string(begin(), end());
>>> +  for (RopePieceBTreeIterator I = begin(), E = end(); I != E;
>>> +       I.MoveToNextPiece())
>>> +    os << I.piece();
>>>    return os;
>>>
>>> Why's it not ok to write ++I? (if there is a good reason, a comment
>>> might help...)
>>>
>> The operator iterates individually through each character in the source
>> file, whereas MoveToNextPiece() gets us directly to the next chunk
>> letting us write/memcpy() directly into the output stream.
> Is this functionality actually used anywhere? Having an iterator over a set of StringRefs would make more sense than one over individual characters IMO.
>
> - Ben
>
>> Looking through SVN history I see that the efficient accessors were
>> 'cleaned' away as dead code and it ended up as a FIXME.
>>
>> Will add a comment.
>>
>> Alp.
>>
>>
>>>
>>>
>>> On Fri, Nov 8, 2013 at 1:41 AM, Alp Toker <alp at nuanti.com
>>> <mailto:alp at nuanti.com>> wrote:
>>>
>>>     Updated patch to use StringRef instead of RopePieceBTreeIterator
>>>     internals.
>>>
>>>     Check it out
>>>
>>>     Alp.
>>>
>>>
>>>     On 07/11/2013 07:08, Alp Toker wrote:
>>>> Rewrite was previously allocating potentially huge std::strings with
>>>> each call and filling it one character at a time.
>>>>
>>>> This has become a hot path for Refactoring, Modernize, FixIt,
>>>     Format,
>>>> and particularly ARCMT/ObjCMT which call it multiple times on
>>>     the full
>>>> source tree -- so copying out contiguous chunks while avoiding large
>>>> allocations is a good idea.
>>>>
>>>> This commit preserves the existing interface of RewriteRope and
>>>     pokes
>>>> into it directly for access to the byte vectors.
>>>>
>>>> Resolves a FIXME back from r101521. No change in behaviour.
>>>>
>>>     --
>>>     http://www.nuanti.com
>>>     the browser experts
>>>
>>>
>>>     _______________________________________________
>>>     cfe-commits mailing list
>>>     cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>> -- 
>> http://www.nuanti.com
>> the browser experts
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

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