Skip to content

Commit

Permalink
Make reshape(::Bitarray,...) type stable (#21670)
Browse files Browse the repository at this point in the history
This works around some of the performance regressions flagged in #20993. The instability itself is not new nor is it the cause of the regression. The trouble is just that there are a few more inlined functions now, each of which needs a dynamic dispatch.

It is worth noting that 0.6 (with this patch) is significantly faster than 0.5 (with this patch). I measure an improvement of ~30%.
  • Loading branch information
mbauman authored and KristofferC committed May 3, 2017
1 parent 5f296f3 commit 253fa74
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
7 changes: 5 additions & 2 deletions base/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -472,10 +472,13 @@ function copy!(dest::BitArray, src::Array)
return unsafe_copy!(dest, 1, src, 1, length(src))
end

function reshape(B::BitArray, dims::NTuple{N,Int}) where N
function reshape(B::BitArray{N}, dims::NTuple{N,Int}) where N
return dims == size(B) ? B : _bitreshape(B, dims)
end
reshape(B::BitArray, dims::Tuple{Vararg{Int}}) = _bitreshape(B, dims)
function _bitreshape(B::BitArray, dims::NTuple{N,Int}) where N
prod(dims) == length(B) ||
throw(DimensionMismatch("new dimensions $(dims) must be consistent with array size $(length(B))"))
dims == size(B) && return B
Br = BitArray{N}(ntuple(i->0,Val{N})...)
Br.chunks = B.chunks
Br.len = prod(dims)
Expand Down
4 changes: 4 additions & 0 deletions test/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ timesofar("conversions")
@check_bit_operation reshape(b1, (n2,n1)) BitMatrix
@test_throws DimensionMismatch reshape(b1, (1,n1))

@test @inferred(reshape(b1, n1*n2)) == @inferred(reshape(b1, (n1*n2,))) == @inferred(reshape(b1, Val{1})) == @inferred(reshape(b1, :))
@test @inferred(reshape(b1, n1, n2)) === @inferred(reshape(b1, Val{2})) === b1
@test @inferred(reshape(b1, n2, :)) == @inferred(reshape(b1, (n2, n1))) != @inferred(reshape(b1, Val{2}))

b1 = bitrand(s1, s2, s3, s4)
@check_bit_operation reshape(b1, (s3,s1,s2,s4)) BitArray{4}
@test_throws DimensionMismatch reshape(b1, (1,n1))
Expand Down

2 comments on commit 253fa74

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

Please sign in to comment.