Add vector time complexity #121262

20jasper · 2024-02-18T10:49:46Z

Added time complexity for Vec methods push, push_within_capacity, pop, and insert.

Reference images

I followed a convention to use #Time complexity that I found in the BinaryHeap 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.

rustbot · 2024-02-18T10:49:53Z

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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

ShE3py · 2024-02-18T11:56:19Z

library/alloc/src/vec/mod.rs

+    ///
+    /// # Time complexity
+    ///
+    /// Takes *O*([`Vec::len`]) time. All items after the insertion index must be


Suggested change

/// 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).

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.

library/alloc/src/vec/mod.rs

the8472 · 2024-02-18T13:22:01Z

Note that the collections module documentation already lists time complexities

Copying is O(n)—not the memory allocation

cuviper · 2024-02-25T02:53:12Z

library/alloc/src/vec/mod.rs

+    /// # 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


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.

Good point! I'll make it more vague

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.

cuviper · 2024-03-04T22:37:47Z

@bors r+ rollup

bors · 2024-03-04T22:37:50Z

📌 Commit 74151cb has been approved by cuviper

It is now in the queue for this repository.

…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.

20jasper added 5 commits February 18, 2024 05:21

time complexity for push

cb8ce9d

time complexity for push_within_capacity

bb6dca0

time complexity for pop

0a5d684

time complexity for insert

d2f825f

intradoc link for vec

ef1a584

rustbot assigned cuviper Feb 18, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 18, 2024

fix typo in push documentation

a9cfeb3

20jasper marked this pull request as ready for review February 18, 2024 11:32

ShE3py reviewed Feb 18, 2024

View reviewed changes

Noratrieb reviewed Feb 18, 2024

View reviewed changes

library/alloc/src/vec/mod.rs Outdated Show resolved Hide resolved

Fix error in push docs

bc52e5d

Copying is O(n)—not the memory allocation

cuviper reviewed Feb 25, 2024

View reviewed changes

Make push docs more vague

74151cb

20jasper force-pushed the add-vector-time-complexity branch from 6c1847b to 74151cb Compare February 25, 2024 07:43

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2024

jhpratt mentioned this pull request Mar 5, 2024

Rollup of 15 pull requests #122008

Closed

This was referenced Mar 5, 2024

Rollup of 10 pull requests #122011

Closed

Rollup of 10 pull requests #122012

Merged

bors merged commit 22827fd into rust-lang:master Mar 5, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Mar 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add vector time complexity #121262

Add vector time complexity #121262

20jasper commented Feb 18, 2024 •

edited

Loading

rustbot commented Feb 18, 2024

ShE3py Feb 18, 2024 •

edited

Loading

cuviper Mar 4, 2024

the8472 commented Feb 18, 2024

cuviper Feb 25, 2024

20jasper Feb 25, 2024

20jasper Feb 25, 2024 •

edited

Loading

cuviper commented Mar 4, 2024

bors commented Mar 4, 2024

	/// 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

Add vector time complexity #121262

Add vector time complexity #121262

Conversation

20jasper commented Feb 18, 2024 • edited Loading

rustbot commented Feb 18, 2024

ShE3py Feb 18, 2024 • edited Loading

Choose a reason for hiding this comment

cuviper Mar 4, 2024

Choose a reason for hiding this comment

the8472 commented Feb 18, 2024

cuviper Feb 25, 2024

Choose a reason for hiding this comment

20jasper Feb 25, 2024

Choose a reason for hiding this comment

20jasper Feb 25, 2024 • edited Loading

Choose a reason for hiding this comment

cuviper commented Mar 4, 2024

bors commented Mar 4, 2024

20jasper commented Feb 18, 2024 •

edited

Loading

ShE3py Feb 18, 2024 •

edited

Loading

20jasper Feb 25, 2024 •

edited

Loading