-
Notifications
You must be signed in to change notification settings - Fork 162
Description
I tried to speed up computations of minors and did some benchmarking. It seems that src/Combinatorics/EnumerativeCombinatorics/combinations.jl:69 is quite costly when it comes to bigger iterations and the point seems to be allocations by the C.v[state] call.
I'm not super into the code, but it seems to me that in 99% of the use cases, C.v will actually be a vector with entries 1, ..., n, so that C.v[state] will effectively return state? Could we maybe shortwire this so that we avoid the allocations?
For comparison: I run the following line with the code as it is at the moment.
julia> @time orig = collect(combinations(100, 5));
10.148674 seconds (150.58 M allocations: 7.292 GiB, 70.52% gc time)
julia> @time orig = collect(combinations(100, 5));
12.519579 seconds (150.58 M allocations: 7.292 GiB, 74.96% gc time)
If I replace that line by return Combinations(state), state, I get
julia> @time collect(combinations(100, 5));
0.427273 seconds (5 allocations: 574.398 MiB)
This does not produce the correct result, however! I need to do Combinations(copy(state)) to get the correct thing which, in fact, messes up the timings again. Yet, I was wondering whether there might be a reasonable angle of attack here.
ping @mjrodgers