[PATCH] Make RewriteBuffer write in chunks

[Alp Toker]

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.

Sure, I didn't want to butcher the interface in case there was a reason
-- someone seems to have put in plenty of work to make it go a byte at a
time.

All the RewriteRope instances I've seen so far remain mostly as intact
chunks even in the face of major rewrite operations so going in chunks
makes sense.

Will see if anything else was actually using the facility, and factor it
down as you suggest unless there are objections.

Thanks
Alp.


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