Skip to content

A grab bag of latency reductions #35714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 70 additions & 15 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -803,26 +803,81 @@ end
## copy between abstract arrays - generally more efficient
## since a single index variable can be used.

copyto!(dest::AbstractArray, src::AbstractArray) =
copyto!(IndexStyle(dest), dest, IndexStyle(src), src)
"""
copyto!(dest::AbstractArray, src) -> dest

function copyto!(::IndexStyle, dest::AbstractArray, ::IndexStyle, src::AbstractArray)
destinds, srcinds = LinearIndices(dest), LinearIndices(src)
isempty(srcinds) || (checkbounds(Bool, destinds, first(srcinds)) && checkbounds(Bool, destinds, last(srcinds))) ||
throw(BoundsError(dest, srcinds))
@inbounds for i in srcinds
dest[i] = src[i]
end
return dest

Copy all elements from collection `src` to array `dest`, whose length must be greater than
or equal to the length `n` of `src`. The first `n` elements of `dest` are overwritten,
the other elements are left untouched.

# Examples
```jldoctest
julia> x = [1., 0., 3., 0., 5.];

julia> y = zeros(7);

julia> copyto!(y, x);

julia> y
7-element Array{Float64,1}:
1.0
0.0
3.0
0.0
5.0
0.0
0.0
```
"""
function copyto!(dest::AbstractArray, src::AbstractArray)
isempty(src) && return dest
src′ = unalias(dest, src)
copyto_unaliased!(IndexStyle(dest), dest, IndexStyle(src′), src′)
end

function copyto!(deststyle::IndexStyle, dest::AbstractArray, srcstyle::IndexStyle, src::AbstractArray)
isempty(src) && return dest
src′ = unalias(dest, src)
copyto_unaliased!(deststyle, dest, srcstyle, src′)
end

function copyto!(::IndexStyle, dest::AbstractArray, ::IndexCartesian, src::AbstractArray)
function copyto_unaliased!(deststyle::IndexStyle, dest::AbstractArray, srcstyle::IndexStyle, src::AbstractArray)
isempty(src) && return dest
destinds, srcinds = LinearIndices(dest), LinearIndices(src)
isempty(srcinds) || (checkbounds(Bool, destinds, first(srcinds)) && checkbounds(Bool, destinds, last(srcinds))) ||
idf, isf = first(destinds), first(srcinds)
Δi = idf - isf
(checkbounds(Bool, destinds, isf+Δi) & checkbounds(Bool, destinds, last(srcinds)+Δi)) ||
throw(BoundsError(dest, srcinds))
i = 0
@inbounds for a in src
dest[i+=1] = a
if deststyle isa IndexLinear
if srcstyle isa IndexLinear
# Single-index implementation
@inbounds for i in srcinds
dest[i + Δi] = src[i]
end
else
# Dual-index implementation
i = idf - 1
@inbounds for a in src
dest[i+=1] = a
end
end
else
iterdest, itersrc = eachindex(dest), eachindex(src)
if iterdest == itersrc
# Shared-iterator implementation
for I in iterdest
@inbounds dest[I] = src[I]
end
else
# Dual-iterator implementation
ret = iterate(iterdest)
@inbounds for a in src
idx, state = ret
dest[idx] = a
ret = iterate(iterdest, state)
end
end
end
return dest
end
Expand Down
50 changes: 30 additions & 20 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,30 @@ function unsafe_copyto!(dest::Ptr{T}, src::Ptr{T}, n) where T
return dest
end


function _unsafe_copyto!(dest, doffs, src, soffs, n)
destp = pointer(dest, doffs)
srcp = pointer(src, soffs)
@inbounds if destp < srcp || destp > srcp + n
for i = 1:n
if isassigned(src, soffs + i - 1)
dest[doffs + i - 1] = src[soffs + i - 1]
else
_unsetindex!(dest, doffs + i - 1)
end
end
else
for i = n:-1:1
if isassigned(src, soffs + i - 1)
dest[doffs + i - 1] = src[soffs + i - 1]
else
_unsetindex!(dest, doffs + i - 1)
end
end
end
return dest
end

"""
unsafe_copyto!(dest::Array, do, src::Array, so, N)

Expand Down Expand Up @@ -279,37 +303,23 @@ function unsafe_copyto!(dest::Array{T}, doffs, src::Array{T}, soffs, n) where T
ccall(:jl_array_typetagdata, Ptr{UInt8}, (Any,), src) + soffs - 1,
n)
else
# handle base-case: everything else above was just optimizations
@inbounds if destp < srcp || destp > srcp + n
for i = 1:n
if isassigned(src, soffs + i - 1)
dest[doffs + i - 1] = src[soffs + i - 1]
else
_unsetindex!(dest, doffs + i - 1)
end
end
else
for i = n:-1:1
if isassigned(src, soffs + i - 1)
dest[doffs + i - 1] = src[soffs + i - 1]
else
_unsetindex!(dest, doffs + i - 1)
end
end
end
_unsafe_copyto!(dest, doffs, src, soffs, n)
end
@_gc_preserve_end t2
@_gc_preserve_end t1
return dest
end

unsafe_copyto!(dest::Array, doffs, src::Array, soffs, n) =
_unsafe_copyto!(dest, doffs, src, soffs, n)

"""
copyto!(dest, do, src, so, N)

