[PATCH] libstdc++: Do not leak empty COW strings

[Jonathan Wakely]

On Thu, 2 Dec 2021 at 17:21, Jonathan Wakely via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> Apart from "don't bother changing the COW string", does anybody see a
> reason we shouldn't do this? This passes all tests for normal COW
> strings and fully-dynamic COW strings.

Pushed to trunk.

>
> When non-const references, pointers or iterators are obtained to the
> contents of a COW std::basic_string, the implementation has to assume it
> could result in a write to the contents. If the string was previously
> shared, it does the "copy-on-write" step of creating a new copy of the
> data that is not shared by another object.  It also marks the string as
> "leaked", so that no future copies of it will share ownership either.
>
> However, if the string is empty then the only character in the sequence
> is the terminating null, and modifying that is undefined behaviour. This
> means that non-const references/pointers/iterators to an empty string
> are effectively const. Since no direct modification is possible, there
> is no need to "leak" the string, it can be safely shared with other
> objects. This avoids unnecessary allocations to create new copies of
> empty strings that can't be modified anyway.
>
> We already did this optimization for strings that share ownership of the
> static _S_empty_rep() object, but not for strings that have non-zero
> capacity, and not for fully-dynamic-strings (where the _S_empty_rep()
> object is never used).
>
> With this change we avoid two allocations in the return statement:
>
>   std::string s;
>   s.reserve(1);       // allocate
>   std::string s2 = s;
>   std::string s3 = s;
>   return s[0] + s2[0] + s3[0]; // leak+allocate twice
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/cow_string.h (basic_string::_M_leak_hard): Do not
>         reallocate an empty string.
> ---
>  libstdc++-v3/include/bits/cow_string.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h
> index 389b39583e4..b21a7422246 100644
> --- a/libstdc++-v3/include/bits/cow_string.h
> +++ b/libstdc++-v3/include/bits/cow_string.h
> @@ -3366,10 +3366,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      basic_string<_CharT, _Traits, _Alloc>::
>      _M_leak_hard()
>      {
> -#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0
> -      if (_M_rep() == &_S_empty_rep())
> +      // No need to create a new copy of an empty string when a non-const
> +      // reference/pointer/iterator into it is obtained. Modifying the
> +      // trailing null character is undefined, so the ref/pointer/iterator
> +      // is effectively const anyway.
> +      if (this->empty())
>         return;
> -#endif
> +
>        if (_M_rep()->_M_is_shared())
>         _M_mutate(0, 0, 0);
>        _M_rep()->_M_set_leaked();
> --
> 2.31.1
>