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

Simplify <Postorder as Iterator>::size_hint #137923

Merged
merged 2 commits into from
Mar 5, 2025

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Mar 3, 2025

The current version is sometimes malformed (cc #137919); let's see if we can get away with a loose but trivially-correct one.

The current version is wrong (cc 137919); let's see if we can get away with a loose but trivially-correct one.
@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
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

@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. labels Mar 3, 2025
@scottmcm
Copy link
Member Author

scottmcm commented Mar 3, 2025

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 3, 2025
@bors
Copy link
Contributor

bors commented Mar 3, 2025

⌛ Trying commit e403654 with merge 6b39be3...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2025
…<try>

Simplify `<Postorder as Iterator>::size_hint`

The current version is sometimes malformed (cc rust-lang#137919); let's see if we can get away with a loose but trivially-correct one.
@bors
Copy link
Contributor

bors commented Mar 3, 2025

☀️ Try build successful - checks-actions
Build commit: 6b39be3 (6b39be36e7a0a260f8df901767b518fb3204cff9)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6b39be3): comparison URL.

Overall result: no relevant changes - no action needed

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-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -3.1%, secondary -2.6%)

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

Cycles

Results (primary 1.4%, secondary -1.4%)

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

Binary size

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

Bootstrap: 772.867s -> 772.98s (0.01%)
Artifact size: 361.94 MiB -> 361.97 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 3, 2025
@tmiasko
Copy link
Contributor

tmiasko commented Mar 3, 2025

Nice catch!

Would you like to fix Preorder size_hint as well (worklist contains duplicates so it cannot be used as a lower bound)? r=me either way.

@scottmcm
Copy link
Member Author

scottmcm commented Mar 3, 2025

Oh, good point. Postorder's visit_stack always returns at least one item per entry, but Preorder checks visited in next. Will do.

@rustbot author

@rustbot rustbot 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 Mar 3, 2025
@scottmcm
Copy link
Member Author

scottmcm commented Mar 4, 2025

Let's check the change to preorder is also perf-neutral...
@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 4, 2025
@bors
Copy link
Contributor

bors commented Mar 4, 2025

⌛ Trying commit fe6cf34 with merge 6ee42d5...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 4, 2025
…<try>

Simplify `<Postorder as Iterator>::size_hint`

The current version is sometimes malformed (cc rust-lang#137919); let's see if we can get away with a loose but trivially-correct one.
@bors
Copy link
Contributor

bors commented Mar 4, 2025

☀️ Try build successful - checks-actions
Build commit: 6ee42d5 (6ee42d5c34b23457320714320583f281708d373b)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6ee42d5): comparison URL.

Overall result: no relevant changes - no action needed

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-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary 0.6%, secondary -2.9%)

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.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.9% [-2.9%, -2.9%] 2
All ❌✅ (primary) 0.6% [0.6%, 0.6%] 1

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: 771.965s -> 772.179s (0.03%)
Artifact size: 362.03 MiB -> 362.03 MiB (0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 4, 2025
@tmiasko
Copy link
Contributor

tmiasko commented Mar 4, 2025

r? tmiasko @bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 4, 2025

📌 Commit fe6cf34 has been approved by tmiasko

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 4, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 4, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 4, 2025
…r=tmiasko

Simplify `<Postorder as Iterator>::size_hint`

The current version is sometimes malformed (cc rust-lang#137919); let's see if we can get away with a loose but trivially-correct one.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
…kingjubilee

Rollup of 33 pull requests

Successful merges:

 - rust-lang#136581 (Retire the legacy `Makefile`-based `run-make` test infra)
 - rust-lang#136764 (Make `ptr_cast_add_auto_to_object` lint into hard error)
 - rust-lang#136798 (Added documentation for flushing per rust-lang#74348)
 - rust-lang#136865 (Perform deeper compiletest path normalization for `$TEST_BUILD_DIR` to account for compare-mode/debugger cases, and normalize long type file filename hashes)
 - rust-lang#136975 (Look for `python3` first on MacOS, not `py`)
 - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs)
 - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion)
 - rust-lang#137327 (Undeprecate env::home_dir)
 - rust-lang#137502 (Don't include global asm in `mir_keys`, fix error body synthesis)
 - rust-lang#137534 ([rustdoc] hide item that is not marked as doc(inline) and whose src is doc(hidden))
 - rust-lang#137565 (Try to point of macro expansion from resolver and method errors if it involves macro var)
 - rust-lang#137643 (Add DWARF test case for non-C-like `repr128` enums)
 - rust-lang#137758 (fix usage of ty decl macro fragments in attributes)
 - rust-lang#137764 (Ensure that negative auto impls are always applicable)
 - rust-lang#137772 (Fix char count in `Display` for `ByteStr`)
 - rust-lang#137798 (ci: use ubuntu 24 on arm large runner)
 - rust-lang#137805 (adjust Layout debug printing to match the internal field name)
 - rust-lang#137808 (Do not require that unsafe fields lack drop glue)
 - rust-lang#137820 (Clarify why InhabitedPredicate::instantiate_opt exists)
 - rust-lang#137825 (Provide more context on resolve error caused from incorrect RTN)
 - rust-lang#137829 (Stabilize [T]::split_off... methods)
 - rust-lang#137834 (rustc_fluent_macro: use CARGO_CRATE_NAME instead of CARGO_PKG_NAME)
 - rust-lang#137850 (Stabilize `box_uninit_write`)
 - rust-lang#137912 (Do not recover missing lifetime with random in-scope lifetime)
 - rust-lang#137913 (Allow struct field default values to reference struct's generics)
 - rust-lang#137923 (Simplify `<Postorder as Iterator>::size_hint`)
 - rust-lang#137949 (Update MSVC INSTALL.md instructions to recommend VS 2022 + recent Windows 10/11 SDK)
 - rust-lang#137963 (Add ``dyn`` keyword to `E0373` examples)
 - rust-lang#137975 (Remove unused `PpMode::needs_hir`)
 - rust-lang#137986 (Fix some typos)
 - rust-lang#137991 (Add `avr-none` to SUMMARY.md and platform-support.md)
 - rust-lang#137993 (Remove obsolete comment from DeduceReadOnly)
 - rust-lang#137996 (Revert "compiler/rustc_data_structures/src/sync/worker_local.rs: delete "unsafe impl Sync"")

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
…kingjubilee

Rollup of 15 pull requests

Successful merges:

 - rust-lang#137829 (Stabilize [T]::split_off... methods)
 - rust-lang#137850 (Stabilize `box_uninit_write`)
 - rust-lang#137912 (Do not recover missing lifetime with random in-scope lifetime)
 - rust-lang#137913 (Allow struct field default values to reference struct's generics)
 - rust-lang#137923 (Simplify `<Postorder as Iterator>::size_hint`)
 - rust-lang#137949 (Update MSVC INSTALL.md instructions to recommend VS 2022 + recent Windows 10/11 SDK)
 - rust-lang#137963 (Add ``dyn`` keyword to `E0373` examples)
 - rust-lang#137975 (Remove unused `PpMode::needs_hir`)
 - rust-lang#137981 (rustdoc search: increase strictness of typechecking)
 - rust-lang#137986 (Fix some typos)
 - rust-lang#137991 (Add `avr-none` to SUMMARY.md and platform-support.md)
 - rust-lang#137993 (Remove obsolete comment from DeduceReadOnly)
 - rust-lang#137996 (Revert "compiler/rustc_data_structures/src/sync/worker_local.rs: delete "unsafe impl Sync"")
 - rust-lang#138019 (Pretty-print `#[deprecated]` attribute in HIR.)
 - rust-lang#138026 (Make CrateItem::body() function return an option)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 81a4349 into rust-lang:master Mar 5, 2025
7 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 5, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
Rollup merge of rust-lang#137923 - scottmcm:fix-postorder-size-hint, r=tmiasko

Simplify `<Postorder as Iterator>::size_hint`

The current version is sometimes malformed (cc rust-lang#137919); let's see if we can get away with a loose but trivially-correct one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants