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

Optimize hash access by safe-navigating Hash#dig #654

Merged
merged 5 commits into from
Apr 30, 2020

Conversation

ashmaroli
Copy link
Contributor

@ashmaroli ashmaroli commented Apr 26, 2020

By using Hash#dig in tandem with the safe navigation operator (both from Ruby 2.3), the logic doesn't need to rescue and return nil (therefore faster and avoids allocating Thread::Backtrace) nor have to assign to [] (on every call) to check if an object is included..

Note: Only lines explicitly that rescue and return nil are patched in this pull request.

@gettalong
Copy link
Owner

Benchmarking #dig(:a, :b) vs #[:a][:b]:

    Benchmark.driver do |x|
      x.prelude %{
        hash = {a: {b: 5}}
      }
      x.report("[][]", %{hash[:a][:b]})
      x.report("dig", %{hash.dig(:a, :b)})
    end

Result on 2.6.5:

Warming up --------------------------------------
                [][]    24.712M i/s -     24.839M times in 1.005128s (40.47ns/i, 53clocks/i)
                 dig    18.893M i/s -     19.088M times in 1.010305s (52.93ns/i, 129clocks/i)
Calculating -------------------------------------
                [][]    38.107M i/s -     74.135M times in 1.945436s (26.24ns/i, 88clocks/i)
                 dig    24.735M i/s -     56.680M times in 2.291450s (40.43ns/i, 102clocks/i)

Comparison:
                [][]:  38107341.0 i/s
                 dig:  24735237.7 i/s - 1.54x  slower

So I wouldn't use #dig if it is not necessary/beneficial.

@ashmaroli
Copy link
Contributor Author

I see.., however with specific focus on line #163 in the diff:

Benchmark.driver do |x|
  x.prelude %{
    options = {}
  }
  x.report("[][]", %{options[:ial][:refs] rescue nil})
  x.report("dig", %{options.dig(:ial, :refs)})
end

Benchmark.driver do |x|
  x.prelude %{
    options = {ial: {}}
  }
  x.report("[][]", %{options[:ial][:refs] rescue nil})
  x.report("dig", %{options.dig(:ial, :refs)})
end

With Ruby 2.4 on Windows:

Warming up --------------------------------------
                [][]   129.116k i/s -    131.448k times in 1.018059s (7.74I¼s/i)
                 dig     8.026M i/s -      8.139M times in 1.014058s (124.59ns/i)
Calculating -------------------------------------
                [][]   131.071k i/s -    387.348k times in 2.955256s (7.63I¼s/i)
                 dig    12.123M i/s -     24.078M times in 1.986242s (82.49ns/i)

Comparison:
                 dig:  12122509.6 i/s
                [][]:    131070.9 i/s - 92.49x  slower

Warming up --------------------------------------
                [][]     8.394M i/s -      8.462M times in 1.008058s (119.13ns/i)
                 dig     5.713M i/s -      5.799M times in 1.015058s (175.04ns/i)
Calculating -------------------------------------
                [][]    12.915M i/s -     25.183M times in 1.949875s (77.43ns/i)
                 dig     7.513M i/s -     17.139M times in 2.281207s (133.10ns/i)

Comparison:
                [][]:  12915309.1 i/s
                 dig:   7513289.4 i/s - 1.72x  slower

I concede that Hash#dig is slower than hash[][].. but Hash#dig has builtin nil guards.
From my understanding, scenario 1 is more common than scenario 2..

@gettalong
Copy link
Owner

Ah, sorry, this is my fault, posted this between things... What I meant with my benchmark comment was that the additional changes are not needed, i.e. the ones you meant with "The remaining changes are for consistency."

@gettalong gettalong merged commit d3458f6 into gettalong:master Apr 30, 2020
@gettalong
Copy link
Owner

Thank you!

@gettalong gettalong self-assigned this Apr 30, 2020
@ashmaroli ashmaroli deleted the safe-navigate-hash-digs branch April 30, 2020 07:27
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