Skip to content
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

Rollup of 11 pull requests #49406

Merged
merged 37 commits into from
Mar 28, 2018
Merged

Rollup of 11 pull requests #49406

merged 37 commits into from
Mar 28, 2018

Conversation

varkor and others added 30 commits March 16, 2018 13:57
The other methods from `UnicodeStr` are already stable inherent
methods on str, but these have not been included.
Add slice::sort_by_cached_key as a memoised sort_by_key

At present, `slice::sort_by_key` calls its key function twice for each comparison that is made. When the key function is expensive (which can often be the case when `sort_by_key` is chosen over `sort_by`), this can lead to very suboptimal behaviour.

To address this, I've introduced a new slice method, `sort_by_cached_key`, which has identical semantic behaviour to `sort_by_key`, except that it guarantees the key function will only be called once per element.

Where there are `n` elements and the key function is `O(m)`:
- `slice::sort_by_cached_key` time complexity is `O(m n log m n)`, compared to `slice::sort_by_key`'s `O(m n + n log n)`.
- `slice::sort_by_cached_key` space complexity remains at `O(n + m)`. (Technically, it now reserves a slice of size `n`, whereas before it reserved a slice of size `n/2`.)

`slice::sort_unstable_by_key` has not been given an analogue, as it is important that unstable sorts are in-place, which is not a property that is guaranteed here. However, this also means that `slice::sort_unstable_by_key` is likely to be slower than `slice::sort_by_cached_key` when the key function does not have negligible complexity. We might want to explore this trade-off further in the future.

Benchmarks (for a vector of 100 `i32`s):
```
# Lexicographic: `|x| x.to_string()`
test bench_sort_by_key ... bench:      112,638 ns/iter (+/- 19,563)
test bench_sort_by_cached_key ... bench:       15,038 ns/iter (+/- 4,814)

# Identity: `|x| *x`
test bench_sort_by_key ... bench:        1,346 ns/iter (+/- 238)
test bench_sort_by_cached_key ... bench:        1,839 ns/iter (+/- 765)

# Power: `|x| x.pow(31)`
test bench_sort_by_key ... bench:        3,624 ns/iter (+/- 738)
test bench_sort_by_cached_key ... bench:        1,997 ns/iter (+/- 311)

# Abs: `|x| x.abs()`
test bench_sort_by_key ... bench:        1,546 ns/iter (+/- 174)
test bench_sort_by_cached_key ... bench:        1,668 ns/iter (+/- 790)
```
(So it seems functions that are single operations do perform slightly worse with this method, but for pretty much any more complex key, you're better off with this optimisation.)

I've definitely found myself using expensive keys in the past and wishing this optimisation was made (e.g. for rust-lang#47415). This feels like both desirable and expected behaviour, at the small cost of slightly more stack allocation and minute degradation in performance for extremely trivial keys.

Resolves rust-lang#34447.
…E0599, r=cramertj

Propose a variant if it is an enum for E0599

Fixes rust-lang#49192.
…QuietMisdreavus

Fix impl assoc constant link not working

Fixes rust-lang#49323.

r? @QuietMisdreavus
Fix pretty-printing for raw identifiers
Add is_whitespace and is_alphanumeric to str.

The other methods from `UnicodeStr` are already stable inherent
methods on str, but these have not been included.

r? @SimonSapin
libsyntax: Remove obsolete.rs

This little piece of infra is obsolete (ha-ha) and is unlikely to be used in the future, even if new obsolete syntax appears.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 27, 2018
@kennytm
Copy link
Member Author

kennytm commented Mar 27, 2018

@bors r+ p=8

@bors
Copy link
Contributor

bors commented Mar 27, 2018

📌 Commit b4bc2b0 has been approved by kennytm

@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 Mar 27, 2018
@kennytm kennytm changed the title Rollup of 8 pull requests Rollup of 9 pull requests Mar 27, 2018
Update compiler-rt with fix for 32bit iOS ARM
@kennytm
Copy link
Member Author

kennytm commented Mar 27, 2018

@bors r+

Added #49417.

@bors
Copy link
Contributor

bors commented Mar 27, 2018

📌 Commit 0873974 has been approved by kennytm

lukaslueg and others added 3 commits March 27, 2018 20:56
The current link is a 404, just link to the main repo page
Update CONTRIBUTING.md

The current link is a 404, just link to the main repo page
@kennytm kennytm changed the title Rollup of 9 pull requests Rollup of 11 pull requests Mar 27, 2018
@kennytm
Copy link
Member Author

kennytm commented Mar 27, 2018

@bors r+

Added #49202, #49426

@bors
Copy link
Contributor

bors commented Mar 27, 2018

📌 Commit 605ea7c has been approved by kennytm

@bors
Copy link
Contributor

bors commented Mar 27, 2018

⌛ Testing commit 605ea7c with merge 35f368f...

bors added a commit that referenced this pull request Mar 27, 2018
Rollup of 11 pull requests

- Successful merges: #48639, #49223, #49333, #49369, #49381, #49395, #49399, #49401, #49417, #49202, #49426
- Failed merges:
@bors
Copy link
Contributor

bors commented Mar 27, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 27, 2018
@kennytm
Copy link
Member Author

kennytm commented Mar 27, 2018

@bors retry

A macOS job is stuck at downloading the sccache binary.

@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 Mar 27, 2018
@bors
Copy link
Contributor

bors commented Mar 28, 2018

⌛ Testing commit 605ea7c with merge 59ec9bf...

bors added a commit that referenced this pull request Mar 28, 2018
Rollup of 11 pull requests

- Successful merges: #48639, #49223, #49333, #49369, #49381, #49395, #49399, #49401, #49417, #49202, #49426
- Failed merges:
@bors
Copy link
Contributor

bors commented Mar 28, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: kennytm
Pushing 59ec9bf to master...

@bors bors merged commit 605ea7c into rust-lang:master Mar 28, 2018
@Centril Centril added the rollup A PR which is a rollup label Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup 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.