Copy `N` elements from collection `src` starting at offset `so`, to array `dest` starting at
offset `do`. Return `dest`.
"""
function copyto!(dest::Array{T}, doffs::Integer, src::Array{T}, soffs::Integer, n::Integer) where T
function copyto!(dest::Array, doffs::Integer, src::Array, soffs::Integer, n::Integer)
n == 0 && return dest
n > 0 || _throw_argerror()
if soffs < 1 || doffs < 1 || soffs+n-1 > length(src) || doffs+n-1 > length(dest)
Expand All @@ -327,7 +337,7 @@ function _throw_argerror()
throw(ArgumentError("Number of elements to copy must be nonnegative."))
end

copyto!(dest::Array{T}, src::Array{T}) where {T} = copyto!(dest, 1, src, 1, length(src))
copyto!(dest::Array, src::Array) = copyto!(dest, 1, src, 1, length(src))

# N.B: The generic definition in multidimensional.jl covers, this, this is just here
# for bootstrapping purposes.
Expand Down
4 changes: 4 additions & 0 deletions base/dict.jl
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,10 @@ end

function setindex!(h::Dict{K,V}, v0, key::K) where V where K
v = convert(V, v0)
setindex!(h, v, key)
end

function setindex!(h::Dict{K,V}, v::V, key::K) where V where K
index = ht_keyindex2!(h, key)

if index > 0
Expand Down
1 change: 1 addition & 0 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,7 @@ function _tryrequire_from_serialized(modkey::PkgId, build_id::UInt64, modpath::U
invokelatest(callback, modkey)
end
for M in mod::Vector{Any}
M = M::Module
if PkgId(M) == modkey && module_build_id(M) === build_id
return M
end
Expand Down
17 changes: 13 additions & 4 deletions base/logging.jl
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,15 @@ logger type.
catch_exceptions(logger) = true


# Prevent invalidation when packages define custom loggers
# Using invoke in combination with @nospecialize eliminates backedges to these methods
function _invoked_shouldlog(logger, level, _module, group, id)
@nospecialize
invoke(shouldlog, Tuple{typeof(logger), typeof(level), typeof(_module), typeof(group), typeof(id)}, logger, level, _module, group, id)
end

_invoked_min_enabled_level(@nospecialize(logger)) = invoke(min_enabled_level, Tuple{typeof(logger)}, logger)


"""
NullLogger()
Expand Down Expand Up @@ -315,7 +324,7 @@ function logmsg_code(_module, file, line, level, message, exs...)
id = $id
# Second chance at an early bail-out (before computing the message),
# based on arbitrary logger-specific logic.
if shouldlog(logger, level, _module, group, id)
if _invoked_shouldlog(logger, level, _module, group, id)
file = $file
line = $line
try
Expand Down Expand Up @@ -374,7 +383,7 @@ struct LogState
logger::AbstractLogger
end

LogState(logger) = LogState(LogLevel(min_enabled_level(logger)), logger)
LogState(logger) = LogState(LogLevel(_invoked_min_enabled_level(logger)), logger)

function current_logstate()
logstate = current_task().logstate
Expand All @@ -391,6 +400,7 @@ end
end

function with_logstate(f::Function, logstate)
@nospecialize
t = current_task()
old = t.logstate
try
Expand All @@ -401,7 +411,6 @@ function with_logstate(f::Function, logstate)
end
end


#-------------------------------------------------------------------------------
# Control of the current logger and early log filtering

Expand Down Expand Up @@ -502,7 +511,7 @@ with_logger(logger) do
end
```
"""
with_logger(f::Function, logger::AbstractLogger) = with_logstate(f, LogState(logger))
with_logger(@nospecialize(f::Function), logger::AbstractLogger) = with_logstate(f, LogState(logger))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not expect this to make a difference; it doesn't affect inference and we will already avoid specializing this function on f.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it doesn't seem to change anything.


"""
current_logger()
Expand Down
55 changes: 5 additions & 50 deletions base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,11 @@ module IteratorsMD
convert(::Type{CartesianIndices{N,R}}, inds::CartesianIndices{N}) where {N,R} =
CartesianIndices(convert(R, inds.indices))

# equality
Base.:(==)(a::CartesianIndices{N}, b::CartesianIndices{N}) where N =
all(map(==, a.indices, b.indices))
Base.:(==)(a::CartesianIndices, b::CartesianIndices) = false

# AbstractArray implementation
Base.axes(iter::CartesianIndices{N,R}) where {N,R} = map(Base.axes1, iter.indices)
Base.IndexStyle(::Type{CartesianIndices{N,R}}) where {N,R} = IndexCartesian()
Expand Down Expand Up @@ -937,56 +942,6 @@ function fill!(A::AbstractArray{T}, x) where T
A
end

"""
copyto!(dest::AbstractArray, src) -> dest


Copy all elements from collection `src` to array `dest`, whose length must be greater than
or equal to the length `n` of `src`. The first `n` elements of `dest` are overwritten,
the other elements are left untouched.

# Examples
```jldoctest
julia> x = [1., 0., 3., 0., 5.];

julia> y = zeros(7);

julia> copyto!(y, x);

julia> y
7-element Array{Float64,1}:
1.0
0.0
3.0
0.0
5.0
0.0
0.0
```
"""
copyto!(dest, src)

function copyto!(dest::AbstractArray{T1,N}, src::AbstractArray{T2,N}) where {T1,T2,N}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method really not needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not actually sure, for now this is just a guess. Have not yet run tests or benchmarks, to me that will be the proof of the pudding.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a good one: on master,

julia> a = [1 2; 3 4]
2×2 Array{Int64,2}:
 1  2
 3  4

julia> b = [3, 1, 4, 2]
4-element Array{Int64,1}:
 3
 1
 4
 2

julia> copyto!(view(a, b), a)
4-element view(::Array{Int64,1}, [3, 1, 4, 2]) with eltype Int64:
 1
 3
 1
 1

vs

julia> copyto!(view(a, b), copy(a))
4-element view(::Array{Int64,1}, [3, 1, 4, 2]) with eltype Int64:
 1
 3
 2
 4

I'm hoping to fix it in this PR, or will report it if not.

isempty(src) && return dest
src′ = unalias(dest, src)
# fastpath for equal axes (#34025)
if axes(dest) == axes(src)
for I in eachindex(IndexStyle(src′,dest), src′)
@inbounds dest[I] = src′[I]
end
# otherwise enforce linear indexing
else
isrc = eachindex(IndexLinear(), src)
idest = eachindex(IndexLinear(), dest)
ΔI = first(idest) - first(isrc)
checkbounds(dest, last(isrc) + ΔI)
for I in isrc
@inbounds dest[I + ΔI] = src′[I]
end
end
return dest
end

function copyto!(dest::AbstractArray{T1,N}, Rdest::CartesianIndices{N},
src::AbstractArray{T2,N}, Rsrc::CartesianIndices{N}) where {T1,T2,N}
isempty(Rdest) && return dest
Expand Down
1 change: 1 addition & 0 deletions base/set.jl
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ delete!(s::Set, x) = (delete!(s.dict, x); s)

copy(s::Set) = copymutable(s)

copymutable(s::Set{T}) where {T} = Set{T}(s)
# Set is the default mutable fall-back
copymutable(s::AbstractSet{T}) where {T} = Set{T}(s)

Expand Down
12 changes: 12 additions & 0 deletions contrib/generate_precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ CTRL_C = '\x03'
UP_ARROW = "\e[A"
DOWN_ARROW = "\e[B"

hardcoded_precompile_statements = """
precompile(Tuple{typeof(Base.stale_cachefile), String, String})
precompile(Tuple{typeof(push!), Set{Module}, Module})
precompile(Tuple{typeof(push!), Set{Method}, Method})
precompile(Tuple{typeof(push!), Set{Base.PkgId}, Base.PkgId})
precompile(Tuple{typeof(setindex!), Dict{String,Base.PkgId}, Base.PkgId, String})
"""

precompile_script = """
2+2
print("")
Expand Down Expand Up @@ -152,6 +160,10 @@ function generate_precompile_statements()
push!(statements, statement)
end

for statement in split(hardcoded_precompile_statements, '\n')
push!(statements, statement)
end

# Create a staging area where all the loaded packages are available
PrecompileStagingArea = Module()
for (_pkgid, _mod) in Base.loaded_modules
Expand Down
8 changes: 7 additions & 1 deletion test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,12 @@ end
R = reshape(A, 2, 2)
A[R] .= reshape((1:4) .+ 2^30, 2, 2)
@test A == [2,1,4,3] .+ 2^30

# unequal dimensionality (see comment in #35714)
a = [1 3; 2 4]
b = [3, 1, 4, 2]
copyto!(view(a, b), a)
@test a == [2 1; 4 3]
end

@testset "Base.mightalias unit tests" begin
Expand Down Expand Up @@ -2799,4 +2805,4 @@ end
showerror(b, err)
@test String(take!(b)) ==
"BoundsError: attempt to access 2×2 Array{Float64,2} at index [10, \"bad index\"]"
end
end