Skip to content

Improve (no)specialization in print_matrix #39194

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 3 commits into from
Mar 8, 2021
Merged

Improve (no)specialization in print_matrix #39194

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7084d38
Improve (no)specialization in print_matrix
timholy Jan 11, 2021
bcb6fd5
Update base/arrayshow.jl
timholy Jan 12, 2021
9f484b6
remove objectionable invokelatest
timholy Feb 16, 2021
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
28 changes: 19 additions & 9 deletions base/arrayshow.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Parameter `sep::Integer` is number of spaces to put between elements.
Alignment is reported as a vector of (left,right) tuples, one for each
column going across the screen.
"""
function alignment(io::IO, X::AbstractVecOrMat,
function alignment(io::IO, @nospecialize(X::AbstractVecOrMat),
rows::AbstractVector{T}, cols::AbstractVector{V},
cols_if_complete::Integer, cols_otherwise::Integer, sep::Integer) where {T,V}
a = Tuple{T, V}[]
Expand Down Expand Up @@ -94,7 +94,7 @@ is specified as string sep.
`print_matrix_row` will also respect compact output for elements.
"""
function print_matrix_row(io::IO,
X::AbstractVecOrMat, A::Vector,
@nospecialize(X::AbstractVecOrMat), A::Vector,
i::Integer, cols::AbstractVector, sep::AbstractString)
for (k, j) = enumerate(cols)
k > length(A) && break
Expand Down Expand Up @@ -158,7 +158,7 @@ string post (printed at the end of the last row of the matrix).
Also options to use different ellipsis characters hdots, vdots, ddots.
These are repeated every hmod or vmod elements.
"""
function print_matrix(io::IO, @nospecialize(X::AbstractVecOrMat),
function print_matrix(io::IO, X::AbstractVecOrMat,
pre::AbstractString = " ", # pre-matrix string
sep::AbstractString = " ", # separator between elements
post::AbstractString = "", # post-matrix string
Expand Down Expand Up @@ -191,12 +191,16 @@ function _print_matrix(io, @nospecialize(X::AbstractVecOrMat), pre, sep, post, h
halfheight = div(screenheight,2)
if m > screenheight
rowsA = [rowsA[(0:halfheight-1) .+ firstindex(rowsA)]; rowsA[(end-div(screenheight-1,2)+1):end]]
else
rowsA = [rowsA;]
end
# Similarly for columns, only necessary to get alignments for as many
# columns as could conceivably fit across the screen
maxpossiblecols = div(screenwidth, 1+sepsize)
if n > maxpossiblecols
colsA = [colsA[(0:maxpossiblecols-1) .+ firstindex(colsA)]; colsA[(end-maxpossiblecols+1):end]]
else
colsA = [colsA;]
end
A = alignment(io, X, rowsA, colsA, screenwidth, screenwidth, sepsize)
# Nine-slicing is accomplished using print_matrix_row repeatedly
Expand Down Expand Up @@ -268,12 +272,15 @@ end

# typeinfo agnostic
# n-dimensional arrays
function show_nd(io::IO, a::AbstractArray, print_matrix::Function, label_slices::Bool)
show_nd(io::IO, a::AbstractArray, print_matrix::Function, label_slices::Bool) =
invokelatest(_show_nd, io, a, print_matrix, label_slices, map(unitrange, axes(a)))
Copy link
Member

Choose a reason for hiding this comment

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

This stood out to me as a little odd. Are there other places where we default to invokelatest like this?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a little odd. We use a similar trick (with invoke) in CoreLogging:

julia/base/logging.jl

Lines 74 to 91 in 6c1824d

# 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
return invoke(
shouldlog,
Tuple{typeof(logger), typeof(level), typeof(_module), typeof(group), typeof(id)},
logger, level, _module, group, id
)
end
function _invoked_min_enabled_level(@nospecialize(logger))
return invoke(min_enabled_level, Tuple{typeof(logger)}, logger)
end
function _invoked_catch_exceptions(@nospecialize(logger))
return invoke(catch_exceptions, Tuple{typeof(logger)}, logger)
end

because the logging framework is so untyped, and also would otherwise be a dependency of a huge number of methods (anything with a @warn, @info, etc). Since that includes the package-loading code, it seemed urgent to put in a barrier.

This is a much less clear-cut case. See #37741 for the background. I'd be willing to get rid of the invokelatest and wrap a in inferencebarrier. Especially with the axes now inferrable in this PR, that may be pretty safe. Thoughts?

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've removed the objectionable invokelatest, see if this passes muster. We have a little while to live with this and see whether it's a juicy invalidation target without it.


function _show_nd(io::IO, @nospecialize(a::AbstractArray), print_matrix::Function, label_slices::Bool, axs::Tuple{Vararg{AbstractUnitRange}})
limit::Bool = get(io, :limit, false)
if isempty(a)
return
end
tailinds = tail(tail(axes(a)))
tailinds = tail(tail(axs))
nd = ndims(a)-2
for I in CartesianIndices(tailinds)
idxs = I.I
Expand All @@ -284,7 +291,7 @@ function show_nd(io::IO, a::AbstractArray, print_matrix::Function, label_slices:
if length(ind) > 10
if ii == ind[firstindex(ind)+3] && all(d->idxs[d]==first(tailinds[d]),1:i-1)
for j=i+1:nd
szj = length(axes(a, j+2))
szj = length(axs[j+2])
indj = tailinds[j]
if szj>10 && first(indj)+2 < idxs[j] <= last(indj)-3
@goto skip
Expand All @@ -302,7 +309,7 @@ function show_nd(io::IO, a::AbstractArray, print_matrix::Function, label_slices:
if label_slices
_show_nd_label(io, a, idxs)
end
slice = view(a, axes(a,1), axes(a,2), idxs...)
slice = view(a, axs[1], axs[2], idxs...)
print_matrix(io, slice)
print(io, idxs == map(last,tailinds) ? "" : "\n\n")
@label skip
Expand Down Expand Up @@ -379,10 +386,13 @@ end
`_show_nonempty(io, X::AbstractMatrix, prefix)` prints matrix X with opening and closing square brackets,
preceded by `prefix`, supposed to encode the type of the elements.
"""
function _show_nonempty(io::IO, X::AbstractMatrix, prefix::String)
_show_nonempty(io::IO, X::AbstractMatrix, prefix::String) =
_show_nonempty(io, inferencebarrier(X), prefix, axes(X))

function _show_nonempty(io::IO, @nospecialize(X::AbstractMatrix), prefix::String, axs::Tuple{AbstractUnitRange,AbstractUnitRange})
@assert !isempty(X)
limit = get(io, :limit, false)::Bool
indr, indc = axes(X,1), axes(X,2)
indr, indc = axs
nr, nc = length(indr), length(indc)
rdots, cdots = false, false
rr1, rr2 = UnitRange{Int}(indr), 1:0
Expand Down