-
Couldn't load subscription status.
- Fork 25
Add Benchmark.ms method and enhance realtime with unit parameter #38
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
Add Benchmark.ms method and enhance realtime with unit parameter #38
Conversation
lib/benchmark.rb
Outdated
| # #=> 509.8029999935534 | ||
| # | ||
| def realtime(unit = :float_second) # :yield: | ||
| r0 = Process.clock_gettime(Process::CLOCK_MONOTONIC, unit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One concern here is there might be a non-trivial overhead to pass a variable unit to Process.clock_gettime, which happens with this change. Ruby implementations might optimize it better if the unit is constant.
So I think it would be safer to:
- Leave
realtimeuntouched - Add
msand inline the logic (it's so simple anyway), and it also avoids an extra method call + block call forms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another use case I know of is to use :nanosecond to avoid any conversion, loss of precision and overhead.
However in that case it's better to just call Process.clock_gettime(Process::CLOCK_MONOTONIC, :nanosecond) directly to avoid an extra method call + block (e.g. in benchmark harnesses like benchmark-ips).
So for that case it wouldn't be the best to use Benchmark.realtime(:nanosecond) { ... } anyway and better to:
start = Process.clock_gettime(Process::CLOCK_MONOTONIC, :nanosecond)
...
end = Process.clock_gettime(Process::CLOCK_MONOTONIC, :nanosecond)
duration_in_ns = end - startThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a fair point! I have no concerns with inlining Process.clock_gettime calls in the ms method. Updated commit & description
b730244 to
f5d4ab5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test? Then I think this is good to go, though it'd be good to get a second review too.
Rails previously included a monkeypatch that added a `ms` method to the Benchmark module to return timing results in milliseconds instead of seconds. This monkeypatch has been deprecated in Rails and will be removed in future versions. ref: rails/rails@4cdf757 This commit adds native support for millisecond timing measurements by adding a new `Benchmark.ms` method that returns elapsed time in milliseconds. For web applications, measuring performance in milliseconds is much more helpful since the majority of operations happen in less than a second. While it's acceptable to display times >1s as "1.2s", showing "0.125s" instead of "125ms" is significantly less readable and intuitive for developers analyzing performance metrics. ```ruby Benchmark.realtime { sleep 0.1 } #=> 0.10023 Benchmark.ms { sleep 0.1 } #=> 100.23 ``` This change provides a clean migration path for Rails applications currently relying on the deprecated monkeypatch while offering enhanced functionality for all benchmark users.
f5d4ab5 to
6a3fe1f
Compare
Context
Rails previously included a monkeypatch that added a
msmethod to the Benchmark module to return timing results in milliseconds instead of seconds. This monkeypatch has been deprecated in Rails and will be removed in future versions. ref: rails/rails@4cdf757This commit adds native support for millisecond timing measurements by adding a new
Benchmark.msmethodthat returns elapsed time in milliseconds.
Reasoning
For web applications, measuring performance in milliseconds is much more helpful since the majority of operations happen in less than a second. While it's acceptable to display times >1s as "1.2s", showing "0.125s" instead of "125ms" is significantly less readable and intuitive for developers analyzing performance metrics.
Examples
This change provides a clean migration path for Rails applications currently relying on the deprecated monkeypatch while offering enhanced functionality for all benchmark users.