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

Rewrite if operator with while loop #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yichaulin
Copy link

Description

#each_slice would cause additional object allocations. Using basic while loop would have better performance.

Performance Benchmark

Test Suit

require 'benchmark/ips'
require "bundler/gem_tasks"
require 'json_logic'

true_case = {
  "if" => [true, 1, 'a']
}

false_case = {
  "if" => [false, 1, 'a']
}


true_case_compiled = JSONLogic.compile(true_case)
false_case_compiled = JSONLogic.compile(false_case)

Benchmark.ips do |x|
  x.report("if_operator[true]") { true_case_compiled.evaluate(nil) }
  x.report("if_operator[false]") { false_case_compiled.evaluate(nil) }
end

Before PR

Warming up --------------------------------------
   if_operator[true]   120.624k i/100ms
  if_operator[false]   113.248k i/100ms
Calculating -------------------------------------
   if_operator[true]      1.183M (± 2.2%) i/s -      6.031M in   5.101916s
  if_operator[false]      1.159M (± 2.1%) i/s -      5.889M in   5.082047s

After PR

Warming up --------------------------------------
   if_operator[true]   164.110k i/100ms
  if_operator[false]   154.327k i/100ms
Calculating -------------------------------------
   if_operator[true]      1.642M (± 0.3%) i/s -      8.370M in   5.097412s
  if_operator[false]      1.533M (± 1.0%) i/s -      7.716M in   5.033156s

About 32% ~ 35% improvement

i = 0
while i < v.length do
condition = v[i]
value = v[i+1]
Copy link

Choose a reason for hiding this comment

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

TIL: access an out-of-bound index will yield nil instead of an exception.

@fill32
Copy link

fill32 commented May 14, 2023

lg, very good on proactively working on these performance improvements.

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.

2 participants