From df40702d4d586474cda7ea7b50dd971b09e6b6a8 Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Mon, 7 Oct 2024 11:59:00 -0400 Subject: [PATCH 1/7] fix `_growbeg!` unncessary resizing This was very explicitly designed such that if there was a bunch of extra space at the end of the array, we would copy rather than allocating, but by making `newmemlen` be at least `overallocation(memlen)` rather than `overallocation(len)`, this branch was never hit. found by https://github.com/JuliaLang/julia/issues/56026 --- base/array.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/array.jl b/base/array.jl index 5b3e6cc398479..8330b2c16bdaa 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1077,7 +1077,7 @@ function _growbeg!(a::Vector, delta::Integer) end # since we will allocate the array in the middle of the memory we need at least 2*delta extra space # the +1 is because I didn't want to have an off by 1 error. - newmemlen = max(overallocation(memlen), len + 2 * delta + 1) + newmemlen = max(overallocation(len), len + 2 * delta + 1) newoffset = div(newmemlen - newlen, 2) + 1 # If there is extra data after the end of the array we can use that space so long as there is enough # space at the end that there won't be quadratic behavior with a mix of growth from both ends. From 433b4ef474ad8840fdee8cf2dd8f199d84df17b7 Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Mon, 7 Oct 2024 12:02:39 -0400 Subject: [PATCH 2/7] add test --- test/arrayops.jl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/arrayops.jl b/test/arrayops.jl index 333b68e287c4c..5a50a3e74b012 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -496,6 +496,11 @@ end @test vc == [v[1:(i-1)]; 5; v[i:end]] end @test_throws BoundsError insert!(v, 5, 5) + + # test that data is copied when there is plenty of room to do so + v = empty!(collect(1:100)) + pushfirst!(v, 1) + @test length(v.ref.mem) == 100 end @testset "popat!(::Vector, i, [default])" begin From 6f178c167d24b25f6eee89bb12f175e61f0fa893 Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Mon, 7 Oct 2024 12:08:31 -0400 Subject: [PATCH 3/7] whitespace --- test/arrayops.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/arrayops.jl b/test/arrayops.jl index 5a50a3e74b012..ec8f54828b965 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -496,7 +496,7 @@ end @test vc == [v[1:(i-1)]; 5; v[i:end]] end @test_throws BoundsError insert!(v, 5, 5) - + # test that data is copied when there is plenty of room to do so v = empty!(collect(1:100)) pushfirst!(v, 1) From cd63e0bae0d48ca723f74dd479bfa67ad72ec0fc Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Mon, 7 Oct 2024 18:22:47 -0400 Subject: [PATCH 4/7] unsetindex correctly --- base/array.jl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/base/array.jl b/base/array.jl index 8330b2c16bdaa..f780af6be89aa 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1085,11 +1085,16 @@ function _growbeg!(a::Vector, delta::Integer) # increasing the size of the array, and that we leave enough space at both the beginning and the end. if newoffset + newlen < memlen newoffset = div(memlen - newlen, 2) + 1 + @show newoffset, newmemlen, offset, len newmem = mem + unsafe_copyto!(newmem, newoffset + delta, mem, offset, len) + for j in offset:newoffset+delta-1 + @inbounds _unsetindex!(a, j) + end else newmem = array_new_memory(mem, newmemlen) + unsafe_copyto!(newmem, newoffset + delta, mem, offset, len) end - unsafe_copyto!(newmem, newoffset + delta, mem, offset, len) if ref !== a.ref @noinline throw(ConcurrencyViolationError("Vector can not be resized concurrently")) end From 1b5f40e342aca5884bc7551ea4978c2b22ac3496 Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Mon, 7 Oct 2024 18:23:05 -0400 Subject: [PATCH 5/7] typo --- base/array.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/base/array.jl b/base/array.jl index f780af6be89aa..a6b7d7d94be90 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1085,7 +1085,6 @@ function _growbeg!(a::Vector, delta::Integer) # increasing the size of the array, and that we leave enough space at both the beginning and the end. if newoffset + newlen < memlen newoffset = div(memlen - newlen, 2) + 1 - @show newoffset, newmemlen, offset, len newmem = mem unsafe_copyto!(newmem, newoffset + delta, mem, offset, len) for j in offset:newoffset+delta-1 From 1b5ec48e2dfc49d8a52d46a6899ad1824699225f Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Tue, 8 Oct 2024 13:54:32 -0400 Subject: [PATCH 6/7] fix --- base/array.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/array.jl b/base/array.jl index a6b7d7d94be90..17009ed094f9a 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1088,7 +1088,7 @@ function _growbeg!(a::Vector, delta::Integer) newmem = mem unsafe_copyto!(newmem, newoffset + delta, mem, offset, len) for j in offset:newoffset+delta-1 - @inbounds _unsetindex!(a, j) + @inbounds _unsetindex!(mem, j) end else newmem = array_new_memory(mem, newmemlen) From 3b25f0e6f3aeef080b06e5a6bf7154a2f2407027 Mon Sep 17 00:00:00 2001 From: Oscar Smith Date: Wed, 9 Oct 2024 14:23:19 -0400 Subject: [PATCH 7/7] fix effects --- base/array.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/base/array.jl b/base/array.jl index 17009ed094f9a..d8a9c725c8102 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1071,6 +1071,7 @@ function _growbeg!(a::Vector, delta::Integer) setfield!(a, :ref, @inbounds memoryref(ref, 1 - delta)) else @noinline (function() + @_terminates_locally_meta memlen = length(mem) if offset + len - 1 > memlen || offset < 1 throw(ConcurrencyViolationError("Vector has invalid state. Don't modify internal fields incorrectly, or resize without correct locks"))