-
Notifications
You must be signed in to change notification settings - Fork 14k
Don't delegate to default implementations of ExactSizeIterator
#149384
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
base: main
Are you sure you want to change the base?
Don't delegate to default implementations of ExactSizeIterator
#149384
Conversation
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
…pl, r=<try> Don't delegate to default implementations of `ExactSizeIterator`
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (ccdc19f): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking 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 Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -3.3%, secondary -2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -1.2%, secondary -0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 470.335s -> 470.278s (-0.01%) |
|
Putting this up for review. Perf is mostly neutral (secondary benchmark that regresses looks bimodal?), so not sure at all whether it's worth the effort and complexity that this adds. OTOH, maybe code outside of rustc uses I don't mind just abandoning this, or maybe whittling it down to only the very straight forward changes. |
The default
ExactSizeIteratorcallsself.size_hint()and then performs an assertion that the size hint's lower and higher bounds are equal, which is wasteful.I'm sure the compiler/LLVM can elide that assertion in most cases (e.g. when the
size_hintimpl returns something like(n, Some(n)), but it still might affect inlining decisions, and maybe even cause LLVM to have to work harder.There are a lot of changes here, some of which are not as clear-cut as others, so first I wanna see if this has any perf impact.