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 13 pull requests #100426

Merged
merged 36 commits into from
Aug 12, 2022
Merged

Rollup of 13 pull requests #100426

merged 36 commits into from
Aug 12, 2022

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

RalfJung and others added 30 commits July 20, 2022 10:22
Co-authored-by: Marco Colombo <mar.colombo13@gmail.com>
This is a more ambitious version of rust-lang#98716.
It still changes the shebang back to python3, for compatibility with non-Unix systems,
but also adds alternative entrypoints for systems without `python3` installed.

These scripts will be necessary for the rust entrypoint (rust-lang#94829), so I see
little downside in adding them early.
To get around the "following path contains more than 968 entries, you
should move the test to some relevant subdirectory" tidy error.
Split render_with_highlighting, which took many optional parameters, into three
functions for specific purposes, which each take a smaller number of mostly
required parameters.

Remove some plumbing to pass through an "edition" parameter, which was used
solely to avoid highlighting some 2021 Edition keywords in non-2021 code.
Resolves all of issue rust-lang#93240

Reproduces a similar change as rust-lang#99086, but with improvements

In particular, this PR inlcludes:
* redesigning the crate-search selector so the background color matches its surroundings
* decrease the font of the dropdown menu to a reaonable size
* add a hover effect
* make the color of the arrow theme-dependent, using a surrounding div, with :after pseudo-element
  that can then be transformed using CSS filters to approximate the desired color
* fix the text "in" to match the title font
* remove the "for xyz" in the "Results for xyz in [All crates]" title when
  searching for search term "xyz"; you can already see what you're searching for
  as it's typed in the search bar!
* in line with rust-lang#99086, handle super-long crate names appropriately without a long <select>
  element escaping the screen area; the improvement is that we also keep the title
  within a single line now; uses some flex layout shenanigans...
* the margins / paddings are adjusted so the selected label of the <select> fits within
  the rest of that title nicely; also some inconsistency in the way that Firefox renders
  a <select> with "appearance: none" (roughly 4px more padding left and right of the text
  than e.g. Chrome) is worked around, and it now produces a result that looks (essentially)
  identical to Chrome
* the color of the help menu and settings menu border in light theme is made to match with
  the color of the corresponding buttons, like they do (match) in the ayu theme
* the casing of "All crates" changes to "all crates"
* the new tests from rust-lang#99086 are temporarily disabled, until they can be adapted later
Fix oversight duplicate property left in CSS (dark theme).

Improve wording in comment that mentions `appearance: none`
Previously the item-info background colors were too bright for a dark
theme, making a bright rectangle that draws the attention.
Co-authored-by: Frank Steffahn <fdsteffahn@gmail.com>
Don't add C runtime or set dynamic linker when linking with clang for
Fuchsia. Clang already does this for us.
It has four arguments that are never used. This avoids lots of argument
passing in functions that feed into `visit_variant_data`.
It's passed three arguments that are never used.
It is passed an argument that is never used.
It is passed an argument that is never used.
Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com>
…eGomez

rustdoc: simplify highlight.rs

Split render_with_highlighting, which took many optional parameters, into three functions for specific purposes, which each take a smaller number of mostly required parameters.

Remove some plumbing to pass through an "edition" parameter, which was used solely to avoid highlighting some 2021 Edition keywords in non-2021 code.

I've tested a build of std docs before and after, and this does not change the generated HTML at all.

Followup from rust-lang#91264 (comment)

r? ```@GuillaumeGomez```
Fix flags when using clang as linker for Fuchsia

Don't add C runtime or set dynamic linker when linking with clang for
Fuchsia. Clang already does this for us.
make raw_eq precondition more restrictive

Specifically, don't allow comparing pointers that way. Comparing pointers is subtle because you have to talk about what happens to the provenance.

This matches what [Miri already implements](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=9eb1dfb8a61b5a2d4a7cee43df2717af), and all existing users are fine with this.

If raw_eq on pointers is ever desired, we can adjust the intrinsic spec and Miri implementation as needed, but for now that seems just unnecessary. Also, this is a const intrinsic, and in const, comparing pointers this way is *not possible* -- so if we allow the intrinsic to compare pointers in general, we need to impose an extra restrictions saying that in const-context, pointers are *not* okay.
…crum

Add `x.sh` and `x.ps1` shell scripts

This is a more ambitious version of rust-lang#98716.
It still changes the x.py shebang back to python3, for compatibility with non-Unix systems,
but also adds alternative entrypoints for systems without `python3` installed.

These scripts will be necessary for the rust entrypoint (rust-lang#94829), so I see
little downside in adding them early.

I'll update the dev-guide to suggest using these instead of x.py once this is merged.

Fixes rust-lang#98650

r? `@Mark-Simulacrum` cc `@dtolnay` `@CAD97` `@yoshuawuyts`
…-ou-se

Fix test: chunks_mut_are_send_and_sync

Follow-up to rust-lang#100023 to make the test actually effective
@bors bors merged commit 2ed0f29 into rust-lang:master Aug 12, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 12, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2ed0f29): comparison url.

Instruction count

  • Primary benchmarks: mixed results
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions ❌
(primary)
0.3% 0.4% 4
Regressions ❌
(secondary)
0.5% 0.7% 5
Improvements ✅
(primary)
-0.9% -0.9% 1
Improvements ✅
(secondary)
-1.8% -1.8% 1
All ❌✅ (primary) 0.0% -0.9% 5

Max RSS (memory usage)

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

Cycles

Results
  • Primary benchmarks: ❌ relevant regression found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions ❌
(primary)
2.1% 2.1% 1
Regressions ❌
(secondary)
4.9% 5.9% 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.0% -3.0% 2
All ❌✅ (primary) 2.1% 2.1% 1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@rustbot rustbot added the perf-regression Performance regression. label Aug 12, 2022
@rylev
Copy link
Member

rylev commented Aug 12, 2022

Going to do perf runs for a few of these:

@rust-timer build 5832462aa1d9373b24ace96ad2c50b7a18af9952 (#100307)
@rust-timer build 23936af287657fa4148aeab40cc2a780810fae52 (#100392)

@rust-timer
Copy link
Collaborator

Queued 5832462aa1d9373b24ace96ad2c50b7a18af9952 with parent 20ffea6, future comparison URL.

@rylev
Copy link
Member

rylev commented Aug 12, 2022

If anyone is curious why this only kicked off one build: rust-lang/rustc-perf#1396

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5832462aa1d9373b24ace96ad2c50b7a18af9952): comparison url.

Instruction count

  • Primary benchmarks: ❌ relevant regressions found
  • Secondary benchmarks: ❌ relevant regression found
mean1 max count2
Regressions ❌
(primary)
0.2% 0.2% 2
Regressions ❌
(secondary)
0.6% 0.6% 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% 0.2% 2

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: ✅ relevant improvement found
mean1 max count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.9% -2.9% 1
All ❌✅ (primary) - - 0

Cycles

Results
  • Primary benchmarks: ❌ relevant regressions found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions ❌
(primary)
2.8% 3.0% 2
Regressions ❌
(secondary)
2.8% 3.2% 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.7% -2.7% 1
All ❌✅ (primary) 2.8% 3.0% 2

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rylev
Copy link
Member

rylev commented Aug 12, 2022

And now the other one:

@rust-timer build 23936af287657fa4148aeab40cc2a780810fae52 (#100392)

@rust-timer
Copy link
Collaborator

Queued 23936af287657fa4148aeab40cc2a780810fae52 with parent 20ffea6, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (23936af287657fa4148aeab40cc2a780810fae52): comparison url.

Instruction count

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

Max RSS (memory usage)

Results
  • Primary benchmarks: ✅ relevant improvement found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.6% -2.6% 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.6% -2.6% 1

Cycles

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: ✅ relevant improvements found
mean1 max count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.1% -3.3% 2
All ❌✅ (primary) - - 0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed perf-regression Performance regression. labels Aug 12, 2022
@rylev rylev added the perf-regression Performance regression. label Aug 12, 2022
@rylev
Copy link
Member

rylev commented Aug 12, 2022

And one last one (as I finally noticed that most of the regressions were in rustdoc):

@rust-timer build f370320e21804276c98109442f99598af33d7cce (#99337)

@rust-timer
Copy link
Collaborator

Queued f370320e21804276c98109442f99598af33d7cce with parent 20ffea6, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f370320e21804276c98109442f99598af33d7cce): comparison url.

Instruction count

  • Primary benchmarks: ❌ relevant regressions found
  • Secondary benchmarks: ❌ relevant regressions found
mean1 max count2
Regressions ❌
(primary)
0.3% 0.4% 8
Regressions ❌
(secondary)
0.4% 0.7% 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% 0.4% 8

Max RSS (memory usage)

Results
  • Primary benchmarks: ✅ relevant improvement found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.5% 3.5% 1
Improvements ✅
(primary)
-4.6% -4.6% 1
Improvements ✅
(secondary)
-2.9% -2.9% 1
All ❌✅ (primary) -4.6% -4.6% 1

Cycles

Results
  • Primary benchmarks: ❌ relevant regression found
  • Secondary benchmarks: ✅ relevant improvement found
mean1 max count2
Regressions ❌
(primary)
2.5% 2.5% 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.1% -3.1% 1
All ❌✅ (primary) 2.5% 2.5% 1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rylev
Copy link
Member

rylev commented Aug 12, 2022

Definitely looks like #99337 is the cause of these perf regressions. We normally have a bit more tolerance for rustdoc perf regressions and these numbers are by no means large.

cc @jsha @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

I'm surprised there is an impact on performance at all. But it was a code improvement change so I think it's still worth it and we expect that it will be reverted with the changes we're currently doing on this part of rustdoc.

@Mark-Simulacrum Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Aug 16, 2022
@Mark-Simulacrum
Copy link
Member

Marking as triaged. I think the regression is within the bounds we don't need to track the fix closely, though it's great that it sounds like there will be work that may improve things in this area.

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. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

internal error: entered unreachable code: in literal form when lowering mac args eq