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 <[email protected]> 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 <[email protected]
<mailto:[email protected]>> 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
    [email protected] <mailto:[email protected]>
    http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


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

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

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

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to