[libcxx-commits] [PATCH] D102727: Use precise growth rather than amortized growth in std::string::resize/__resize_default_init.

[Evan Brown via Phabricator via libcxx-commits]

ezbr added a comment.

> Could you rebase your patch on main? Sorry for the inconvenience.

Done

> I'm sorry, but I can't approve something with potentially such a sharp edge without more guidance. I realize you're just trying to upstream a change that is useful on your end and I appreciate that, but I'm a bit stuck because I have to try to represent everybody's interests, and I'm really lacking data to make anything but the conservative "let's keep the status quo" decision.

I understand. I've gathered more evidence and revised the patch description. I've tried to provide more evidence that this change is more beneficial than it is risky, and that it's the right tradeoff to make for everyone's benefit. Please take another look.

> I also just did an internal code search and in just 5 minutes, I can find at least a couple places that appear to be resizing in a loop with an increasing size like that.

I think there's a distinction between code that calls `resize` in a loop, and code that would have a measurable regression. For example, if the loop's trip count is always small or if the loop is not run frequently, then it's not likely to be a problem. From my experimentation and testing, code that measurably regresses due to precise resizing is exceedingly rare within Google, and I expect that outside of Google, it would also be very rare.

> Instead of changing resize() like this, have you considered using shrink_to_fit() more aggressively in places that have a noticeable memory impact?

I added to the change description that we expect to save ~0.1% of total memory usage with this change, but the savings are spread diffusely across a very large code base so I think it would not be feasible for me to get the same savings with `shrink_to_fit`. I'd also like to reemphasize that from my testing, I expect the fraction of uses of `resize` that measurably regress in performance to be very small so I think it is much more feasible to change those small fraction of cases to manually use exponential growth than it is to change the overwhelming majority of cases that can benefit from precise growth.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102727/new/

https://reviews.llvm.org/D102727