[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke added a comment. Hello. I would just like to follow up on the status of this patch. Is there anything else that is needed from me? I just want to make sure that nobody is waiting on me for anything; I'm still not 100% familiar with the process. Thanks in advance.

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke updated this revision to Diff 130819. tvanslyke added a comment. Moved `__clear_and_shrink()` to live with the other public dunder methods and adapted the associated test accordingly. https://reviews.llvm.org/D41976 Files: include/string

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke updated this revision to Diff 130775. tvanslyke added a comment. Since `__clear_and_shrink()` is private the test covers copy and move assignment. I also ran the libcxx/strings/basic.string and std/strings tests with a hard-coded `assert(__invariants());` at the end of

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke updated this revision to Diff 129969. tvanslyke added a comment. Implemented changes to ensure string state is valid after calling `__clear_and_shrink()`. Benchmark results are identical. https://reviews.llvm.org/D41976 Files: string Index: string

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke added a comment. Sorry if I'm spamming too much. Just to be clear, I agree completely with your sentiment but I'm pretty sure that `__clear_and_shrink()` has exactly the same effects as `clear(); shrink_to_fit();`. Maybe `reserve(0);` should should be unconditionally calling

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke added a comment. Just to elaborate, in `reserve(0)` after the deallocation there's a check `if (__now_long)` which takes the else branch and only ends up calling `__set_short_size(__sz); // __sz = 0 here`. So even without this diff the only thing `reserve(0)` does after deallocating

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke added a comment. I think the only thing we need is a call to __set_long_cap(0) in the body of the if() to make the string be in a valid state, but if you follow the logic for a call to reserve(0) (after having called clear()) it doesn't even end up doing that (unless I'm missing

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke added a comment. I can adapt it to the style guide if it is decided that it should be added. https://reviews.llvm.org/D41976 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke added a comment. I'll add that the reason I felt compelled to submit this change was because perf was saying most of my program, that only ever moved strings, was spending a majority of it's time in string::reserve() calls. If for no other reason, I think it's confusing for people

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke added a comment. Here are the results: Comparing old-copymove.json to new-copymove.json BenchmarkTime CPU Time Old Time New CPU Old CPU New

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke updated this revision to Diff 129685. tvanslyke added a comment. I went ahead and just pulled it out to a small inline member function and added it to __copy_assign_alloc(). Removed some other redundancies. https://reviews.llvm.org/D41976 Files: string Index: string

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

tvanslyke created this revision. tvanslyke added a reviewer: howard.hinnant. Herald added a subscriber: cfe-commits. shrink_to_fit() ends up doing a lot work to get information that we already know since we just called clear(). This change seems concise enough to be worth the couple extra