Skip to content

Add next_permutation and prev_permutation onto MutableOrdVector<T>. #14623

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

Merged
merged 1 commit into from
Jun 4, 2014
Merged

Add next_permutation and prev_permutation onto MutableOrdVector<T>. #14623

merged 1 commit into from
Jun 4, 2014

Conversation

exscape
Copy link
Contributor

@exscape exscape commented Jun 3, 2014

Unlike ImmutableClonableVector::permutations() which returns an iterator,
cloning the entire array each iteration, these methods mutate the vector in-place.
For that reason, these methods are much faster; between 35-55 times faster,
depending on the benchmark. They also generate permutations in lexicographical order.

Unlike ImmutableClonableVector::permutations() which returns an iterator,
cloning the entire array each iteration, these methods mutate the vector in-place.
For that reason, these methods are much faster; between 35-55 times faster,
depending on the benchmark. They also generate permutations in lexicographical order.
@exscape
Copy link
Contributor Author

exscape commented Jun 3, 2014

This is almost identical to #13761, with the main difference being the renaming of MutableTotalOrdVector to MutableOrdVector, after the TotalOrd -> Ord and TotalEq -> Eq rename. That is, this could merge clearly, which the old request likely didn't.

Other changes are that two unnecessary muts were removed from the tests, and one mostly useless test was removed. I'll copy/paste the rest of that top comment, also, only editing the changed trait names etc:

Anyway. This adds two methods to mutable vectors (MutableOrdVector<T>): next_permutation() and prev_permutation().
They mutate the vector to the next/previous lexicographic permutation, in place.
The existing permutation functionality, ImmutableCloneableVector::permutations(), uses iterators and clones the entire vector each iteration, resulting in bad performance. For even a 10-element vector, that causes 10! = 3628800 calls to clone(), on the entire 10-element vector.
It also uses a swapping algorithm which does not generate lexicographic permutations; if you want lexicographical permutations, as far as I can tell, Rust currently cannot provide that.

So does Rust need this? I don't know. I wanted to use it, and realized it didn't exist, so I created it. Others may disagree about the necessity, of course.

Sample benchmarks:

test bench_iterated_permutations_one_iter      ... bench:     721 ns/iter (+/- 212)
test bench_lexicographic_permutations_one_iter ... bench:      14 ns/iter (+/- 1)
test bench_iterated_permutations_bulk          ... bench: 2449743 ns/iter (+/- 328460)
test bench_lexicographic_permutations_bulk     ... bench:   69704 ns/iter (+/- 2235)

51x faster per single iteration (one permutation per benchmark iteration), 35x faster to loop through all 5040 permutations of range(0,7) per benchmark iteration.

One possible (style-ish) issue with this code is the return types: they return true if successful, and false if the vector was already at the first/last permutation. This makes looping rather easy, but I'm not sure if it's considered "pretty", so to speak. However, since iterators are much too expensive, at least if returning a cloned vector each iteration is the only way to make them work, I found no better way to solve this, and at least IMHO it looks fine.
Also, I'm not sure if the comments inside the actual functions is considered excessive or not, but that is of course easily changed, if that would be the case.

bors added a commit that referenced this pull request Jun 4, 2014
Unlike ImmutableClonableVector::permutations() which returns an iterator,
cloning the entire array each iteration, these methods mutate the vector in-place.
For that reason, these methods are much faster; between 35-55 times faster,
depending on the benchmark. They also generate permutations in lexicographical order.
@bors bors closed this Jun 4, 2014
@bors bors merged commit 3b5d6fd into rust-lang:master Jun 4, 2014
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