re-use self.get_all_attrs result for pass indirectly attribute by folkertdev · Pull Request #148600 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation11 Commits1 Checks12 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Could be a fix for a potential performance regression reported here #144529 (comment). Apparently the regression later disappeared. Nevertheless, this seems like a decent refactor.

r? @JonathanBrouwer (vaguely attribute-related, maybe there are other optimizations to that PR that we're missing)

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.

labels

Nov 6, 2025

This comment has been minimized.

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request

Nov 6, 2025

re-use self.get_all_attrs result for pass indirectly attribute

get_all_attrs is pretty fast so I can't really imagine this being a performance improvement, but I think this improves the code anyways so lets merge it
@bors r+ rollup

📌 Commit 1a9cc78 has been approved by JonathanBrouwer

It is now in the queue for this repository.

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

Nov 6, 2025

Does anything else in #144529 jump out to you?

Hmmm we do need to call pass_indirectly_in_non_rustic_abis on every argument, which does need to look through the flags, but I can't see that being significant and I can't see why it would specifically affect this benchmark.

I just took a look at the implementation of get_all_attrs, and it's just a btreemap lookup so it should not be that slow. I (and humans in general) are not good at predicting performance tho so lets let the numbers speak. We call get_all_attrs a ton of times throughout the compiler though so if that's slow we are leaving a lot of performance on the table.

Alternatively it could've just been noise, since the benchmark did just bounce back. I think it being noise is the most likely answer.

☀️ Try build successful (CI)
Build commit: 1869732 (18697328f3073a12811e91ab3af2d58b27d9689f, parent: c90bcb9571b7aab0d8beaa2ce8a998ffaf079d38)

This comment has been minimized.

Finished benchmarking commit (1869732): 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)

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

Cycles

Results (primary -2.4%, secondary -3.4%)

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

Binary size

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

Bootstrap: 475.185s -> 475.633s (0.09%)
Artifact size: 390.81 MiB -> 390.80 MiB (-0.00%)

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Nov 7, 2025

…ttrs, r=JonathanBrouwer

re-use self.get_all_attrs result for pass indirectly attribute

Could be a fix for a potential performance regression reported here rust-lang#144529 (comment). Apparently the regression later disappeared. Nevertheless, this seems like a decent refactor.

