Fix inefficiency with ArrayList.insertSlice

[LucasSantos91]

There’s an inefficiency in insertSlice of std.ArrayList. This is the current implementation:

pub fn insertSlice(self: *Self, i: usize, items: []const T) Allocator.Error!void {
    try self.ensureUnusedCapacity(items.len);
    self.items.len += items.len;

    mem.copyBackwards(T, self.items[i + items.len .. self.items.len], self.items[i .. self.items.len - items.len]);
    @memcpy(self.items[i..][0..items.len], items);
}

The problem is that if ensureUnusedCapacity triggers a resize, it will copy all the elements to the new allocation. insertSlice will then copy a slice of these same items to the end of the allocation, to make space for the insertion. It’s better to copy everything to their final place at once.

This pull request introduces a new function to the public interface of ArrayList / ArrayListUnmanaged:

        /// Resize the array, adding `count` new elements at position `index`, which have `undefined` values.
        /// The return value is a slice pointing to the newly allocated elements. The returned pointer
        /// becomes invalid when the list is resized. Resizes list if self.capacity is not large enough.
        pub fn addManyAtIndex(
            self: *Self,
            allocator: Allocator,
            index: usize,
            count: usize,
        ) Allocator.Error![]T

This function makes space for a new slice of items at any position in the list. As other add* functions, the items are left unitialized. With this, it is trivial to reimplement insertSlice.

addManyAtIndex solves the inefficiency mentioned earlier by adapting code from ensureTotalCapacityPrecise and the old insertSlice. It manually grows the memory, if necessary, and copy everything only once.

In the process of implementing this function, I noticed none of the tests for insertSlice covered the case where the ArrayList needs to grow the capacity, so I wrote one.

[andrewrk]

Suggest to run the std lib tests locally before pushing, it will help you catch mistakes before the CI runs.

[andrewrk]

I rebased this branch, squashed your commits into one, and then added my own commit on top to make some more changes, and then finally have set this PR to auto-merge upon successful completion of the CI. Thanks for working on this 👍

[jnordwick]

This is a weird thing to worry about.

The array needs to grown by a factor of its length every time or it will be turn an O(1) ammortized append into an O(n) append.

realloc for most allocators may give you a few byte. There may be space recetly freed it can coallesce or just it rounded the last allocation up. You might be get the rest of the page of the original request was large enough to cause malloc to mmap a new region.

Ideally in the last case the the list will know that and bump up request sizes to page multiples and and always be as large as possible. Unless these allocs are humdreds of megs, don't bother with mremap. The page tables games it plays are very slow and cause a full TLB flush.

In the end, this is overly complex making up a poor allocation strategy at creation. Just make your next delta0 = (size0 + othersize) * 1.5 + C or something equally tribial and closedl forrm. Allocate and copy to final destination. If the used wants something wtih tighter bounds, there are class (or should be calls) to do that.

[squeek502]

@jnordwick note that this 'try to resize in place, if it fails then allocate a new slice' strategy is used throughout ArrayList; here's the function where basically all ArrayList allocation happens:

zig/lib/std/array_list.zig

Lines 1003 to 1012 in ae2cd5f

	const old_memory = self.allocatedSlice();
	if (allocator.resize(old_memory, new_capacity)) {
	self.capacity = new_capacity;
	} else {
	const new_memory = try allocator.alignedAlloc(T, alignment, new_capacity);
	@memcpy(new_memory[0..self.items.len], self.items);
	allocator.free(old_memory);
	self.items.ptr = new_memory.ptr;
	self.capacity = new_memory.len;
	}

So benchmarks showing that ArrayList performs better without the resize attempts would definitely be of interest.