[PATCH] Make RewriteBuffer write in chunks

Benjamin Kramer benny.kra at gmail.com
Tue Nov 12 10:19:34 PST 2013


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





More information about the cfe-commits mailing list