[libcxx-commits] [libcxx] [libc++][safety] Enhance exception safety for vector assignments in reallocation scenarios (PR #117516)

[Nikolas Klauser via libcxx-commits]

philnik777 wrote:

> Thank you for your comments and insights.
> 
> > I'm really not convinced that this is "without extra overhead" as you say.
> 
> By "without extra overhead", I mean this approach does not add any additional processing time, as it simply reorders the existing operations without introducing extra ones, while still providing a strong exception guarantee in this specific scenario.
> 
> > We still hold on to the old resources we don't actually need anymore, which may result in significant overhead, especially when the same allocation could be re-used (e.g. a free-list allocator that allocates at least 16 bytes, but the vector grows just from 4 bytes to 16 bytes).
> 
> I acknowledge that the peak memory usage is increased. However, in most cases where memory constraints are not stringent or when dealing with small vectors, this peak increase is generally manageable, as is the case with operations such as `push_back`, `emplace_back`, `reserve`, and `resize`.

In the scenario I'm describing it's not about the increased memory usage, but rather that the allocator may have to take a much slower path now, potentially even back to the kernel, instead of giving back the just deallocated block.

> > OTOH we don't actually have improved guarantees, since this improves only the reallocating scenario.
> 
> As you mentioned, this improvement specifically targets the reallocating scenario. I acknowledge that this enhancement applies solely to the reallocation scenario. The reason for focusing on this scenario is that it allows us to enhance safety guarantees without incurring any additional time costs. For non-reallocating scenarios, achieving the strong guarantees would indeed require extra processing time.
> 
> > If someone actually cares to have a strong exception guarantee (which seems to be almost no one given that we had some exception bugs in vector for over a year without anybody complaining), they can instead construct a new vector and swap it with the one they want to assign to.
> 
> In the scenario you described, where someone cares about having a strong exception guarantee, this patch indeed reduces the user's effort in achieving that strong guarantee. Users can determine whether their assignments would trigger reallocation by checking the capacity and avoid the need to apply the copy-and-swap technique in the reallocation scenario. For the non-reallocation scenario, it is up to the user if they can afford the additional time to achieve strong guarantees.

I don't think we save anything for a user here. If a user cares about the exceptions safety they'll just implement exactly what we'd do in the allocating case after this patch. The branches you're describing would be pretty much identical, so there doesn't seem to be much benefit in conditionally using `assign` instead of constructing a new vector.

> I appreciate your feedback and am open to further discussion on this topic.



https://github.com/llvm/llvm-project/pull/117516