From b50992fff0f35aae215657bf34efcda8d4ce3d35 Mon Sep 17 00:00:00 2001 From: Michael Abbott <32575566+mcabbott@users.noreply.github.com> Date: Tue, 21 Dec 2021 21:18:41 -0500 Subject: [PATCH 1/7] faster copyto --- base/abstractarray.jl | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index af29aee6a74b6..8db3fc53f7e63 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -914,9 +914,18 @@ end function copyto!(dest::AbstractArray, dstart::Integer, src) i = Int(dstart) - for x in src - dest[i] = x - i += 1 + if haslength(src) + checkbounds(dest, i) + checkbounds(dest, i + length(src) - 1) + for x in src + @inbounds dest[i] = x + i += 1 + end + else + for x in src + dest[i] = x + i += 1 + end end return dest end From 8597c9a578f4ab0dca28a9fb3f1eee0ef9f85758 Mon Sep 17 00:00:00 2001 From: Michael Abbott <32575566+mcabbott@users.noreply.github.com> Date: Tue, 21 Dec 2021 23:47:10 -0500 Subject: [PATCH 2/7] update to N5N3's suggestion --- base/abstractarray.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 8db3fc53f7e63..784b5deaf69d0 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -915,11 +915,11 @@ end function copyto!(dest::AbstractArray, dstart::Integer, src) i = Int(dstart) if haslength(src) - checkbounds(dest, i) checkbounds(dest, i + length(src) - 1) - for x in src - @inbounds dest[i] = x - i += 1 + I = eachindex(dest)[i] + @inbounds for x in src + dest[I] = x + I = nextind(dest, I) end else for x in src From 559bcb69fe0cf0bad4cb53eb0869f93da4817236 Mon Sep 17 00:00:00 2001 From: Michael Abbott <32575566+mcabbott@users.noreply.github.com> Date: Tue, 21 Dec 2021 23:48:55 -0500 Subject: [PATCH 3/7] method for copyto!(dest, src) too --- base/abstractarray.jl | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 784b5deaf69d0..9913618195fa5 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -901,13 +901,23 @@ end typeof(function copyto! end).name.max_methods = UInt8(1) function copyto!(dest::AbstractArray, src) - destiter = eachindex(dest) - y = iterate(destiter) - for x in src - y === nothing && + if haslength(src) + length(dest) < length(src) && throw(ArgumentError("destination has fewer elements than required")) - dest[y[1]] = x - y = iterate(destiter, y[2]) + I = firstindex(dest) + @inbounds for x in src + dest[I] = x + I = nextind(dest, I) + end + else + destiter = eachindex(dest) + y = iterate(destiter) + for x in src + y === nothing && + throw(ArgumentError("destination has fewer elements than required")) + dest[y[1]] = x + y = iterate(destiter, y[2]) + end end return dest end From 05b055d360bd319d77ad8845272b585642b9751a Mon Sep 17 00:00:00 2001 From: Michael Abbott <32575566+mcabbott@users.noreply.github.com> Date: Wed, 22 Dec 2021 00:45:35 -0500 Subject: [PATCH 4/7] Revert "update to N5N3's suggestion" This reverts commit eaaefb1baa55e9bae1663bb3557e5fcd955c7ba6. --- base/abstractarray.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 9913618195fa5..28188c4d02707 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -925,11 +925,11 @@ end function copyto!(dest::AbstractArray, dstart::Integer, src) i = Int(dstart) if haslength(src) + checkbounds(dest, i) checkbounds(dest, i + length(src) - 1) - I = eachindex(dest)[i] - @inbounds for x in src - dest[I] = x - I = nextind(dest, I) + for x in src + @inbounds dest[i] = x + i += 1 end else for x in src From 58ad472c9c2ee2eac078396f66787b51a3e89bd2 Mon Sep 17 00:00:00 2001 From: Michael Abbott <32575566+mcabbott@users.noreply.github.com> Date: Wed, 22 Dec 2021 00:55:18 -0500 Subject: [PATCH 5/7] replace nextind with i+1 --- base/abstractarray.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 28188c4d02707..496c3c4529ebf 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -904,10 +904,10 @@ function copyto!(dest::AbstractArray, src) if haslength(src) length(dest) < length(src) && throw(ArgumentError("destination has fewer elements than required")) - I = firstindex(dest) + i = Int(firstindex(dest)) @inbounds for x in src - dest[I] = x - I = nextind(dest, I) + dest[i] = x + i += 1 end else destiter = eachindex(dest) From 713198206d7c9b86bdef38ad29ca136a1e65c3b1 Mon Sep 17 00:00:00 2001 From: Michael Abbott <32575566+mcabbott@users.noreply.github.com> Date: Wed, 5 Jan 2022 22:42:38 -0500 Subject: [PATCH 6/7] revert copyto!(dest, src) --- base/abstractarray.jl | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 496c3c4529ebf..8db3fc53f7e63 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -901,23 +901,13 @@ end typeof(function copyto! end).name.max_methods = UInt8(1) function copyto!(dest::AbstractArray, src) - if haslength(src) - length(dest) < length(src) && + destiter = eachindex(dest) + y = iterate(destiter) + for x in src + y === nothing && throw(ArgumentError("destination has fewer elements than required")) - i = Int(firstindex(dest)) - @inbounds for x in src - dest[i] = x - i += 1 - end - else - destiter = eachindex(dest) - y = iterate(destiter) - for x in src - y === nothing && - throw(ArgumentError("destination has fewer elements than required")) - dest[y[1]] = x - y = iterate(destiter, y[2]) - end + dest[y[1]] = x + y = iterate(destiter, y[2]) end return dest end From 1d0bfdb3f1a91f2e8f4bfd2f5b888908a4215061 Mon Sep 17 00:00:00 2001 From: Michael Abbott <32575566+mcabbott@users.noreply.github.com> Date: Wed, 5 Jan 2022 22:51:34 -0500 Subject: [PATCH 7/7] fix an empty case, add tests --- base/abstractarray.jl | 5 ++--- test/arrayops.jl | 10 ++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 8db3fc53f7e63..c59603376b6b4 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -914,9 +914,8 @@ end function copyto!(dest::AbstractArray, dstart::Integer, src) i = Int(dstart) - if haslength(src) - checkbounds(dest, i) - checkbounds(dest, i + length(src) - 1) + if haslength(src) && length(dest) > 0 + @boundscheck checkbounds(dest, i:(i + length(src) - 1)) for x in src @inbounds dest[i] = x i += 1 diff --git a/test/arrayops.jl b/test/arrayops.jl index b2badb66ce93d..e289f6d87d889 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -2105,6 +2105,16 @@ end @test_throws ArgumentError LinearAlgebra.copy_transpose!(a,2:3,1:3,b,1:5,2:7) end +@testset "empty copyto!" begin + @test isempty(copyto!(Int[], ())) + @test isempty(copyto!(Int[], Int[])) + @test copyto!([1,2], ()) == [1,2] + + @test isempty(copyto!(Int[], 1, ())) + @test isempty(copyto!(Int[], 1, Int[])) + @test copyto!([1,2], 1, ()) == [1,2] +end + module RetTypeDecl using Test import Base: +, *, broadcast, convert