Skip to content

perf experiment for #147185 - #148243

Closed
RalfJung wants to merge 2 commits into
rust-lang:masterfrom
RalfJung:perf-transparent-checks
Closed

perf experiment for #147185#148243
RalfJung wants to merge 2 commits into
rust-lang:masterfrom
RalfJung:perf-transparent-checks

Conversation

@RalfJung

@RalfJungRalfJung commented Oct 29, 2025

Copy link
Copy Markdown
Member

Trying to figure out why #147185 regresses syn builds so much.

@rustbotrustbot 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. labels Oct 29, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@RalfJung
RalfJung marked this pull request as draft October 29, 2025 07:34
@rustbotrustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 29, 2025
@RalfJung

Copy link
Copy Markdown
MemberAuthor

@bors try
@rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-borsBot added a commit that referenced this pull request Oct 29, 2025
@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 29, 2025
@rust-bors

rust-borsBot commented Oct 29, 2025

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: f3ba3f5 (f3ba3f55843086cd3e7e91cfccec0c492804392d, parent: 907705abea355952d84ed39cea256ed4570bdda6)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (f3ba3f5): comparison URL.

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

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

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

Instruction count

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

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
0.4%[0.4%, 0.5%]4
Improvements ✅
(primary)
-1.7%[-2.9%, -0.3%]9
Improvements ✅
(secondary)
-1.0%[-1.0%, -1.0%]1
All ❌✅ (primary)-1.7%[-2.9%, -0.3%]9

Max RSS (memory usage)

Results (primary -2.8%)

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

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
-2.8%[-2.8%, -2.8%]1
Improvements ✅
(secondary)
--0
All ❌✅ (primary)-2.8%[-2.8%, -2.8%]1

Cycles

Results (primary 2.0%, secondary 0.1%)

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

meanrangecount
Regressions ❌
(primary)
2.0%[2.0%, 2.0%]1
Regressions ❌
(secondary)
4.1%[2.6%, 5.6%]2
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-3.8%[-4.9%, -2.7%]2
All ❌✅ (primary)2.0%[2.0%, 2.0%]1

Binary size

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

Bootstrap: 474.692s -> 475.969s (0.27%)
Artifact size: 390.07 MiB -> 390.07 MiB (-0.00%)

@rustbotrustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Oct 29, 2025
@RalfJung

Copy link
Copy Markdown
MemberAuthor

I have no explanation how this can make such a big difference for syn... even if syn has thousands of repr(transparent) types, this should hardly change anything as it just checks one more flag?

@RalfJung

Copy link
Copy Markdown
MemberAuthor

Cc @rust-lang/wg-compiler-performance in case someone has any idea how this could be happening.

@RalfJung
RalfJungforce-pushed the perf-transparent-checks branch from 134bed6 to dd40018CompareOctober 29, 2025 11:31
@RalfJung

Copy link
Copy Markdown
MemberAuthor

@bors try
@rust-timer queue

@rust-timer

This comment has been minimized.

rust-borsBot added a commit that referenced this pull request Oct 29, 2025
@rust-bors

This comment has been minimized.

@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 29, 2025
@rust-bors

rust-borsBot commented Oct 29, 2025

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 9fcd3c8 (9fcd3c810e6e2bb555bfca0af2cc081141a7a911, parent: c6d42d774d1edfb270b8faaefacc67c213b0260b)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (9fcd3c8): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

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

Instruction count

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

Max RSS (memory usage)

Results (primary 2.1%, secondary 1.1%)

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

meanrangecount
Regressions ❌
(primary)
2.1%[2.1%, 2.1%]1
Regressions ❌
(secondary)
3.1%[2.2%, 4.0%]2
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-3.0%[-3.0%, -3.0%]1
All ❌✅ (primary)2.1%[2.1%, 2.1%]1

Cycles

Results (primary 2.1%, secondary -0.4%)

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

meanrangecount
Regressions ❌
(primary)
2.1%[2.1%, 2.1%]1
Regressions ❌
(secondary)
2.3%[2.0%, 2.6%]2
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-3.1%[-3.4%, -2.7%]2
All ❌✅ (primary)2.1%[2.1%, 2.1%]1

Binary size

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

Bootstrap: 475.179s -> 475.205s (0.01%)
Artifact size: 390.26 MiB -> 390.27 MiB (0.00%)

@rustbotrustbot removed the perf-regression Performance regression. label Oct 29, 2025
@rustbotrustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 29, 2025
@RalfJung
RalfJungforce-pushed the perf-transparent-checks branch 2 times, most recently from ece2bd9 to 82b29a4CompareOctober 29, 2025 18:00
@RalfJung
RalfJungforce-pushed the perf-transparent-checks branch from 82b29a4 to 60765e3CompareOctober 29, 2025 18:01
@RalfJung

Copy link
Copy Markdown
MemberAuthor

@bors try
@rust-timer queue

@rust-timer

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

rust-borsBot added a commit that referenced this pull request Oct 29, 2025
@rust-bors

This comment has been minimized.

@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 29, 2025
@rust-bors

rust-borsBot commented Oct 29, 2025

Copy link
Copy Markdown
Contributor

💔 Test for 3a5a811 failed: CI

@Kobzol

Copy link
Copy Markdown
Member

https://perf.rust-lang.org/detailed-query.html?commit=f3ba3f55843086cd3e7e91cfccec0c492804392d&benchmark=syn-2.0.101-check&scenario=incr-unchanged&base_commit=907705abea355952d84ed39cea256ed4570bdda6 there were some query changes, maybe that could be it? Cachegrind diff looks kinda interesting:

