rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
93.93k stars 12.09k forks source link

Inline Duration construction into `Duration::from_{secs,millis,micros,nanos}` #125232

Closed coolreader18 closed 2 weeks ago

coolreader18 commented 2 weeks ago

The millis/micros/nanos cases I don't feel as strongly about, but I see no reason why Duration::from_secs should call into Duration::new - that's just creating unnecessary work for the inlining and DCE passes.

rustbot commented 2 weeks ago

r? @jhpratt

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

jhpratt commented 2 weeks ago

I prefer this change for clarity even if there is no performance impact. I don't foresee a negative performance change from this, so I'm going to merge it.

@bors r+ rollup=iffy

bors commented 2 weeks ago

:pushpin: Commit 53b317710da997fe5e8e852ffbe45f3a3d7af35c has been approved by jhpratt

It is now in the queue for this repository.

bors commented 2 weeks ago

:hourglass: Testing commit 53b317710da997fe5e8e852ffbe45f3a3d7af35c with merge 94be5ab448f9bd0209e3ec20fc355643772da7d0...

bors commented 2 weeks ago

:sunny: Test successful - checks-actions Approved by: jhpratt Pushing 94be5ab448f9bd0209e3ec20fc355643772da7d0 to master...

rust-timer commented 2 weeks ago

Finished benchmarking commit (94be5ab448f9bd0209e3ec20fc355643772da7d0): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

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

Cycles

Results (secondary 1.5%) 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) | 3.5% | [2.0%, 6.3%] | 5 | | Improvements ✅
(primary) | - | - | 0 | | Improvements ✅
(secondary) | -3.4% | [-4.1%, -2.8%] | 2 | | All ❌✅ (primary) | - | - | 0 |

Binary size

Results (primary -0.0%) 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.0% | [0.0%, 0.0%] | 2 | | Regressions ❌
(secondary) | - | - | 0 | | Improvements ✅
(primary) | -0.1% | [-0.1%, -0.1%] | 3 | | Improvements ✅
(secondary) | - | - | 0 | | All ❌✅ (primary) | -0.0% | [-0.1%, 0.0%] | 5 |

Bootstrap: 668.38s -> 668.437s (0.01%) Artifact size: 316.17 MiB -> 316.01 MiB (-0.05%)