r? @JonathanBrouwer (vaguely attribute-related, maybe there are other optimizations to that PR that we're missing)

bors added a commit that referenced this pull request

Nov 7, 2025

Rollup of 7 pull requests

Successful merges:

• #145656 (Stabilize s390x vector target feature and is_s390x_feature_detected! macro)
• #148204 (Modify contributor email entries in .mailmap)
• #148556 (Fix suggestion for returning async closures)
• #148585 ([rustdoc] Replace print methods with functions to improve code readability)
• #148600 (re-use self.get_all_attrs result for pass indirectly attribute)
• #148612 (Add note for identifier with attempted hygiene violation)
• #148613 (Switch hexagon targets to rust-lld)

r? @ghost @rustbot modify labels: rollup

bors added a commit that referenced this pull request

Nov 7, 2025

Rollup of 7 pull requests

Successful merges:

• #145656 (Stabilize s390x vector target feature and is_s390x_feature_detected! macro)
• #148204 (Modify contributor email entries in .mailmap)
• #148556 (Fix suggestion for returning async closures)
• #148585 ([rustdoc] Replace print methods with functions to improve code readability)
• #148600 (re-use self.get_all_attrs result for pass indirectly attribute)
• #148612 (Add note for identifier with attempted hygiene violation)
• #148613 (Switch hexagon targets to rust-lld)

r? @ghost @rustbot modify labels: rollup

bors added a commit that referenced this pull request

Nov 8, 2025

Rollup of 7 pull requests

Successful merges:

• #145656 (Stabilize s390x vector target feature and is_s390x_feature_detected! macro)
• #148204 (Modify contributor email entries in .mailmap)
• #148556 (Fix suggestion for returning async closures)
• #148585 ([rustdoc] Replace print methods with functions to improve code readability)
• #148600 (re-use self.get_all_attrs result for pass indirectly attribute)
• #148612 (Add note for identifier with attempted hygiene violation)
• #148613 (Switch hexagon targets to rust-lld)

r? @ghost @rustbot modify labels: rollup

jhpratt added a commit to jhpratt/rust that referenced this pull request

Nov 8, 2025

…ttrs, r=JonathanBrouwer

re-use self.get_all_attrs result for pass indirectly attribute

Could be a fix for a potential performance regression reported here rust-lang#144529 (comment). Apparently the regression later disappeared. Nevertheless, this seems like a decent refactor.

r? @JonathanBrouwer (vaguely attribute-related, maybe there are other optimizations to that PR that we're missing)

bors added a commit that referenced this pull request

Nov 8, 2025

Rollup of 15 pull requests

Successful merges:

• #147404 (Fix issue with callsite inline attribute not being applied sometimes.)
• #147534 (Implement SIMD funnel shifts in const-eval/Miri)
• #147686 (update isolate_highest_one for NonZero)
• #148020 (Show backtrace on allocation failures when possible)
• #148204 (Modify contributor email entries in .mailmap)
• #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
• #148555 (Fix rust-by-example spanish translation)
• #148556 (Fix suggestion for returning async closures)
• #148585 ([rustdoc] Replace print methods with functions to improve code readability)
• #148600 (re-use self.get_all_attrs result for pass indirectly attribute)
• #148612 (Add note for identifier with attempted hygiene violation)
• #148613 (Switch hexagon targets to rust-lld)
• #148644 ([bootstrap] Make --open option work with doc src/tools/error_index_generator)
• #148649 (don't completely reset HeadUsages)
• #148675 (Remove eslint-js from npm dependencies)

r? @ghost @rustbot modify labels: rollup

jhpratt added a commit to jhpratt/rust that referenced this pull request

Nov 8, 2025

…ttrs, r=JonathanBrouwer

re-use self.get_all_attrs result for pass indirectly attribute

Could be a fix for a potential performance regression reported here rust-lang#144529 (comment). Apparently the regression later disappeared. Nevertheless, this seems like a decent refactor.

r? @JonathanBrouwer (vaguely attribute-related, maybe there are other optimizations to that PR that we're missing)

bors added a commit that referenced this pull request

Nov 8, 2025

Rollup of 16 pull requests

Successful merges:

• #147534 (Implement SIMD funnel shifts in const-eval/Miri)
• #147686 (update isolate_highest_one for NonZero)
• #148020 (Show backtrace on allocation failures when possible)
• #148204 (Modify contributor email entries in .mailmap)
• #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
• #148279 (rustc_builtin_macros: rename bench parameter to avoid collisions with user-defined function names)
• #148555 (Fix rust-by-example spanish translation)
• #148556 (Fix suggestion for returning async closures)
• #148585 ([rustdoc] Replace print methods with functions to improve code readability)
• #148600 (re-use self.get_all_attrs result for pass indirectly attribute)
• #148612 (Add note for identifier with attempted hygiene violation)
• #148613 (Switch hexagon targets to rust-lld)
• #148619 (Enable std locking functions on AIX)
• #148644 ([bootstrap] Make --open option work with doc src/tools/error_index_generator)
• #148649 (don't completely reset HeadUsages)
• #148675 (Remove eslint-js from npm dependencies)

r? @ghost @rustbot modify labels: rollup

bors added a commit that referenced this pull request

Nov 8, 2025

Rollup of 10 pull requests

Successful merges:

• #145656 (Stabilize s390x vector target feature and is_s390x_feature_detected! macro)
• #147024 (std_detect: Support run-time detection on OpenBSD using elf_aux_info)
• #147534 (Implement SIMD funnel shifts in const-eval/Miri)
• #147540 (Stabilise as_array in [_] and *const [_]; stabilise as_mut_array in [_] and *mut [_].)
• #147686 (update isolate_highest_one for NonZero)
• #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
• #148555 (Fix rust-by-example spanish translation)
• #148556 (Fix suggestion for returning async closures)
• #148585 ([rustdoc] Replace print methods with functions to improve code readability)
• #148600 (re-use self.get_all_attrs result for pass indirectly attribute)

r? @ghost @rustbot modify labels: rollup

bors added a commit that referenced this pull request

Nov 8, 2025

Rollup of 10 pull requests

Successful merges:

• #145656 (Stabilize s390x vector target feature and is_s390x_feature_detected! macro)
• #147024 (std_detect: Support run-time detection on OpenBSD using elf_aux_info)
• #147534 (Implement SIMD funnel shifts in const-eval/Miri)
• #147540 (Stabilise as_array in [_] and *const [_]; stabilise as_mut_array in [_] and *mut [_].)
• #147686 (update isolate_highest_one for NonZero)
• #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
• #148555 (Fix rust-by-example spanish translation)
• #148556 (Fix suggestion for returning async closures)
• #148585 ([rustdoc] Replace print methods with functions to improve code readability)
• #148600 (re-use self.get_all_attrs result for pass indirectly attribute)

r? @ghost @rustbot modify labels: rollup

bors added a commit that referenced this pull request

Nov 9, 2025

Rollup of 10 pull requests

Successful merges:

• #145656 (Stabilize s390x vector target feature and is_s390x_feature_detected! macro)
• #147024 (std_detect: Support run-time detection on OpenBSD using elf_aux_info)
• #147534 (Implement SIMD funnel shifts in const-eval/Miri)
• #147540 (Stabilise as_array in [_] and *const [_]; stabilise as_mut_array in [_] and *mut [_].)
• #147686 (update isolate_highest_one for NonZero)
• #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
• #148555 (Fix rust-by-example spanish translation)
• #148556 (Fix suggestion for returning async closures)
• #148585 ([rustdoc] Replace print methods with functions to improve code readability)
• #148600 (re-use self.get_all_attrs result for pass indirectly attribute)

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit that referenced this pull request

Nov 9, 2025

Rollup merge of #148600 - folkertdev:pass-indirectly-reuse-attrs, r=JonathanBrouwer

re-use self.get_all_attrs result for pass indirectly attribute

Could be a fix for a potential performance regression reported here #144529 (comment). Apparently the regression later disappeared. Nevertheless, this seems like a decent refactor.

r? @JonathanBrouwer (vaguely attribute-related, maybe there are other optimizations to that PR that we're missing)

github-actions bot pushed a commit to rust-lang/stdarch that referenced this pull request

Nov 10, 2025

github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request

Nov 10, 2025

github-actions bot pushed a commit to rust-lang/miri that referenced this pull request

Nov 10, 2025

Labels

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.