Skip to content

VLA copy assignment: use allocator reallocate when growing #212

Description

@thirtytwobits

Summary

copy_assign_from has a TODO to use the allocator's reallocate extension when the destination has to grow:

It doesn't make any sense, until we support realloc here (TODO: support realloc here), to move the existing data with the resize since we're going to be copying over it right away.

This issue tracks deciding whether to do that and, if so, how.

Current behaviour

When rhs.size() exceeds the destination's capacity (or the destination adopts an unequal allocator through POCCA), copy assignment:

  1. destroys and deallocates the destination's buffer,
  2. adopts the rhs allocator if POCCA,
  3. reserve(rhs.size()), which allocates exactly rhs.size(),
  4. copy-constructs every element.

Peak memory is rhs.capacity() + rhs.size(). The destination's old buffer is never live at the same time as its new one.

The VLA already detects an allocator reallocate(value_type*, old_count, new_count) member (detection), which polymorphic_allocator forwards to memory_resource::reallocate. reserve() and shrink_to_fit() use it, but copy assignment does not.

What realloc support could look like

Use it only when the destination keeps its allocator: the allocators are equal, or POCCA is false.

  1. Destroy all destination elements first. The buffer may move, so copy-assigning over the overlap isn't safe, and with no live objects a byte-moving reallocate is harmless.
  2. Call reallocate(data_, capacity_, rhs.size()). On success adopt the returned buffer. On failure fall back to the current deallocate-then-allocate path.
  3. Copy-construct all rhs.size() elements into the buffer.

Possible benefits:

  • Allocators that can grow a block in place (for example a buffer resource whose block is at the end of its arena, or an o1heap neighbour that's free) avoid a free/allocate pair and the fragmentation that comes with it.
  • Fewer calls into the allocator on constrained targets.

Open questions

  • Peak memory can get worse. memory_resource::reallocate follows std::realloc semantics: if it can't grow in place, it allocates a new block, copies the bytes and frees the old one. That briefly holds old capacity + new size in the destination's arena, on top of the source, and copies bytes that are about to be overwritten. Today's path never does that. This works against the "keep resource use small" policy in VLA move construction with unequal allocators should preserve source capacity #211, so a plain reallocate call may only be acceptable if:
    • there's a way to ask for in-place-only growth (try to extend; fail rather than move), or
    • the policy accepts the transient peak for allocators that opt in.
  • Contract. The comment in reserve() says reallocate is expected to "extend the reserved area for the same memory pointer", but the memory_resource::reallocate contract allows the block to move. We should pin down which one the VLA relies on.
  • Exception guarantee. The current path gives the basic guarantee (destination left empty on failure, see VLA move construction with unequal allocators should preserve source capacity #211). The realloc path should be no worse. If reallocate fails it leaves the original block valid, so falling back to the current path keeps that.
  • Scope. Should move assignment's unequal-allocator reallocate branch (VLA move construction with unequal allocators should preserve source capacity #211) get the same treatment for consistency?

Done when

  • We've decided to support it (with an in-place-only mechanism or a documented peak trade-off) or not to.
  • If yes: copy assignment uses it under the conditions above, with tests in test_variable_length_array_detailed_allocation.cpp for: reallocated in place, reallocate failed with fallback, and an allocator without reallocate.
  • If no: the TODO is replaced with a comment explaining why deallocate-then-allocate is deliberate.

Related

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions