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

Add defensive check to avoid unnecessary gsub #153

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

bitwise-aiden
Copy link
Contributor

image

When instantiating a large volume of Measurable objects gsub is run indiscriminately. I'm adding a defensive check as benchmarks show this is a lot faster.

bench.rb
require "benchmark/ips"

class BenchGsub
  VALUES = 1000.times.map { |v| (v / 10).to_s }

  class << self
    def raw
      VALUES.each { |v| v.gsub(/\.0*\Z/, "") }
    end

    def check(value)
      VALUES.each { |v| v.gsub(/\.0*\Z/, "") if /\.0*\Z/.match?(v) }
    end
  end
end

Benchmark.ips do |x|
  x.report("raw", &BenchGsub.method(:raw))
  x.report("check", &BenchGsub.method(:check))

  x.compare!
end
                 raw    71.000 i/100ms
               check   432.286B i/100ms
Calculating -------------------------------------
                 raw      3.405k (± 4.1%) i/s -     17.040k in   5.015206s
               check      3.948Q (± 4.1%) i/s -     19.628Q in   4.985779s

Comparison:
               check: 3947665236439899.5 i/s
                 raw:     3405.4 i/s - 1159229966836.24x  slower

@bitwise-aiden bitwise-aiden force-pushed the ba-optimize-measurable-init branch from 55d39eb to 7b3d180 Compare March 12, 2024 21:40
Copy link

@kirs kirs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
The way I read this code is it trims any .000 at the end. And it's faster to do that if we check for match? before hand.

@bitwise-aiden
Copy link
Contributor Author

Nice!
The way I read this code is it trims any .000 at the end. And it's faster to do that if we check for match? before hand.

Correct!

Copy link
Member

@kmcphillips kmcphillips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good! Is .sub faster? Probably not.

There's another PR open for ruby version so we can do a release this week of measured and measured-rails after both are merged. The latter may need the same ruby version changes.

@bitwise-aiden
Copy link
Contributor Author

This is good! Is .sub faster? Probably not.
I've not measured it, but I suspect that we need gsub to remove all training 0.

@bitwise-aiden bitwise-aiden merged commit e61ffe9 into main Mar 18, 2024
25 checks passed
@bitwise-aiden bitwise-aiden deleted the ba-optimize-measurable-init branch March 18, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants