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 4 pull requests #124716

Merged
merged 10 commits into from
May 4, 2024
Merged

Rollup of 4 pull requests #124716

merged 10 commits into from
May 4, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

a1phyr and others added 10 commits April 12, 2024 09:43
This moves some code around and adds some documentation comments to make
it easier to understand what's going on with the entrypoint logic, which
is a bit complicated.

The only change in behavior is consolidating the error messages for
unix_sigpipe to make the code slightly simpler.
…Denton

Improve several `Read` implementations

- `read_to_end` and `read_to_string` for `Cursor`
- Error on OOM in `read_to_string` of `&[u8]` and `VecDeque<u8>`
- Avoid making the slices contiguous in `VecDeque::read_to_string`
- ~`read_exact` and (unstable) `read_buf_exact` for `Take`~
- ~`read_buf` for `UnixStream` and `&UnixStream`~ (moved to rust-lang#123084)
- `read_to_end` for `ChildStdErr`
Various improvements to entrypoint code

This moves some code around and adds some documentation comments to make it easier to understand what's going on with the entrypoint logic, which is a bit complicated.

The only change in behavior is consolidating the error messages for unix_sigpipe to make the code slightly simpler.
…_use_unchecked, r=Nilstrieb

Use `unchecked_sub` in `split_at`

LLVM currently isn't figuring it out on its own, even in the checked version where it hypothetically could.

Before: <https://rust.godbolt.org/z/PEY38YrKs>
```llvm
bb1:                                              ; preds = %start
  %4 = getelementptr inbounds float, ptr %x.0, i64 %n
  %5 = sub i64 %x.1, %n
```

After:
```llvm
bb1:                                              ; preds = %start
  %4 = getelementptr inbounds float, ptr %x.0, i64 %n
  %5 = sub nuw i64 %x.1, %n
```

This is not using the wrapper because there's already a ubcheck covering it, so I don't want this to get a second one once rust-lang#121571 lands.

---

This is basically the same as rust-lang#108763, since `split_at` is essentially doing two `get_unchecked`s.
…piler-errors

interpret, miri: uniform treatments of intrinsics/functions with and without return block

A long time ago we didn't have a `dest: &MPlaceTy<'tcx, Self::Provenance>` for diverging functions, and since `dest` is used so often we special-cased these non-returning intrinsics and functions so that we'd have `dest` available everywhere else. But this has changed a while ago, now only the return block `ret` is optional, and there's a convenient `return_to_block` function for dealing with the `None` case.

So there no longer is any reason to treat diverging intrinsics/functions any different from those that do return.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. rollup A PR which is a rollup labels May 4, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=4

@bors
Copy link
Contributor

bors commented May 4, 2024

📌 Commit 743be1e has been approved by matthiaskrgr

It is now in the queue for this repository.

@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 May 4, 2024
@bors
Copy link
Contributor

bors commented May 4, 2024

⌛ Testing commit 743be1e with merge d568423...

@bors
Copy link
Contributor

bors commented May 4, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 4, 2024
@bors bors merged commit d568423 into rust-lang:master May 4, 2024
7 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 4, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#122441 Improve several Read implementations 996721e716d467447f329ee08abebc593718249e (link)
#124584 Various improvements to entrypoint code 6f03a50e4a408ea064943e39a015237a3451757a (link)
#124699 Use unchecked_sub in split_at 748881c603de94d45d86dfc3b51d0703f1631c11 (link)
#124715 interpret, miri: uniform treatments of intrinsics/functions… 9aeafa0df8a203eb5260356e1a0d350bbd736aa4 (link)

previous master: 1a851da73c

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 (d568423): 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)
- - 0
Regressions ❌
(secondary)
0.3% [0.3%, 0.5%] 6
Improvements ✅
(primary)
-0.8% [-0.8%, -0.8%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.8% [-0.8%, -0.8%] 1

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)
2.5% [2.5%, 2.5%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-5.2% [-5.2%, -5.2%] 1
Improvements ✅
(secondary)
-2.7% [-2.7%, -2.7%] 1
All ❌✅ (primary) -1.4% [-5.2%, 2.5%] 2

Cycles

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

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.2%, 0.2%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.2% [-0.5%, -0.0%] 6
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-0.5%, 0.2%] 9

Bootstrap: 674.198s -> 674.414s (0.03%)
Artifact size: 315.96 MiB -> 315.90 MiB (-0.02%)

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

Kobzol commented May 5, 2024

@rust-timer build 6f03a50

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6f03a50): 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.4% [0.3%, 0.5%] 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

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

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: 674.198s -> 674.958s (0.11%)
Artifact size: 315.96 MiB -> 315.94 MiB (-0.01%)

@Kobzol
Copy link
Contributor

Kobzol commented May 6, 2024

Looks like #124584 caused the small regressions (they seem small enough though).

@pnkfelix
Copy link
Member

pnkfelix commented May 7, 2024

Visiting for perf triage

  • all regressions are secondary (specifically on unused-warnings benchmark)
  • regression identified by Kobzol as caused by PR #124584 "Various improvements to entrypoint code"
  • seems like noise to pnkfelix

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label May 7, 2024
@matthiaskrgr matthiaskrgr deleted the rollup-ni58ie1 branch September 1, 2024 17:35
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.

10 participants