< -39,350,149 ???:
-6,659,324 <&mut serde_json::ser::Serializer<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>> as serde_core::ser::Serializer>::serialize_str
-4,215,955 <std::io::buffered::bufwriter::BufWriter<std::io::stdio::Stderr> as std::io::Write>::write_all
-2,730,341 <rustc_errors::styled_buffer::StyledBuffer>::putc
-2,580,516 <rustc_span::source_map::SourceMap>::lookup_char_pos
-2,511,566 rustc_errors::emitter::normalize_whitespace
-2,492,928 <rustc_errors::styled_buffer::StyledBuffer>::render
-2,307,220 <rustc_span::SourceFile>::get_line
-1,407,502 <rustc_errors::emitter::HumanEmitter>::draw_line
-1,249,210 _rjem_je_arena_ralloc
-942,782 <rustc_errors::styled_buffer::StyledBuffer>::ensure_lines
-868,556 realloc
-818,220 core::str::count::do_count_chars
-803,060 <serde_json::ser::Compound<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>, serde_json::ser::CompactFormatter> as serde_core::ser::SerializeStruct>::serialize_field::<usize>
-733,739 free
-691,479 <rustc_errors::emitter::HumanEmitter>::emit_messages_default_inner::{closure#0}
-567,521 core::fmt::write
-544,679 rustc_incremental::persist::load::setup_dep_graph
-535,045 <<rustc_errors::json::Diagnostic>::from_errors_diagnostic::BufWriter as std::io::Write>::write
-510,903 malloc
-464,106 <rustc_errors::json::DiagnosticSpan as serde_core::ser::Serialize>::serialize::<&mut serde_json::ser::Serializer<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>>>
-422,919 <anstyle::style::Style>::fmt_to
-326,780 <hashbrown::raw::RawTable<((usize, rustc_data_structures::stable_hasher::HashingControls), rustc_data_structures::fingerprint::Fingerprint)>>::reserve_rehash::<hashbrown::map::make_hasher<(usize, rustc_data_structures::stable_hasher::HashingControls), rustc_data_structures::fingerprint::Fingerprint, rustc_hash::FxBuildHasher>::{closure#0}>
326,776 <hashbrown::raw::RawTable<((*const (), rustc_data_structures::stable_hasher::HashingControls), rustc_data_structures::fingerprint::Fingerprint)>>::reserve_rehash::<hashbrown::map::make_hasher<(*const (), rustc_data_structures::stable_hasher::HashingControls), rustc_data_structures::fingerprint::Fingerprint, rustc_hash::FxBuildHasher>::{closure#0}>
-303,372 <std::io::default_write_fmt::Adapter<<rustc_errors::json::Diagnostic>::from_errors_diagnostic::BufWriter> as core::fmt::Write>::write_str
-261,576 <rustc_errors::json::DiagnosticSpanLine>::from_span
-258,560 <rustc_errors::styled_buffer::StyledBuffer>::append
-229,844 <rustc_span::source_map::SourceMap>::span_to_lines
-217,550 core::fmt::Formatter::pad
-196,577 _rjem_je_arena_cache_bin_fill_small
-186,318 rustc_span::char_width
-165,353 <rustc_span::span_encoding::Span>::to
-154,852 rustc_errors::emitter::emit_to_destination
-129,414 _rjem_je_malloc_default
-104,344 <rustc_errors::json::DiagnosticSpan>::from_span_full::<core::iter::sources::from_fn::FromFn<<rustc_span::span_encoding::Span>::macro_backtrace::{closure#0}>>
-101,388 <anstyle::style::Style as core::fmt::Display>::fmt
-92,904 <<rustc_errors::json::Diagnostic>::from_errors_diagnostic::BufWriter as std::io::Write>::write_fmt
-90,530 <anstyle::color::DisplayBuffer>::write_str
-87,516 <rustc_span::source_map::SourceMap>::span_to_snippet
-85,554 rustc_span::str_width
-83,035 <alloc::string::String as core::fmt::Write>::write_str
-80,730 <serde_json::ser::Compound<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>, serde_json::ser::CompactFormatter> as serde_core::ser::SerializeStruct>::serialize_field::<u32>
-79,817 <alloc::raw_vec::RawVec<rustc_expand::mbe::TokenTree>>::grow_one
79,817 <alloc::raw_vec::RawVec<object::write::Symbol>>::grow_one
-66,234 <rustc_errors::snippet::Style>::anstyle
-55,212 <core::str::lossy::Utf8Chunks as core::iter::traits::iterator::Iterator>::next
-54,930 core::ptr::drop_in_place::<rustc_errors::json::DiagnosticSpan>
-54,173 <serde_json::ser::Compound<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>, serde_json::ser::CompactFormatter> as serde_core::ser::SerializeStruct>::serialize_field::<core::option::Option<alloc::string::String>>
-53,844 rustc_middle::ty::codec::encode_with_shorthand::<rustc_middle::query::on_disk_cache::CacheEncoder, rustc_middle::ty::Ty, <rustc_middle::query::on_disk_cache::CacheEncoder as rustc_middle::ty::codec::TyEncoder>::type_shorthands>
-53,628 <rustc_errors::emitter::HumanEmitter>::draw_line_num
-52,752 rustc_middle::lint::explain_lint_level_source
-50,079 <rustc_errors::emitter::HumanEmitter as rustc_errors::emitter::Emitter>::emit_diagnostic
-49,110 _rjem_je_tcache_bin_flush_small
-46,860 <core::iter::sources::from_fn::FromFn<<rustc_span::span_encoding::Span>::macro_backtrace::{closure#0}> as core::iter::traits::iterator::Iterator>::next
-45,249 <rustc_errors::emitter::FileWithAnnotatedLines>::collect_annotations::add_annotation_to_file
-44,424 <rustc_span::source_map::SourceMap>::span_until_char
-42,623 <core::str::iter::SplitInternal<char>>::next
< -1,618,401 ./string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:__memcpy_avx_unaligned_erms
< -47,730 ./nptl/./nptl/pthread_mutex_trylock.c:pthread_mutex_trylock@@GLIBC_2.34

Maybe there are a bunch of new warnings emitted after your PR?

warning: `syn` (lib) generated 19 warnings (run `cargo fix --lib -p syn` to apply 19 suggestions)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.09s

vs

warning: `syn` (lib) generated 67 warnings (45 duplicates) (run `cargo fix --lib -p syn` to apply 22 suggestions)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.28s

@RalfJung

Copy link
Copy Markdown
MemberAuthor

The query changes looked entirely insignificant to me... or maybe I just don't know how to read that page.

Maybe there are a bunch of new warnings emitted after your PR?

Where did you get that output from?

We did crater this, but maybe this is an ancient version of syn that still triggers the lint. The PR also changed repr_transparent_external_private_fields to report-in-deps, after all.

@Kobzol

Copy link
Copy Markdown
Member

I just ran cargo build with the previous/new compiler version on https://github.com/rust-lang/rustc-perf/tree/master/collector/compile-benchmarks/syn-2.0.101.

@Kobzol

Copy link
Copy Markdown
Member

The syn version is 6 months old, for reference, so not exactly ancient.

@RalfJung

Copy link
Copy Markdown
MemberAuthor

Ah, on the latest nightly we are getting

warning: lint `repr_transparent_external_private_fields` has been renamed to `repr_transparent_non_zst_fields`
--> src/token.rs:146:28
|
146 | #[allow(unknown_lints, repr_transparent_external_private_fields)] // False positive: https://github.com/rust-lang/rust/issues/78586#issuecomment-1722680482
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `repr_transparent_non_zst_fields`
|
= note: `#[warn(renamed_and_removed_lints)]` on by default
warning: lint `repr_transparent_external_private_fields` has been renamed to `repr_transparent_non_zst_fields`
--> src/token.rs:327:36
|
327 | #[allow(unknown_lints, repr_transparent_external_private_fields)] // False positive: https://github.com/rust-lang/rust/issues/78586#issuecomment-1722680482
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `repr_transparent_non_zst_fields`
...
521 | / define_punctuation_structs! {
522 | | "_" pub struct Underscore/1 /// wildcard patterns, inferred types, unnamed items in constants, extern crates, use declarations, and destructuring assignment
523 | | }
| |_- in this macro invocation
|
= note: this warning originates in the macro `define_punctuation_structs` (in Nightly builds, run with -Z macro-backtrace for more info)
warning: lint `repr_transparent_external_private_fields` has been renamed to `repr_transparent_non_zst_fields`
--> src/token.rs:327:36
|
327 | #[allow(unknown_lints, repr_transparent_external_private_fields)] // False positive: https://github.com/rust-lang/rust/issues/78586#issuecomment-1722680482
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `repr_transparent_non_zst_fields`

And I guess what we are seeing is the effort of rendering those lints.

Good catch, thanks!

@rustbotrustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 29, 2025
@Kobzol

Copy link
Copy Markdown
Member

Yeah, the diagnostics emitting code is ludicrously slow.

@RalfJung
RalfJung deleted the perf-transparent-checks branch October 29, 2025 19:17
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 18, 2025
…=nnethercote
repr(transparent) check: do not compute check_unsuited more than once
`field_infos` is an iterator that we execute multiple times. However, we usually ignore the `unsuited` field -- we only need it in the last iteration. So move the computation of that field to that iteration to avoid computing it multiple times. Computing `unsuited` involves a recursive traversal over the types of all non-trivial fields, so there can be non-trivial amounts of work here.
(I benchmarked this in rust-lang#148243 and saw no changes, probably because we don't have a benchmark with many repr(transparent) types. But still, computing this each time just seemed silly.)
rust-timer added a commit that referenced this pull request Nov 19, 2025
Rollup merge of #148281 - RalfJung:repr-transparent-check, r=nnethercote
repr(transparent) check: do not compute check_unsuited more than once
`field_infos` is an iterator that we execute multiple times. However, we usually ignore the `unsuited` field -- we only need it in the last iteration. So move the computation of that field to that iteration to avoid computing it multiple times. Computing `unsuited` involves a recursive traversal over the types of all non-trivial fields, so there can be non-trivial amounts of work here.
(I benchmarked this in #148243 and saw no changes, probably because we don't have a benchmark with many repr(transparent) types. But still, computing this each time just seemed silly.)
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Nov 19, 2025
repr(transparent) check: do not compute check_unsuited more than once
`field_infos` is an iterator that we execute multiple times. However, we usually ignore the `unsuited` field -- we only need it in the last iteration. So move the computation of that field to that iteration to avoid computing it multiple times. Computing `unsuited` involves a recursive traversal over the types of all non-trivial fields, so there can be non-trivial amounts of work here.
(I benchmarked this in rust-lang/rust#148243 and saw no changes, probably because we don't have a benchmark with many repr(transparent) types. But still, computing this each time just seemed silly.)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-perfStatus: Waiting on a perf run to be completed.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@RalfJung@rustbot@rust-timer@Kobzol@BoxyUwU