-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add vector time complexity #121262
Add vector time complexity #121262
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cuviper (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
/// | ||
/// # Time complexity | ||
/// | ||
/// Takes *O*([`Vec::len`]) time. All items after the insertion index must be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Takes *O*([`Vec::len`]) time. All items after the insertion index must be | |
/// Takes at worst *O*(*n*) time. All items after the insertion index must be |
And perhaps specify that if index = n then it's O(1).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine -- Big O notation doesn't have to be precise, and this does represent the average (len/2) number of items that have to move, since constants like 1/2 are usually dropped.
Note that the collections module documentation already lists time complexities |
Copying is O(n)—not the memory allocation
library/alloc/src/vec/mod.rs
Outdated
/// # Time complexity | ||
/// | ||
/// Takes amortized *O*(1) time. If the vector's length would exceed its | ||
/// capacity after the push, the capacity is doubled by allocating |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While true for now, I don't think doubling is something we want to promise in documentation, just amortized growth. For example, some languages only double up to a certain size and then use a smaller growth factor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I'll make it more vague
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of this?
/// Takes amortized *O*(1) time. If the vector's length would exceed its
/// capacity after the push, *O*(*capacity*) time is taken to copy the
/// vector's elements to a larger allocation. This expensive operation is
/// offset by the *capacity* *O*(1) insertions it allows.
6c1847b
to
74151cb
Compare
@bors r+ rollup |
…y, r=cuviper Add vector time complexity Added time complexity for `Vec` methods `push`, `push_within_capacity`, `pop`, and `insert`. <details> <summary> Reference images </summary> ![`Vec::push` documentation](https://github.com/rust-lang/rust/assets/78604367/dc966bbd-e92e-45a6-af82-35afabfa79a9) ![`Vec::push_within_capacity` documentation](https://github.com/rust-lang/rust/assets/78604367/9aadaf48-46ed-4fad-bdd5-74b98a61f4bb) ![`Vec::pop` documentation](https://github.com/rust-lang/rust/assets/78604367/88ec0389-a346-4ea5-a3b7-17caf514dd8b) ![`Vec::insert` documentation](https://github.com/rust-lang/rust/assets/78604367/960c15c3-ef8e-4aa7-badc-35ce80f6f221) </details> I followed a convention to use `#Time complexity` that I found in [the `BinaryHeap` documentation](https://doc.rust-lang.org/std/collections/struct.BinaryHeap.html#time-complexity-1). Looking through the rest of standard library collections, there is not a consistent way to handle this. [`Vec::swap_remove`](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.swap_remove) does not have a dedicated section for time complexity but does list it. [`VecDeque::rotate_left`](https://doc.rust-lang.org/std/collections/struct.VecDeque.html#complexity) uses a `#complexity` heading.
Rollup of 15 pull requests Successful merges: - rust-lang#121065 (Add basic i18n guidance for `Display`) - rust-lang#121202 (Limit the number of names and values in check-cfg diagnostics) - rust-lang#121213 (Add an example to demonstrate how Rc::into_inner works) - rust-lang#121262 (Add vector time complexity) - rust-lang#121287 (Clarify/add `must_use` message for Rc/Arc/Weak::into_raw.) - rust-lang#121664 (Adjust error `yield`/`await` lowering) - rust-lang#121838 (Use the correct logic for nested impl trait in assoc types) - rust-lang#121860 (Add a tidy check that checks whether the fluent slugs only appear once) - rust-lang#121913 (Don't panic when waiting on poisoned queries) - rust-lang#121959 (Removing absolute path in proc-macro) - rust-lang#121975 (hir_analysis: enums return `None` in `find_field`) - rust-lang#121978 (Fix duplicated path in the "not found dylib" error) - rust-lang#121987 (pattern analysis: abort on arity mismatch) - rust-lang#121993 (Avoid using unnecessary queries when printing the query stack in panics) - rust-lang#121997 (interpret/cast: make more matches on FloatTy properly exhaustive) r? `@ghost` `@rustbot` modify labels: rollup
…y, r=cuviper Add vector time complexity Added time complexity for `Vec` methods `push`, `push_within_capacity`, `pop`, and `insert`. <details> <summary> Reference images </summary> ![`Vec::push` documentation](https://github.com/rust-lang/rust/assets/78604367/dc966bbd-e92e-45a6-af82-35afabfa79a9) ![`Vec::push_within_capacity` documentation](https://github.com/rust-lang/rust/assets/78604367/9aadaf48-46ed-4fad-bdd5-74b98a61f4bb) ![`Vec::pop` documentation](https://github.com/rust-lang/rust/assets/78604367/88ec0389-a346-4ea5-a3b7-17caf514dd8b) ![`Vec::insert` documentation](https://github.com/rust-lang/rust/assets/78604367/960c15c3-ef8e-4aa7-badc-35ce80f6f221) </details> I followed a convention to use `#Time complexity` that I found in [the `BinaryHeap` documentation](https://doc.rust-lang.org/std/collections/struct.BinaryHeap.html#time-complexity-1). Looking through the rest of standard library collections, there is not a consistent way to handle this. [`Vec::swap_remove`](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.swap_remove) does not have a dedicated section for time complexity but does list it. [`VecDeque::rotate_left`](https://doc.rust-lang.org/std/collections/struct.VecDeque.html#complexity) uses a `#complexity` heading.
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#121213 (Add an example to demonstrate how Rc::into_inner works) - rust-lang#121262 (Add vector time complexity) - rust-lang#121287 (Clarify/add `must_use` message for Rc/Arc/Weak::into_raw.) - rust-lang#121664 (Adjust error `yield`/`await` lowering) - rust-lang#121826 (Use root obligation on E0277 for some cases) - rust-lang#121838 (Use the correct logic for nested impl trait in assoc types) - rust-lang#121913 (Don't panic when waiting on poisoned queries) - rust-lang#121987 (pattern analysis: abort on arity mismatch) - rust-lang#121993 (Avoid using unnecessary queries when printing the query stack in panics) - rust-lang#121997 (interpret/cast: make more matches on FloatTy properly exhaustive) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#121262 - 20jasper:add-vector-time-complexity, r=cuviper Add vector time complexity Added time complexity for `Vec` methods `push`, `push_within_capacity`, `pop`, and `insert`. <details> <summary> Reference images </summary> ![`Vec::push` documentation](https://github.com/rust-lang/rust/assets/78604367/dc966bbd-e92e-45a6-af82-35afabfa79a9) ![`Vec::push_within_capacity` documentation](https://github.com/rust-lang/rust/assets/78604367/9aadaf48-46ed-4fad-bdd5-74b98a61f4bb) ![`Vec::pop` documentation](https://github.com/rust-lang/rust/assets/78604367/88ec0389-a346-4ea5-a3b7-17caf514dd8b) ![`Vec::insert` documentation](https://github.com/rust-lang/rust/assets/78604367/960c15c3-ef8e-4aa7-badc-35ce80f6f221) </details> I followed a convention to use `#Time complexity` that I found in [the `BinaryHeap` documentation](https://doc.rust-lang.org/std/collections/struct.BinaryHeap.html#time-complexity-1). Looking through the rest of standard library collections, there is not a consistent way to handle this. [`Vec::swap_remove`](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.swap_remove) does not have a dedicated section for time complexity but does list it. [`VecDeque::rotate_left`](https://doc.rust-lang.org/std/collections/struct.VecDeque.html#complexity) uses a `#complexity` heading.
Added time complexity for
Vec
methodspush
,push_within_capacity
,pop
, andinsert
.Reference images
I followed a convention to use
#Time complexity
that I found in theBinaryHeap
documentation. Looking through the rest of standard library collections, there is not a consistent way to handle this.Vec::swap_remove
does not have a dedicated section for time complexity but does list it.VecDeque::rotate_left
uses a#complexity
heading.