[libcxx-commits] [PATCH] D80588: Optimize vector push_back to avoid continuous load and store of end pointer.

[Michael Schellenberger Costa via Phabricator via libcxx-commits]

miscco added a comment.

That are great numbers!

If I understand it correctly this patch has two parts:

1. Pass `this->__end_` to the helper functions
2. Return the new end pointer from those helpers

I am skeptical why step 2 is needed at all. You never remove setting of `this->__end_`. So why do you need to do work that has already been done? Could you please verify that the second part is indeed necessary?

If it is indeed necessary I note that you pessimize the slow path by decrementing and then incrementing.

I would greatly prefer it if you would directly return in both the fast and the slow path.

Finally, your `__construct_one_at` helper function diverges from the previous code.

- The ASAN annotation is new and not done in the slow path.
- `__construct_at_end` has the additional `_ConstructTransaction __tx(*this, 1);` I am new to the party but I am suspicious of exception safety here in case of a throwing constructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80588