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

Decreased performance compared to the original distance_of_time_in_words #79

Open
Brotakuu opened this issue Dec 5, 2016 · 2 comments

Comments

@Brotakuu
Copy link

Brotakuu commented Dec 5, 2016

Great gem, but the performance seems to be significantly slower than the method it replaces. Running a benchmark 10000 times, it is on average 4x slower.

Use dotiw   2.620000   0.010000   2.630000 (  2.666948)
No dotiw    0.670000   0.010000   0.680000 (  0.697011)

Any idea how to speed it up?

@guilherme
Copy link

@Brotakuu could you provide your benchmark?
I've tried this:

# clone the repo.
# gem install rails benchmark-ips
# add this file to the root folder of the repo.
$: << File.join(`pwd`.chomp, "lib")
require 'active_support/all'
require 'action_view'
require 'benchmark/ips'
require 'dotiw'

include ActionView::Helpers::DateHelper

Benchmark.ips do |bm|
  bm.report("distance_of_time_in_words") { distance_of_time_in_words(Time.now, Time.now + 100.years + 30.days + 50.seconds) }
  bm.report("old_distance_of_time_in_words") { old_distance_of_time_in_words(Time.now, Time.now + 100.years + 30.days + 50.seconds) }
  bm.compare!
end 

These are the results I got:

Calculating -------------------------------------
distance_of_time_in_words
                          2.893k (± 2.4%) i/s -     14.739k in   5.097142s
old_distance_of_time_in_words
                          7.533k (± 2.4%) i/s -     37.850k in   5.027373s

Comparison:
old_distance_of_time_in_words:     7533.1 i/s
distance_of_time_in_words:     2893.2 i/s - 2.60x  slower

@guilherme
Copy link

guilherme commented Mar 8, 2018

I've noticed that it'd include the time spent calculating: Time.now + the dates math of the arguments.
and also I am not 100% sure if I this is the worst case scenario for both.
It's an interesting problem to look. I started to poke it.

This is more accurate, for this case:

# clone the repo.
# gem install rails benchmark-ips
# add this file to the root folder of the repo.
$: << File.join(`pwd`.chomp, "lib")
require 'active_support/all'
require 'action_view'
require 'benchmark/ips'
require 'dotiw'

include ActionView::Helpers::DateHelper

Benchmark.ips do |bm|
  now = Time.now
  future_time =  Time.now + 100.years + 30.days + 50.seconds
  bm.report("distance_of_time_in_words") { distance_of_time_in_words(now,future_time) }
  bm.report("old_distance_of_time_in_words") { old_distance_of_time_in_words(now, future_time, include_seconds: true) }
  bm.compare!
end 
Warming up --------------------------------------
distance_of_time_in_words
                       379.000  i/100ms
old_distance_of_time_in_words
                         2.312k i/100ms
Calculating -------------------------------------
distance_of_time_in_words
                          3.674k (± 4.4%) i/s -     18.571k in   5.065120s
old_distance_of_time_in_words
                         24.772k (± 3.5%) i/s -    124.848k in   5.046092s

Comparison:
old_distance_of_time_in_words:    24772.5 i/s
distance_of_time_in_words:     3674.0 i/s - 6.74x  slower

Observations:

  • It's interesting how is 6x because the new one split the distance in 6: (years, weeks, days, hours, minutes, seconds) and the old one pick just the greatest. 🤷‍♂️

@dblock dblock changed the title Performance running slow Decreased performance compared to the original distance_of_time_in_words Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants