Skip to content

more latency improvements for #33615 #33779

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.

Sign up for GitHub

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 1 commit into from
Nov 9, 2019
Merged

more latency improvements for #33615 #33779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion base/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,7 @@ function hcat(V::Vector{T}...) where T
throw(DimensionMismatch("vectors must have same lengths"))
end
end
return [ V[j][i]::T for i=1:length(V[1]), j=1:length(V) ]
return T[ V[j][i] for i=1:length(V[1]), j=1:length(V) ]
end

function vcat(arrays::Vector{T}...) where T
Expand Down
4 changes: 4 additions & 0 deletions base/compiler/abstractinterpretation.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,8 @@ function abstract_call(@nospecialize(f), fargs::Union{Nothing,Vector{Any}}, argt
return ret
end
return Any
elseif f === Tuple && la == 2 && !isconcretetype(widenconst(argtypes[2]))
return Tuple
elseif is_return_type(f)
rt_rt = return_type_tfunc(argtypes, vtypes, sv)
if rt_rt !== nothing
Expand Down Expand Up @@ -842,6 +844,8 @@ function abstract_call(@nospecialize(f), fargs::Union{Nothing,Vector{Any}}, argt
end
elseif length(argtypes) == 2 && istopfunction(f, :typename)
return typename_static(argtypes[2])
elseif max_methods > 1 && istopfunction(f, :copyto!)
max_methods = 1
Copy link
Member

Choose a reason for hiding this comment

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

This can be a bit unfortunate for the common return copyto!(dest, src) construct (or otherwise using the return value instead of dest, as in foo!(copyto!(dest, src))) with typeof(dest) known but typeof(src) unknown. All matching methods of copyto! should be inferable as typeof(dest), but that won't help. If only all those places had used copyto!(dest, src); return dest...

Since #35965, this e.g. also affects the Array constructor:

julia> code_typed(Vector{Any}, Tuple{Vector}; optimize=false)
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = Core.apply_type(Base.Array, $(Expr(:static_parameter, 1)), $(Expr(:static_parameter, 2)))::Core.Const(Vector{Any}, false)
│   %2 = Base.size(x)::Tuple{Int64}
│   %3 = (%1)(Base.undef, %2)::Vector{Any}
│   %4 = Base.copyto_axcheck!(%3, x)::Any
└──      return %4
) => Any

Just increasing this to 2 restores

julia> code_typed(Vector{Any}, Tuple{Vector}; optimize=false)
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = Core.apply_type(Base.Array, $(Expr(:static_parameter, 1)), $(Expr(:static_parameter, 2)))::Core.Const(Vector{Any}, false)
│   %2 = Base.size(x)::Tuple{Int64}
│   %3 = (%1)(Base.undef, %2)::Vector{Any}
│   %4 = Base.copyto_axcheck!(%3, x)::Vector{Any}
└──      return %4
) => Vector{Any}

Not sure whether we should

  • live with this
  • undo this change (or at least bump max_methods to at least 2)
  • audit all call sites of copyto! to not use the return value

Copy link
Member

Choose a reason for hiding this comment

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

FWIW, The copyto! method from #35965 should be removable with denizyuret/Knet.jl@bd7c162

end

atype = argtypes_to_type(argtypes)
Expand Down
4 changes: 1 addition & 3 deletions base/compiler/tfuncs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ function instanceof_tfunc(@nospecialize(t))
t′ = unwrap_unionall(t)
t′′, isexact = instanceof_tfunc(t′)
tr = rewrap_unionall(t′′, t)
# Note: adding the <:Tuple upper bound in NamedTuple was part of the load time
# regression in #33615.
if t′′ isa DataType && !has_free_typevars(tr) && t′′.name !== NamedTuple_typename
if t′′ isa DataType && !has_free_typevars(tr)
# a real instance must be within the declared bounds of the type,
# so we can intersect with the original wrapper.
tr = typeintersect(tr, t′′.name.wrapper)
Expand Down
23 changes: 15 additions & 8 deletions base/methodshow.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ function arg_decl_parts(m::Method)
for t in tv
show_env = ImmutableDict(show_env, :unionall_env => t)
end
decls = Any[argtype_decl(show_env, argnames[i], sig, i, m.nargs, m.isva)
decls = Tuple{String,String}[argtype_decl(show_env, argnames[i], sig, i, m.nargs, m.isva)
for i = 1:m.nargs]
else
decls = Any[("", "") for i = 1:length(sig.parameters)]
decls = Tuple{String,String}[("", "") for i = 1:length(sig.parameters::SimpleVector)]
end
return tv, decls, file, line
end
Expand Down Expand Up @@ -183,9 +183,11 @@ function show(io::IO, m::Method)
ft = unwrap_unionall(ft0)
d1 = decls[1]
if sig === Tuple
print(io, m.name)
decls = Any[(), ("...", "")]
elseif ft <: Function && isa(ft, DataType) &&
# Builtin
print(io, m.name, "(...) in ", m.module)
return
end
if ft <: Function && isa(ft, DataType) &&
isdefined(ft.name.module, ft.name.mt.name) &&
# TODO: more accurate test? (tn.name === "#" name)
ft0 === typeof(getfield(ft.name.module, ft.name.mt.name))
Expand All @@ -201,7 +203,7 @@ function show(io::IO, m::Method)
print(io, "(", d1[1], "::", d1[2], ")")
end
print(io, "(")
join(io, [isempty(d[2]) ? d[1] : d[1]*"::"*d[2] for d in decls[2:end]],
join(io, String[isempty(d[2]) ? d[1] : d[1]*"::"*d[2] for d in decls[2:end]],
", ", ", ")
kwargs = kwarg_decl(m)
if !isempty(kwargs)
Expand Down Expand Up @@ -259,7 +261,7 @@ function show_method_table(io::IO, ms::MethodList, max::Int=-1, header::Bool=tru
if max==-1 || n<max
n += 1
println(io)
print(io, "[$(n)] ")
print(io, "[$n] ")
show(io, meth)
file, line = updated_methodloc(meth)
push!(LAST_SHOWN_LINE_INFOS, (string(file), line))
Expand Down Expand Up @@ -340,6 +342,11 @@ function show(io::IO, ::MIME"text/html", m::Method)
ft0 = sig.parameters[1]
ft = unwrap_unionall(ft0)
d1 = decls[1]
if sig === Tuple
# Builtin
print(io, m.name, "(...) in ", m.module)
return
end
if ft <: Function && isa(ft, DataType) &&
isdefined(ft.name.module, ft.name.mt.name) &&
ft0 === typeof(getfield(ft.name.module, ft.name.mt.name))
Expand All @@ -355,7 +362,7 @@ function show(io::IO, ::MIME"text/html", m::Method)
print(io, "(", d1[1], "::<b>", d1[2], "</b>)")
end
print(io, "(")
join(io, [isempty(d[2]) ? d[1] : d[1]*"::<b>"*d[2]*"</b>"
join(io, String[isempty(d[2]) ? d[1] : d[1]*"::<b>"*d[2]*"</b>"
for d in decls[2:end]], ", ", ", ")
kwargs = kwarg_decl(m)
if !isempty(kwargs)
Expand Down
8 changes: 4 additions & 4 deletions base/show.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,8 @@ show(io::IO, @nospecialize(x)) = show_default(io, x)
show_default(io::IO, @nospecialize(x)) = _show_default(io, inferencebarrier(x))

function _show_default(io::IO, @nospecialize(x))
t = typeof(x)::DataType
show(io, t)
t = typeof(x)
show(io, inferencebarrier(t))
print(io, '(')
nf = nfields(x)
nb = sizeof(x)
Expand Down Expand Up @@ -1135,7 +1135,7 @@ function show_generator(io, ex, indent)
end

function valid_import_path(@nospecialize ex)
return Meta.isexpr(ex, :(.)) && length(ex.args) > 0 && all(a->isa(a,Symbol), ex.args)
return Meta.isexpr(ex, :(.)) && length((ex::Expr).args) > 0 && all(a->isa(a,Symbol), (ex::Expr).args)
end

function show_import_path(io::IO, ex)
Expand Down Expand Up @@ -1514,7 +1514,7 @@ function show_unquoted(io::IO, ex::Expr, indent::Int, prec::Int)

elseif (head === :import || head === :using) && nargs == 1 &&
(valid_import_path(args[1]) ||
(Meta.isexpr(args[1], :(:)) && length(args[1].args) > 1 && all(valid_import_path, args[1].args)))
(Meta.isexpr(args[1], :(:)) && length((args[1]::Expr).args) > 1 && all(valid_import_path, (args[1]::Expr).args)))
print(io, head)
print(io, ' ')
first = true
Expand Down