Skip to content
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

optimize permute! and invpermute! #84

Merged
merged 1 commit into from
Jun 23, 2023
Merged

optimize permute! and invpermute! #84

merged 1 commit into from
Jun 23, 2023

Conversation

piever
Copy link
Contributor

@piever piever commented Jun 23, 2023

After JuliaLang/julia#44941 permute! and invpermute! no longer call the Base.permute!! and Base.invpermute!! methods. This ensures that permute! and invpermute! stay optimized for PooledArrays, otherwise they get a suboptimal fallback (see benchmark below).

julia> using BenchmarkTools, PooledArrays, Random

julia> perm = randperm(10_000);

julia> @btime permute!($v, $perm) setup=(v=PooledArray(rand(1:1000, 10_000)));
  9.800 μs (2 allocations: 39.11 KiB)

julia> @btime ($v .= $v[$perm]) setup=(v=PooledArray(rand(1:1000, 10_000))); # current permute! fallback
  10.800 μs (4 allocations: 39.17 KiB)

julia> @btime invpermute!($v, $perm) setup=(v=PooledArray(rand(1:1000, 10_000)));
  11.900 μs (2 allocations: 39.11 KiB)

julia> @btime ($v[$perm] = $v) setup=(v=PooledArray(rand(1:1000, 10_000))); # current invpermute! fallback
  103.400 μs (4 allocations: 39.17 KiB)

The improvement is much more substantial for invpermute!, but I imagine it makes sense to have both methods fully optimized. I was actually surprised that the permute! fallback was already almost optimal. EDIT: that was just due to it calling the optimized getindex(v::PooledArray, i::AbstractVector) and copyto!(dst::PooledArray, src::PooledArray) methods, whereas invpermute! has to call setindex!, which is more costly.

@quinnj quinnj merged commit 1bb9c95 into main Jun 23, 2023
@quinnj quinnj deleted the pv/permute branch June 23, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants