Skip to content

Conversation

RalfJung
Copy link
Member

I saw O(n m log n) in the docs and found that really hard to parse. In particular, I don't think we should use blank space as syntax for both multiplication and function calls, that is just confusing.

This PR makes both multiplication and function calls explicit using Rust-like syntax. If you prefer, I can also leave one of them implicit, but I believe explicit is better here.

While I was at it I also added backticks consistently.

@rust-highfive
Copy link
Contributor

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 15, 2020
@@ -644,7 +643,7 @@ impl<T: Ord> BinaryHeap<T> {
/// The remaining elements will be removed on drop in heap order.
///
/// Note:
/// * `.drain_sorted()` is O(n lg n); much slower than `.drain()`.
/// * `.drain_sorted()` is `O(n * lg(n))`; much slower than `.drain()`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what lg is supposed to mean here. Is it log with a typo, or is it supposed to indicate a particular base?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the only place it is used here, i assume it is a typo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.math10.com/en/algebra/logarithm-log-ln-lg.html suggests that lg == log base 10

https://mathworld.wolfram.com/Lg.html suggests it might mean log base 2

Copy link
Member Author

@RalfJung RalfJung Apr 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean anyway the difference between base 2 and base 10 is a constant factor, so in the context of big-O the base just doesn't matter... but then there are two places in the docs that explicitly give a base in big-O.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I changed this here to log and removed the two explicitly given bases.

@@ -40,7 +40,7 @@ use UnderflowResult::*;
/// performance on *small* nodes of elements which are cheap to compare. However in the future we
/// would like to further explore choosing the optimal search strategy based on the choice of B,
/// and possibly other factors. Using linear search, searching for a random element is expected
/// to take O(B log<sub>B</sub>n) comparisons, which is generally worse than a BST. In practice,
/// to take O(B * log<sub>B</sub>(n)) comparisons, which is generally worse than a BST. In practice,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Elsewhere we are using log_2 to indicate the base, so this is inconsistent. But for now I decided not to change this.

@shepmaster
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 17, 2020

📌 Commit 818bef5 has been approved by shepmaster

@bors 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 Apr 17, 2020
@shepmaster
Copy link
Member

Thanks!

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2020
Rollup of 6 pull requests

Successful merges:

 - rust-lang#70467 (Use `call` instead of `invoke` for functions that cannot unwind )
 - rust-lang#71070 (rustbuild: Remove stage 0 LLD flavor workaround for MSVC)
 - rust-lang#71167 (big-O notation: parenthesis for function calls, explicit multiplication)
 - rust-lang#71238 (Miri: fix typo)
 - rust-lang#71242 (Format Mailmap To Work With GitHub)
 - rust-lang#71243 (Account for use of `try!()` in 2018 edition and guide users in the right direction)

Failed merges:

r? @ghost
@bors bors merged commit d5d9bf0 into rust-lang:master Apr 18, 2020
@RalfJung RalfJung deleted the big-o branch April 18, 2020 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants