Skip to content

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Jun 20, 2025

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

deven and others added 24 commits June 11, 2025 00:27
Implements `trim_prefix` and `trim_suffix` methods for both `slice` and
`str` types which remove at most one occurrence of a prefix/suffix while
always returning a string/slice (rather than Option), enabling easy
method chaining.
Be more consistent with the otherwise top-down organization of this
file.
Introduce `MacroTcbCtx` that holds everything relevant to transcription.
This allows for the following changes:

* Split `transcribe_sequence` and `transcribe_metavar` out of the
  heavily nested `transcribe`
* Split `metavar_expr_concat` out of `transcribe_metavar_expr`

This is a nonfunctional change.
We only called it it one place, which isn't generic and can be replaced
with a field access.
Add `trim_prefix` and `trim_suffix` methods for both `slice` and `str` types.

Implements `trim_prefix` and `trim_suffix` methods for both `slice` and `str` types, which remove at most one occurrence of a prefix/suffix while always returning a string/slice (rather than Option), enabling easy method chaining.

## Tracking issue
rust-lang#142312

## API
```rust
impl str {
    pub fn trim_prefix<P: Pattern>(&self, prefix: P) -> &str;
    pub fn trim_suffix<P: Pattern>(&self, suffix: P) -> &str
    where
        for<'a> P::Searcher<'a>: ReverseSearcher<'a>;
}

impl<T> [T] {
    pub fn trim_prefix<P: SlicePattern<Item = T> + ?Sized>(&self, prefix: &P) -> &[T]
    where
        T: PartialEq;
    pub fn trim_suffix<P: SlicePattern<Item = T> + ?Sized>(&self, suffix: &P) -> &[T]
    where
        T: PartialEq;
}
```

## Examples
```rust
// Method chaining
assert_eq!(" <https://example.com/> ".trim().trim_prefix('<').trim_suffix('>').trim(), "https://example.com/");

// Slices
let v = &[10, 40, 30];
assert_eq!(v.trim_prefix(&[10]), &[40, 30][..]);
```

## ACP
Originally proposed in rust-lang/libs-team#597
Rework #[cold] attribute parser

r? `@oli-obk`
…docs, r=oli-obk

Fix missing docs in `rustc_attr_parsing`
… r=oli-obk

Better template for `#[repr]` attributes
…ailure, r=lolbinarycat

Fix random failure when JS code is executed when the whole file was not read yet

Very randomly (and rarely), when I arrived on a page with `?search=something` in the URL, I got this error:

![Screenshot From 2025-06-14 11-27-46](https://github.com/user-attachments/assets/4b61b067-4e80-49c1-9a45-cff1509bf86a)

Moving the `initSearch` function at the bottom to ensure everything has been loaded fixes the issue.

PS: Sorry for the noise. Pushed to the wrong branch and rust-lang#142496 closed. ><
Ensure copy* intrinsics also perform the static self-init checks

fixes rust-lang#142532

r? `@RalfJung`
…petrochenkov

Refactor Translator

My main motivation was to simplify the usage of `SilentEmitter` for users like rustfmt. A few refactoring opportunities arose along the way.

* Replace `Translate` trait with `Translator` struct
* Replace `Emitter: Translate` with `Emitter::translator`
* Split `SilentEmitter` into `FatalOnlyEmitter` and `SilentEmitter`
…r=petrochenkov

mbe: Refactor transcription

Introduce `MacroTcbCtx` that holds everything relevant to transcription. This allows for the following changes:

* Split `transcribe_sequence` and `transcribe_metavar` out of the heavily nested `transcribe`
* Split `metavar_expr_concat` out of `transcribe_metavar_expr`

This is a nonfunctional change.
…aumeGomez

rustdoc: Remove `FormatRenderer::cache`

We only called it it one place, which isn't generic and can be replaced with a field access.
@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-rustdoc-json Area: Rustdoc JSON backend A-rustdoc-search Area: Rustdoc's search feature A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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. labels Jun 20, 2025
@tgross35
Copy link
Contributor Author

Ah you answered right before me, don't mind me either then 😆

@bors
Copy link
Collaborator

bors commented Jun 20, 2025

⌛ Testing commit 61f4918 with merge 15c701f...

@bors
Copy link
Collaborator

bors commented Jun 21, 2025

☀️ Test successful - checks-actions
Approved by: tgross35
Pushing 15c701f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 21, 2025
@bors bors merged commit 15c701f into rust-lang:master Jun 21, 2025
11 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 21, 2025
@tgross35 tgross35 deleted the rollup-iae7okj branch June 21, 2025 02:06
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#142331 Add trim_prefix and trim_suffix methods for both `slice… 79ed30102bd770b06326970da3b4878abbddf95b (link)
#142491 Rework #[cold] attribute parser 8b18a200eeeda37682c24943b879cbaf143edc20 (link)
#142494 Fix missing docs in rustc_attr_parsing 2228a644b92354dcf92d1721fa0d8323a6b6843c (link)
#142495 Better template for #[repr] attributes 9ed520219e1601b9a934fad72587556e9bffa24a (link)
#142497 Fix random failure when JS code is executed when the whole … c78a1d814221fe60327ddba3834b766481dcac2b (link)
#142575 Ensure copy* intrinsics also perform the static self-init c… 543e757cbf569b43fe5482931286df0a8e9ab298 (link)
#142650 Refactor Translator 62dadd2f9b7510ff67bcb3f6485d5bdf5f744959 (link)
#142713 mbe: Refactor transcription 7c928508c11816b71001f101aba6b5ffa0beca87 (link)
#142755 rustdoc: Remove FormatRenderer::cache 5024179adaf907aa6792c12cced0877466f370c4 (link)

previous master: 5526a2f47c

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

Copy link
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 5526a2f (parent) -> 15c701f (this PR)

Test differences

Show 184 test diffs

Stage 1

  • [ui] tests/ui/attributes/expected-word.rs: [missing] -> pass (J0)
  • [ui] tests/ui/statics/read_before_init.rs: [missing] -> pass (J0)

Stage 2

  • [ui] tests/ui/attributes/expected-word.rs: [missing] -> pass (J1)
  • [ui] tests/ui/statics/read_before_init.rs: [missing] -> pass (J1)

Additionally, 180 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 15c701fbc995eb6c5b3a86021c18185f8eee020d --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-apple-1: 8266.9s -> 6243.9s (-24.5%)
  2. dist-aarch64-apple: 6044.5s -> 4719.8s (-21.9%)
  3. dist-apple-various: 6674.5s -> 5442.3s (-18.5%)
  4. mingw-check-1: 1573.0s -> 1861.0s (18.3%)
  5. x86_64-msvc-1: 8357.9s -> 9655.2s (15.5%)
  6. aarch64-gnu-debug: 3447.4s -> 3960.6s (14.9%)
  7. x86_64-rust-for-linux: 2544.7s -> 2910.3s (14.4%)
  8. i686-gnu-2: 5339.8s -> 6029.5s (12.9%)
  9. i686-gnu-nopt-1: 7091.4s -> 7915.6s (11.6%)
  10. mingw-check-2: 1838.9s -> 2052.2s (11.6%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (15c701f): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
2.9% [2.9%, 2.9%] 1
Regressions ❌
(secondary)
0.8% [0.2%, 1.2%] 10
Improvements ✅
(primary)
-0.2% [-0.2%, -0.2%] 4
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.4% [-0.2%, 2.9%] 5

Max RSS (memory usage)

Results (primary -3.0%, secondary 2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
5.2% [4.6%, 5.9%] 2
Improvements ✅
(primary)
-3.0% [-3.0%, -3.0%] 1
Improvements ✅
(secondary)
-2.0% [-2.0%, -2.0%] 1
All ❌✅ (primary) -3.0% [-3.0%, -3.0%] 1

Cycles

Results (primary 2.8%, secondary 0.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)
2.0% [2.0%, 2.1%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) 2.8% [2.8%, 2.8%] 1

Binary size

Results (primary 1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Bootstrap: 692.994s -> 690.617s (-0.34%)
Artifact size: 371.93 MiB -> 371.83 MiB (-0.02%)

@rustbot rustbot added the perf-regression Performance regression. label Jun 21, 2025
@tgross35
Copy link
Contributor Author

Good chance it's #142713 based on regressions in macro expansion

@tgross35
Copy link
Contributor Author

^ The above has identical regression results

@rylev
Copy link
Member

rylev commented Jun 24, 2025

@tgross35 any thoughts on how to win back the performance lost here?

@tgross35
Copy link
Contributor Author

I have been doing some experiments at #142815, but unfortunately haven't had any luck. I'm going to keep trying a few things but am a bit pessimistic here - the PR was only refactoring, which is making me think that using early return from smaller functions is just getting us worse codegen than the huge match statement.

@tgross35
Copy link
Contributor Author

tgross35 commented Jun 25, 2025

I still don't really know how to interpret perf results well, but is it possible the clap_derive regression is a bit spurious? The graphs on the diff seem to indicate more time spent in the backend, which doesn't make any sense for a frontend-only refactoring, and it seems to have come back down from the blip here https://perf.rust-lang.org/index.html?start=2025-06-20&end=2025-06-21&benchmark=clap_derive-4.5.32&profile=opt&scenario=full&stat=instructions%3Au&kind=raw.

The tt muncher benchmarks seem more legit, and make sense with the change. I'd like to recommend triaging as accepted here; ignoring Clap, this is only a pretty small regression on a heavily recursive macro. Unfortunately I'm not having any luck getting perf back since #142815 isn't really successful.

Still trying one last thing out at #142990.

@panstromek
Copy link
Contributor

Yea, clap_derive is bimodal, that regression is spurious.

The tt_muncher isn't and it's a bit suspicous when it's caused by a refactor. Whether to accept or not I guess depends on what's the purpose of that PR (I don't have any context for that). I assume it's laying the groundwork for some feature? If that's the case, it seems fine (we're talking about ~10ms on pretty sensitive benchmarks here IIUC).

@tgross35
Copy link
Contributor Author

Whether to accept or not I guess depends on what's the purpose of that PR (I don't have any context for that). I assume it's laying the groundwork for some feature?

Indeed; #142713 can stand on its own as a cleanup of messy code, but I have some more significant changes to metavariable expressions based on it. Since #[inline(always)] didn't seem to make any difference, my guess is that LLVM is having some trouble optimizing through the context struct everything got collected to. Which is pretty unfortunate, but I don't think there is a clean way to work around it (short of reverting).

@panstromek
Copy link
Contributor

Ok. If you plan to make more changes in this area, then the perf impact will probably change anyway, so I think we can accept this and keep an eye on perf for future changes.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jun 25, 2025
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 26, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang#142331 (Add `trim_prefix` and `trim_suffix` methods for both `slice` and `str` types.)
 - rust-lang#142491 (Rework #[cold] attribute parser)
 - rust-lang#142494 (Fix missing docs in `rustc_attr_parsing`)
 - rust-lang#142495 (Better template for `#[repr]` attributes)
 - rust-lang#142497 (Fix random failure when JS code is executed when the whole file was not read yet)
 - rust-lang#142575 (Ensure copy* intrinsics also perform the static self-init checks)
 - rust-lang#142650 (Refactor Translator)
 - rust-lang#142713 (mbe: Refactor transcription)
 - rust-lang#142755 (rustdoc: Remove `FormatRenderer::cache`)

r? `@ghost`
`@rustbot` modify labels: rollup
tautschnig pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 3, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang#142331 (Add `trim_prefix` and `trim_suffix` methods for both `slice` and `str` types.)
 - rust-lang#142491 (Rework #[cold] attribute parser)
 - rust-lang#142494 (Fix missing docs in `rustc_attr_parsing`)
 - rust-lang#142495 (Better template for `#[repr]` attributes)
 - rust-lang#142497 (Fix random failure when JS code is executed when the whole file was not read yet)
 - rust-lang#142575 (Ensure copy* intrinsics also perform the static self-init checks)
 - rust-lang#142650 (Refactor Translator)
 - rust-lang#142713 (mbe: Refactor transcription)
 - rust-lang#142755 (rustdoc: Remove `FormatRenderer::cache`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-rustdoc-json Area: Rustdoc JSON backend A-rustdoc-search Area: Rustdoc's search feature A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.