Google software engineer focused on efficiency
- User Since
- May 17 2021, 7:40 AM (11 w, 21 h)
Thu, Jul 29
Could you rebase your patch on main? Sorry for the inconvenience.
Remove ifdef guards (so we change the default behavior) and rebase onto a newer version of llvm.
Wed, Jul 21
Update config comment and change test to not hardcode basic_string::alignment.
Thanks for your replies. I'd like to understand: how did you measure the memory improvements you found? Ideally, it would be nice to have something that we can reproduce on a large open-source code base (e.g. Chrome) so that the experiment can be verified. Is that possible? Otherwise, you have to understand I'm blindly basing the decision on "an internal Google experiment", which while I do trust, is not the best from a due diligence perspective. I'm being very conservative because silently introducing quadratic behavior in user code is pretty close to the scariest thing a maintainer can think about -- very bad but also very difficult to detect.
Tue, Jul 20
Regarding kAlign, this is basic_string::__alignment, which is private. Do you think it's okay to include this implementation detail here? If not, I think it could be computed, but I thought that could add complexity to the test and that this value is unlikely to change often. Do you prefer computing it in the test?
Add wstring coverage and add a comment in the test.
I followed @Quuxplusone's recommendation of using a new __config flag and enabling it by default for unstable ABI/ABI version 2.
Fix an issue with the previous version of the review.
Mon, Jul 19
Somehow the current version of this phabricator review doesn't seem to include my changes other than the new test file. I'll try to figure it out tomorrow.
Try again to update phabricator review. The previous attempt somehow only had the test file change.
Guard changes behind new _LIBCPP_ABI_EXACT_STRING_RESIZE config flag and add a test.
Wed, Jul 14
Change from adding an option for precise resize to doing so by default and update the change description.
Jun 16 2021
I updated the documentation and added more details about the motivation to the revision summary.
Updated configuration macro documentation.