Add Arena::clone_from & Entry::clone_from

[Cobrand]

An aspect of the Clone trait which is not much known is clone_from, which is there to allow a struct to re-use pre-allocated memory for a clone. For instance if we were to have 2 generational_arenas almost identical, and we need to clone one to the other,
instead of doing arena2 = arena1.clone(), we could do arena2.clone_from(&arena1) to re-use allocated space from arena2 and simply copy the content from arena1 where possible.

This pull request implements the clone_from aspect for both Arena and Entry, so that the T can also fully use the capabilities of clone_from they may have (for instance if an Entry is/contains a Vec).

[Yamakaky]

Note: items.capacity() may be higher than items.len(). From Arena::capacity, it seems you assume that they are equal? From what I can see it doesn't seem to matter, but I may be missing something.

[fitzgen]

Thanks for opening this PR.

@Yamakaky is on to something: there is the property that self.items.len() == self.items.capacity() so that all capacity is available for insertion and is represented in the free list if it isn't occupied. So if self.items has more capacity than other.items does, then it should fill the free list with the additional capacity.

Can you also debug_assert! this property at the end of clone_from?

Thanks!

[fitzgen]

Is this a functional change or did it just slip into the commit?

[Yamakaky]

Since clone_from is for performance, I'd say to do the least amount of work and just let capacity() >= len(). We also can't call shrink_to_fit since that makes a new allocation. The additional capacity is not lost though, the next call to reserve() will just be faster. capacity() should be changed to use vec.capacity() to match the semantic of the doc, though. From what I can see this doesn't seem to impact the rest of the code but I may have missed something.

[Cobrand]

Thanks for opening this PR.

@Yamakaky is on to something: there is the property that self.items.len() == self.items.capacity() so that all capacity is available for insertion and is represented in the free list if it isn't occupied. So if self.items has more capacity than other.items does, then it should fill the free list with the additional capacity.

Can you also debug_assert! this property at the end of clone_from?

Thanks!

Actually, this PR has 2 commits, and the first one did exactly what you said (that is why there is a "leftover line" in the overall view). However I reverted the part where I filled the existing capacity with Free, because it was a problem for determinism (and the self.items.len() == self.items.capacity() is not always true either).

If we start with the self.items.len() == self.items.capacity() predicate, it's true if we use Arena::with_capacity which calls Vec::with_capacity, but if we start to insert over capacity, this assumption may not be true anymore. The logic for when the Arena is full and we need to re-allocate uses https://doc.rust-lang.org/std/vec/struct.Vec.html#method.reserve_exact , and according to the documentation:

Reserves the minimum capacity for exactly additional more elements to be inserted in the given Vec. After calling reserve_exact, capacity will be greater than or equal to self.len() + additional.

The capacity can still be greater than what we actually expect (although it's a "better try" than the reserve method)

For the determinism part, with the first commit only (so what you described), the behavior of this struct may change depending on whether or not the destination Arena (the one we re-use the allocation from) had bigger capacity than the source or not. If it's bigger, then the free_head is overwritten, and suddenly we have 2 Arena which are supposed to be the same because we just cloned one into the other which won't insert at the same place: the source arena will have the expected behavior, but the cloned into will have its new elements inserted at the end instead.

This is fine until we start using the iter() method: the entries won't be iterated in the same order even though they were inserted in the same order and in the same original structs.

This can be a very nasty problem to debug since you expect 2 cloned structs to behave the same same ways, and for no gain in performance either: we if go over capacity and the Vec behind it already has a capacity of len() + additional_capacity, no re-allocation will be done regardless.