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

time: Add EventTime#to_time for faster Time creation #2469

Merged
merged 1 commit into from
Jun 21, 2019

Conversation

repeatedly
Copy link
Member

Signed-off-by: Masahiro Nakagawa repeatedly@gmail.com

Which issue(s) this PR fixes:
None

What this PR does / why we need it:
Fluent::EventTime can be converted into Time via Time.at but
this approach is slower. Output plugins sometimes require Time
for client libraries so adding faster method.

Here is benchmark code:

require 'benchmark/ips'
require 'fluent/time'

ot = Time.now
et = Fluent::EventTime.from_time(ot)

Benchmark.ips do |x|
  x.report "to_time" do
    et.to_time
  end

  x.report "Time.at" do
    Time.at(et)
  end
end
Warming up --------------------------------------
             to_time   171.865k i/100ms
             Time.at    92.733k i/100ms
Calculating -------------------------------------
             to_time      2.861M (± 5.1%) i/s -     14.265M in   5.002172s
             Time.at      1.184M (± 4.1%) i/s -      5.935M in   5.022106s

Docs Changes:
No need for now.

Release Note:
Same as title

@repeatedly repeatedly added the enhancement Feature request or improve operations label Jun 20, 2019
@repeatedly repeatedly requested a review from ganmacs June 20, 2019 20:37
@repeatedly repeatedly force-pushed the event-time-to_time branch 2 times, most recently from d9518f3 to 1f66148 Compare June 20, 2019 20:59
@ganmacs
Copy link
Member

ganmacs commented Jun 21, 2019

the change looks good but 2.4 or before tests failed.
https://travis-ci.org/fluent/fluentd/jobs/548400039

@repeatedly
Copy link
Member Author

That's weird. This test passed with ruby 2.4.6 on my Mac...

Fluent::EventTime can be converted into Time via Time.at but
this approach is slower. Output plugins sometimes require Time
for client libraries so adding faster method.

Warming up --------------------------------------
             to_time   171.865k i/100ms
             Time.at    92.733k i/100ms
Calculating -------------------------------------
             to_time      2.861M (± 5.1%) i/s -     14.265M in   5.002172s
             Time.at      1.184M (± 4.1%) i/s -      5.935M in   5.022106s

Signed-off-by: Masahiro Nakagawa <repeatedly@gmail.com>
@repeatedly repeatedly merged commit a0240f6 into master Jun 21, 2019
@repeatedly
Copy link
Member Author

Test now passed on older environment

@repeatedly repeatedly deleted the event-time-to_time branch June 21, 2019 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or improve operations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants