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

Performance and memory fixes #52

Merged
merged 7 commits into from
Apr 22, 2019

Conversation

krzysiek1507
Copy link
Collaborator

It should be a little bit faster (or a lot faster in some cases) and use less memory.

@krzysiek1507 krzysiek1507 force-pushed the feature/performance-fixes branch from eccae36 to c55d171 Compare April 18, 2019 18:07
@krzysiek1507
Copy link
Collaborator Author

lib/hashdiff/util.rb:11:20: C: Use (count_a + count_b).zero? instead of count_a + count_b == 0.
    return true if count_a + count_b == 0
                   ^^^^^^^^^^^^^^^^^^^^^^

In fact number.zero? is about 30% slower than number == 0.

@krzysiek1507 krzysiek1507 force-pushed the feature/performance-fixes branch from 25e61ad to bdab797 Compare April 18, 2019 18:20
@krzysiek1507
Copy link
Collaborator Author

If it is easier to review, I can split it into smaller PRs.

@liufengyun
Copy link
Owner

Thanks @krzysiek1507 -- One PR is fine. I'm wondering why some test failed, could you have a look?

@krzysiek1507
Copy link
Collaborator Author

It's rubocop and this particular rule doesn't make sense in this case.

lib/hashdiff/util.rb:85:5: C: Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.
    if options[:numeric_tolerance].is_a?(Numeric) && obj1.is_a?(Numeric) && obj2.is_a?(Numeric)
    ^^

when both objects are empty hashes or arrays:

Warming up --------------------------------------
            original    18.539k i/100ms
           optimized   120.971k i/100ms
Calculating -------------------------------------
            original    204.282k (± 2.1%) i/s -      1.038M in   5.084371s
           optimized      1.968M (± 3.1%) i/s -      9.920M in   5.046494s

Comparison:
           optimized:  1967768.6 i/s
            original:   204281.6 i/s - 9.63x  slower

Calculating -------------------------------------
            original     1.568k memsize (     0.000  retained)
                        20.000  objects (     0.000  retained)
                         2.000  strings (     0.000  retained)
           optimized   232.000  memsize (     0.000  retained)
                         1.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
           optimized:        232 allocated
            original:       1568 allocated - 6.76x more
@krzysiek1507 krzysiek1507 force-pushed the feature/performance-fixes branch from bdab797 to e106ef2 Compare April 18, 2019 19:10
@krzysiek1507
Copy link
Collaborator Author

Some stats:

obj1 = obj2 = []

Warming up --------------------------------------
              master    49.253k i/100ms
Calculating -------------------------------------
              master    590.795k (± 2.1%) i/s -      2.955M in   5.004451s
Calculating -------------------------------------
              master   856.000  memsize (     0.000  retained)
                         7.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Warming up --------------------------------------
           optimized    82.442k i/100ms
Calculating -------------------------------------
           optimized      1.140M (± 1.8%) i/s -      5.771M in   5.061820s
Calculating -------------------------------------
           optimized   544.000  memsize (     0.000  retained)
                         4.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

================================================

obj1 = obj2 = {}

Warming up --------------------------------------
              master    24.736k i/100ms
Calculating -------------------------------------
              master    274.993k (± 5.6%) i/s -      1.385M in   5.059656s
Calculating -------------------------------------
              master     1.024k memsize (     0.000  retained)
                        16.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Warming up --------------------------------------
           optimized    25.828k i/100ms
Calculating -------------------------------------
           optimized    314.045k (± 5.0%) i/s -      1.576M in   5.032663s
Calculating -------------------------------------
           optimized   824.000  memsize (     0.000  retained)
                        11.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

================================================

obj1 = { a: 1, b: { b2: [], b3: [{ b4: 1 }] }, h: [1, 2, 3, 4, 5], i: 2, j: 2, k: [] }
obj2 = { a: 1, b: { b2: [], b3: [{ b4: 1 }] }, g: [1, 2, 3, 4, 5], i: 2, j: 4, k: {} }

Warming up --------------------------------------
              master     1.540k i/100ms
Calculating -------------------------------------
              master     15.653k (± 8.4%) i/s -     78.540k in   5.072408s
Calculating -------------------------------------
              master    14.608k memsize (     0.000  retained)
                       164.000  objects (     0.000  retained)
                        17.000  strings (     0.000  retained)

Warming up --------------------------------------
           optimized     1.789k i/100ms
Calculating -------------------------------------
           optimized     18.383k (± 2.6%) i/s -     93.028k in   5.063884s
Calculating -------------------------------------
           optimized    12.592k memsize (     0.000  retained)
                       128.000  objects (     0.000  retained)
                        17.000  strings (     0.000  retained)

@liufengyun
Copy link
Owner

Thanks @krzysiek1507. Maybe we can silence rubocop in that case?

obj1 = 1, obj2 = 1.1, numeric_tolerance = 0.1

Warming up --------------------------------------
            original   106.128k i/100ms
           optimized   161.873k i/100ms
Calculating -------------------------------------
            original      1.682M (± 5.8%) i/s -      8.384M in   5.005149s
           optimized      3.560M (± 2.4%) i/s -     17.806M in   5.005019s

Comparison:
           optimized:  3559889.7 i/s
            original:  1682449.9 i/s - 2.12x  slower

Calculating -------------------------------------
            original    40.000  memsize (     0.000  retained)
                         1.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)
           optimized     0.000  memsize (     0.000  retained)
                         0.000  objects (     0.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
           optimized:          0 allocated
            original:         40 allocated - Infx more
@krzysiek1507 krzysiek1507 force-pushed the feature/performance-fixes branch from e106ef2 to 6d028a6 Compare April 22, 2019 16:47
@krzysiek1507
Copy link
Collaborator Author

@liufengyun done!

@liufengyun
Copy link
Owner

Thanks a lot @krzysiek1507 🎉

@liufengyun liufengyun merged commit 2040cc9 into liufengyun:master Apr 22, 2019
@krzysiek1507 krzysiek1507 deleted the feature/performance-fixes branch April 22, 2019 22:54
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