Skip to content

Commit 2c358b8

Browse files
authored
Optimize permutations with low effort (no change in algorithm) (#184)
* perf: optimize `permutations` (`increment!` loop) - The construct previously used in the `for` loop is inefficient. - The loop can exit early as soon as `state[i] <= max` is satisfied. - All array indexing can be marked `@inbounds`. * style: trailing white spaces
1 parent 9380656 commit 2c358b8

File tree

2 files changed

+12
-10
lines changed

2 files changed

+12
-10
lines changed

src/permutations.jl

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ end
1616

1717
function has_repeats(state::Vector{Int})
1818
# This can be safely marked inbounds because of the type restriction in the signature.
19-
# If the type restriction is ever loosened, please check safety of the `@inbounds`
19+
# If the type restriction is ever loosened, please check safety of the `@inbounds`.
2020
@inbounds for outer in eachindex(state)
2121
for inner in (outer+1):lastindex(state)
2222
if state[outer] == state[inner]
@@ -28,12 +28,14 @@ function has_repeats(state::Vector{Int})
2828
end
2929

3030
function increment!(state::Vector{Int}, min::Int, max::Int)
31-
state[end] += 1
32-
for i in reverse(eachindex(state))[firstindex(state):end-1]
33-
if state[i] > max
34-
state[i] = min
35-
state[i-1] += 1
36-
end
31+
# All array indexing can be marked inbounds because of the type restriction in the signature.
32+
# If the type restriction is ever loosened, please check safety of the `@inbounds`.
33+
@inbounds state[end] += 1
34+
i = lastindex(state)
35+
@inbounds while i > firstindex(state) && state[i] > max
36+
state[i] = min
37+
state[i-1] += 1
38+
i -= 1
3739
end
3840
end
3941

@@ -96,7 +98,7 @@ permutations(a) = permutations(a, length(a))
9698
permutations(a, t)
9799
98100
Generate all size `t` permutations of an indexable object `a`.
99-
Only works for `a` with defined length.
101+
Only works for `a` with defined length.
100102
If `(t <= 0) || (t > length(a))`, then returns an empty vector of eltype of `a`
101103
102104
# Examples
@@ -122,7 +124,7 @@ julia> [ (len, collect(permutations(1:3, len))) for len in -1:4 ]
122124
"""
123125
function permutations(a, t::Integer)
124126
if t == 0
125-
# Correct behavior for a permutation of length 0 is a vector containing a single empty vector
127+
# Correct behavior for a permutation of length 0 is a vector containing a single empty vector
126128
return [Vector{eltype(a)}()]
127129
elseif t == 1
128130
# Easy case, just return each element in its own vector

test/permutations.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ end
3030
@test collect(permutations([], -1)) == Any[]
3131
@test collect(permutations([], 0)) == [Any[]]
3232
@test collect(permutations([], 1)) == Any[]
33-
33+
3434
@testset "permutation lengths" begin
3535
expected_lengths = [1, 5, 20, 60, 120, 120]
3636
ks = 0:5

0 commit comments

Comments
 (0)