Skip to content

Avoid unnecessary seconds conversion before to_i#10979

Merged
aduth merged 2 commits intomainfrom
aduth-seconds-to-i
Jul 23, 2024
Merged

Avoid unnecessary seconds conversion before to_i#10979
aduth merged 2 commits intomainfrom
aduth-seconds-to-i

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jul 23, 2024

🛠 Summary of changes

ActiveSupport::Duration#to_i returns the duration in seconds (docs), so it's redundant to convert a duration to seconds before calling .to_i.

It also has a large impact on performance, relatively speaking.

Instances discovered via project search for .seconds.to_i

Performance Impact

Benchmark code:

require 'benchmark/ips'

Benchmark.ips do |x|
  x.report('before') do
    5.minutes.seconds.to_i
  end

  x.report('after') do
    5.minutes.to_i
  end

  x.compare!
end
Warming up --------------------------------------
              before   124.827k i/100ms
               after   278.805k i/100ms
Calculating -------------------------------------
              before      1.257M (± 1.1%) i/s -      6.366M in   5.064966s
               after      2.788M (± 0.7%) i/s -     13.940M in   5.001014s

Comparison:
               after:  2787607.3 i/s
              before:  1257057.5 i/s - 2.22x  slower

📜 Testing Plan

Verify tests pass.

changelog: Internal, Performance, Avoid unnecessary seconds conversion before to_i
@aduth
Copy link
Contributor Author

aduth commented Jul 23, 2024

I'm open to swapping #to_i with #in_seconds if part of the perceived value here is clearly communicating the desire for seconds units.

@mitchellhenke
Copy link
Contributor

I'm open to swapping #to_i with #in_seconds if part of the perceived value here is clearly communicating the desire for seconds units.

Yeah, I kinda prefer that

More clearly communicates the expressed desire for value in seconds
@aduth
Copy link
Contributor Author

aduth commented Jul 23, 2024

I'm open to swapping #to_i with #in_seconds if part of the perceived value here is clearly communicating the desire for seconds units.

Yeah, I kinda prefer that

Updated these (and all other Duration#to_i) to use #in_seconds in 34f6a2f.

@aduth aduth merged commit e543ced into main Jul 23, 2024
@aduth aduth deleted the aduth-seconds-to-i branch July 23, 2024 20:40
mitchellhenke pushed a commit that referenced this pull request Jul 31, 2024
* Avoid unnecessary seconds conversion before to_i

changelog: Internal, Performance, Avoid unnecessary seconds conversion before to_i

* Replace Duration#to_i with #in_seconds

More clearly communicates the expressed desire for value in seconds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants