Uh oh!
There was an error while loading. Please reload this page.
Deque: fix an assertion, speed up and big cleanup - #7562
Merged
Conversation
bluss
commented
Jul 5, 2013
ContributorAuthor
Ok it failed again in run::tests::test_inherit_env I think that's a spurious fail.. http://buildbot.rust-lang.org/builders/auto-mac-32-opt/builds/261/steps/test/logs/stdio I want to change the FromIterator impl anyway, so this PR can wait |
added 4 commits
July 6, 2013 05:42
Without this, it will hit the assert in fn grow after 32 consecutive add_front.
Add a test that excercises deque growing. Add bench tests for grow, new, add_back, add_front, to expose how slow these functions are.
Fix some issues with the deque being very slow, keep the same vec around instead of constructing a new. Move as few elements as possible, so the self.lo point is not moved after grow. [o o o o o|o o o] hi...^ ^.... lo grows to [. . . . .|o o o o o o o o|. . .] ^.. lo ^.. hi If the deque is append-only, it will result in moving no elements on grow. If the deque is prepend-only, all will be moved each time. The bench tests added show big improvements: Timed using `rust build -O --test extra.rs && ./extra --bench deque` Old version: test deque::tests::bench_add_back ... bench: 4976 ns/iter (+/- 9) test deque::tests::bench_add_front ... bench: 4108 ns/iter (+/- 18) test deque::tests::bench_grow ... bench: 416964 ns/iter (+/- 4197) test deque::tests::bench_new ... bench: 408 ns/iter (+/- 12) With this commit: test deque::tests::bench_add_back ... bench: 12 ns/iter (+/- 0) test deque::tests::bench_add_front ... bench: 16 ns/iter (+/- 0) test deque::tests::bench_grow ... bench: 1515 ns/iter (+/- 30) test deque::tests::bench_new ... bench: 419 ns/iter (+/- 3)
So that deque can be used with IteratorUtil::collect()
bluss
commented
Jul 6, 2013
ContributorAuthor
Updated. .reserve() remains to do, as well as deciding on method names, smaller representation, and smaller bits like Clone (Clone and Eq now done). |
added 7 commits
July 6, 2013 07:26
The deque is determined by vec self.elts.len(), self.nelts, and self.lo, and self.hi is calculated from these. self.hi is just the raw index of element number `self.nelts`
The previous implementation of reverse iterators used modulus (%) of negative indices, which did work but was fragile due to dependency on the divisor.
The deque is split at the marker lo, or logical index 0. Move the shortest part (split by lo) when growing. This way add_front is just as fast as add_back, on average.
We need a reasonably small initial capacity to make Deques faster for the common case.
bluss
commented
Jul 6, 2013
ContributorAuthor
Ready for merge. Feedback appreciated. |
bors added a commit
that referenced
this pull request
Jul 6, 2013
Fix an assertion in grow when using add_front. Speeds up grow (and the functions that use it) by a lot. We retain the vector instead of creating it anew for each grow. The struct field hi is removed since it is redundant, and all iterators are updated to use a representation closer to the Deque itself, it should make it work even if deque sizes are not powers of two. Deque::with_capacity is also implemented, but .reserve() is not yet done.
flip1995 pushed a commit
to flip1995/rust
that referenced
this pull request
Sep 3, 2021
…rednet Fix false positive on `filter_next` fixesrust-lang#7561 changelog: Fix false positive on [`filter_next`] when a method does not implement `Iterator`
U007D pushed a commit
to U007D/rust-mos
that referenced
this pull request
Aug 21, 2026
7562: add `generate_enum_match` assist r=matklad a=yoshuawuyts
This adds a `generate_enum_match` assist, which generates `is_` variants for enums (e.g. `Option::{is_none,is_some}` in std). This is my first attempt at contributing to Rust-Analyzer, so I'm not sure if I've gotten everything right. Thanks!
## Example
**Input**
```rust
pub(crate) enum Variant {
Undefined,
Minor, // cursor here
Major,
}
```
**Output**
```rust
pub(crate) enum Variant {
Undefined,
Minor,
Major,
}
impl Variant {
pub(crate) fn is_minor(&self) -> bool {
matches!(self, Self::Minor)
}
}
```
## Future Directions
I made this as a stepping stone for some of the more involved refactors (e.g. rust-lang#5944). I'm not sure yet how to create, use, and test `window.showQuickPick`-based asssists in RA. But once that's possible, it'd probably be nice to be able to generate match methods in bulk through the quickpick UI rather than one-by-one:
```
[x] Select enum members to generate methods for. (3 selected) [ OK ]
---------------------------------------------------------------------------
[x] Undefined
[x] Minor
[x] Major
```
Co-authored-by: Yoshua Wuyts <yoshuawuyts+github@gmail.com>
Co-authored-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix an assertion in grow when using add_front.
Speeds up grow (and the functions that use it) by a lot. We retain the vector instead of creating it anew for each grow.
The struct field hi is removed since it is redundant, and all iterators are updated to use a representation closer to the Deque itself, it should make it work even if deque sizes are not powers of two.
Deque::with_capacity is also implemented, but .reserve() is not yet done.