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 8 pull requests #122182

Merged
merged 35 commits into from
Mar 8, 2024
Merged

Rollup of 8 pull requests #122182

merged 35 commits into from
Mar 8, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

haydonryan and others added 30 commits December 11, 2023 11:41
This makes more sense because most cases then second one is unwind target.
Some implementations of `Write::write_vectored` in the standard
library (`BufWriter`, `LineWriter`, `Stdout`, `Stderr`) check all
buffers to calculate the total length. This is O(n) over the number of
buffers.

It's common that only a limited number of buffers is written at a
time (e.g. 1024 for `writev(2)`). `write_vectored_all` will then call
`write_vectored` repeatedly, leading to a runtime of O(n²) over the
number of buffers.

The fix is to only calculate as much as needed if it's needed.
Improve std::fs::read_to_string example

Resolves  [rust-lang#118621](rust-lang#118621)

For the original code to succeed it requires address.txt to contain a socketaddress, however it is much easier to follow if this is just any strong - eg address could be a street address or just text.

Also changed the variable name from "foo" to something more meaningful as cargo clippy warns you against using foo as a placeholder.

```
$ cat main.rs
use std::fs;
use std::error::Error;

fn main() -> Result<(), Box<dyn Error>> {
    let addr: String = fs::read_to_string("address.txt")?.parse()?;
    println!("{}", addr);
    Ok(())
}

$ cat address.txt
123 rusty lane
san francisco 94999

$ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `/home/haydon/workspace/rust-test-pr/tester/target/debug/tester`
123 rusty lane
san francisco 94999

```
Add asm goto support to `asm!`

Tracking issue: rust-lang#119364

This PR implements asm-goto support, using the syntax described in "future possibilities" section of [RFC2873](https://rust-lang.github.io/rfcs/2873-inline-asm.html#asm-goto).

Currently I have only implemented the `label` part, not the `fallthrough` part (i.e. fallthrough is implicit). This doesn't reduce the expressive though, since you can use label-break to get arbitrary control flow or simply set a value and rely on jump threading optimisation to get the desired control flow. I can add that later if deemed necessary.

r? ``@Amanieu``
cc ``@ojeda``
@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 8, 2024
@bors
Copy link
Contributor

bors commented Mar 8, 2024

⌛ Testing commit 0d235ef with merge 1b2c53a...

@bors
Copy link
Contributor

bors commented Mar 8, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 1b2c53a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 8, 2024
@bors bors merged commit 1b2c53a into rust-lang:master Mar 8, 2024
12 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 8, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#118623 Improve std::fs::read_to_string example 0991649e36c15a622b33510b9eded2483a9bb09e (link)
#119365 Add asm goto support to asm! d842784a20f023cdccb35a6bac977c7d753e2c26 (link)
#120608 Docs for std::ptr::slice_from_raw_parts c4b0dfcf352d14958daf4f4d8dfefb3a4912ebc8 (link)
#121832 Add new Tier-3 target: loongarch64-unknown-linux-musl 2b522bdc17c21f208dfb70fbd7970012f8c7b1d5 (link)
#121938 Fix quadratic behavior of repeated vectored writes 1b4481e7fd73791bba4b01f2c6603c54d6171229 (link)
#122099 Add #[inline] to BTreeMap::new constructor 495b677058446e9065c089c48d19c859f7eaf71f (link)
#122103 Make TAITs and ATPITs capture late-bound lifetimes in scope bdb95761f0ffe2cfc8865affe5f4c73054e7c47c (link)
#122143 PassWrapper: update for llvm/llvm-project@a3319371970b 25432ca0fbb5a8b491bce1accddf92731739042a (link)

previous master: 14fbc3c005

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1b2c53a): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.3% [1.3%, 1.3%] 1
Regressions ❌
(secondary)
0.8% [0.2%, 1.3%] 2
Improvements ✅
(primary)
-0.3% [-0.7%, -0.2%] 17
Improvements ✅
(secondary)
-0.5% [-0.9%, -0.2%] 15
All ❌✅ (primary) -0.3% [-0.7%, 1.3%] 18

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
8.5% [8.5%, 8.5%] 1
Regressions ❌
(secondary)
5.7% [5.7%, 5.7%] 1
Improvements ✅
(primary)
-4.1% [-4.1%, -4.1%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.2% [-4.1%, 8.5%] 2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.4% [1.4%, 1.4%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.4% [1.4%, 1.4%] 1

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.2% [0.1%, 0.4%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.0% [-0.0%, -0.0%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [-0.0%, 0.4%] 5

Bootstrap: 648.483s -> 652.903s (0.68%)
Artifact size: 172.46 MiB -> 172.55 MiB (0.05%)

@rustbot rustbot added the perf-regression Performance regression. label Mar 8, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Mar 8, 2024

The bootstrap regression looks real.

@rust-timer build 495b677

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (495b677): comparison URL.

Overall result: ❌ regressions - no action needed

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
5.6% [5.6%, 5.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 648.483s -> 651.069s (0.40%)
Artifact size: 172.46 MiB -> 172.47 MiB (0.00%)

@pnkfelix
Copy link
Member

pnkfelix commented Mar 12, 2024

Visiting for weekly perf triage

@matthiaskrgr
Copy link
Member Author

to be honest I am not completely unamused by the fact that "our build took 5 seconds longer" is considered a serious regression :D

@rust-timer build bdb9576

@rust-timer

This comment has been minimized.

@Kobzol
Copy link
Contributor

Kobzol commented Mar 12, 2024

It's not necessarily about the regression itself (I'd personally OK it), just that it would be nice to know what has caused it (if it is real), so that we can have a better idea about what effects do various code changes have on the compilation time of rustc.

@nnethercote
Copy link
Contributor

Plus rustc is the only benchmark we have that is not just a single crate, so it's interesting in that way.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (bdb9576): comparison URL.

Overall result: ❌ regressions - no action needed

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
6.1% [6.1%, 6.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 648.483s -> 648.725s (0.04%)
Artifact size: 172.46 MiB -> 310.26 MiB (79.90%)

@matthiaskrgr
Copy link
Member Author

🤔
image

@Kobzol
Copy link
Contributor

Kobzol commented Mar 12, 2024

That's just LLVM artifact size tracking working again. The unrolled build was created too soon, so it still contained an old master merge as a reference, which didn't have the artifact recorded.

@pnkfelix
Copy link
Member

It's not necessarily about the regression itself (I'd personally OK it), just that it would be nice to know what has caused it (if it is real), so that we can have a better idea about what effects do various code changes have on the compilation time of rustc.

marking as triaged

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Mar 13, 2024
@matthiaskrgr matthiaskrgr deleted the rollup-gzimi4c branch March 16, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.