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

fix: compare inherited properties not prototypes #22

Merged
merged 2 commits into from
Oct 18, 2016
Merged

fix: compare inherited properties not prototypes #22

merged 2 commits into from
Oct 18, 2016

Conversation

meeber
Copy link
Contributor

@meeber meeber commented Oct 18, 2016

  • The v1.0.0 refactor created a breaking change by only performing deep
    equality comparisons between objects' own enumerable properties, and
    performing a strict equality comparison between objects' prototypes.
    This fix reverts the behavior so that deep equality comparisons are
    performed between objects' own and inherited enumerable properties,
    and no comparison is performed between objects' prototypes.

- The v1.0.0 refactor created a breaking change by only performing deep
  equality comparisons between objects' own enumerable properties, and
  performing a strict equality comparison between objects' prototypes.
  This fix reverts the behavior so that deep equality comparisons are
  performed between objects' own and inherited enumerable properties,
  and no comparison is performed between objects' prototypes.
@keithamus
Copy link
Member

This LGTM. Curious though, how is the impact on our benchmarks?

@lucasfcosta
Copy link
Member

lucasfcosta commented Oct 18, 2016

LGTM too, since this seems as performatic and correct as it can be.
Anyway it would be cool to see the benchmarks before merging (just in case), as @keithamus said.

@keithamus
Copy link
Member

@meeber looks like this one needs a rebase 😄

@keithamus
Copy link
Member

Scratch that, looks like it could update automatically

@lucasfcosta
Copy link
Member

lucasfcosta commented Oct 18, 2016

@keithamus Github should offer a "rebase on master" button for us.

@meeber
Copy link
Contributor Author

meeber commented Oct 18, 2016

Yeah changes shouldn't conflict among the PRs so just gotta wait for travis to finish catching up :D

@meeber
Copy link
Contributor Author

meeber commented Oct 18, 2016

I do all my development on a small vps so dunno how accurate or consistent benchmarks will be when I run them. But below is what I got. I'd expect the biggest difference to be when comparing objects with huge prototypes.

Test                                 Before                                           After
equal references                     x 32,640,621 ops/sec ±0.34% (87 runs sampled)    x 34,622,949 ops/sec ±0.17% (89 runs sampled)
string literal                       x 31,846,241 ops/sec ±0.61% (85 runs sampled)    x 31,630,608 ops/sec ±0.32% (87 runs sampled)
array literal                        x 882,798 ops/sec ±1.16% (85 runs sampled)       x 869,210 ops/sec ±1.52% (89 runs sampled)
boolean literal                      x 32,818,037 ops/sec ±0.11% (90 runs sampled)    x 31,649,311 ops/sec ±0.18% (88 runs sampled)
object literal                       x 225,614 ops/sec ±0.88% (91 runs sampled)       x 189,562 ops/sec ±0.66% (89 runs sampled)
object from null                     x 448,505 ops/sec ±1.77% (84 runs sampled)       x 432,434 ops/sec ±0.92% (90 runs sampled)
regex literal                        x 485,084 ops/sec ±1.05% (87 runs sampled)       x 486,414 ops/sec ±0.99% (89 runs sampled)
number literal                       x 34,852,650 ops/sec ±0.14% (91 runs sampled)    x 34,850,096 ops/sec ±0.21% (89 runs sampled)
null                                 x 32,925,222 ops/sec ±0.16% (91 runs sampled)    x 33,046,678 ops/sec ±0.29% (90 runs sampled)
undefined                            x 32,591,832 ops/sec ±0.14% (92 runs sampled)    x 32,891,653 ops/sec ±0.30% (90 runs sampled)
buffer                               x 696,219 ops/sec ±0.47% (91 runs sampled)       x 659,592 ops/sec ±0.74% (90 runs sampled)
date                                 x 534,556 ops/sec ±1.00% (90 runs sampled)       x 517,606 ops/sec ±1.63% (89 runs sampled)
map                                  x 175,217 ops/sec ±0.97% (90 runs sampled)       x 179,450 ops/sec ±0.92% (91 runs sampled)
map (complex)                        x 91,390 ops/sec ±1.10% (87 runs sampled)        x 93,703 ops/sec ±1.07% (87 runs sampled)
regex constructor                    x 472,641 ops/sec ±1.00% (88 runs sampled)       x 505,953 ops/sec ±1.09% (91 runs sampled)
set                                  x 179,691 ops/sec ±1.14% (85 runs sampled)       x 169,910 ops/sec ±1.07% (91 runs sampled)
string constructor                   x 287,224 ops/sec ±0.76% (88 runs sampled)       x 286,699 ops/sec ±0.80% (89 runs sampled)
arguments                            x 293,624 ops/sec ±0.83% (89 runs sampled)       x 304,509 ops/sec ±1.21% (86 runs sampled)
string literal (differing)           x 32,032,067 ops/sec ±0.19% (90 runs sampled)    x 31,848,021 ops/sec ±0.15% (91 runs sampled)
array literal (differing)            x 817,320 ops/sec ±1.09% (87 runs sampled)       x 809,607 ops/sec ±1.96% (87 runs sampled)
boolean literal (differing)          x 25,011,391 ops/sec ±0.15% (88 runs sampled)    x 25,248,095 ops/sec ±0.12% (87 runs sampled)
object literal (differing)           x 217,846 ops/sec ±1.02% (86 runs sampled)       x 214,787 ops/sec ±0.81% (90 runs sampled)
regex literal (differing)            x 483,227 ops/sec ±0.97% (90 runs sampled)       x 491,333 ops/sec ±1.03% (90 runs sampled)
number literal (differing)           x 30,531,383 ops/sec ±0.14% (90 runs sampled)    x 30,556,888 ops/sec ±0.14% (89 runs sampled)
null & undefined                     x 24,780,196 ops/sec ±0.10% (85 runs sampled)    x 24,606,122 ops/sec ±0.14% (91 runs sampled)
buffer (differing)                   x 796,321 ops/sec ±1.35% (90 runs sampled)       x 751,495 ops/sec ±1.30% (80 runs sampled)
date (differing)                     x 548,705 ops/sec ±0.72% (90 runs sampled)       x 564,425 ops/sec ±0.58% (90 runs sampled)
error                                x 298,021 ops/sec ±0.93% (89 runs sampled)       x 322,948 ops/sec ±1.11% (88 runs sampled)
map (differing)                      x 174,350 ops/sec ±1.21% (88 runs sampled)       x 172,117 ops/sec ±1.13% (90 runs sampled)
regex ctor (differing)               x 497,581 ops/sec ±1.19% (90 runs sampled)       x 504,210 ops/sec ±0.95% (90 runs sampled)
set (differing)                      x 125,355 ops/sec ±0.83% (88 runs sampled)       x 174,546 ops/sec ±1.31% (84 runs sampled)
string ctor (differing)              x 277,464 ops/sec ±0.78% (89 runs sampled)       x 278,561 ops/sec ±1.33% (86 runs sampled)
weakmap                              x 864,241 ops/sec ±0.90% (90 runs sampled)       x 848,063 ops/sec ±1.93% (89 runs sampled)
weakset                              x 861,116 ops/sec ±1.10% (88 runs sampled)       x 845,162 ops/sec ±1.02% (92 runs sampled)
arguments (differing)                x 296,218 ops/sec ±0.91% (91 runs sampled)       x 303,600 ops/sec ±1.09% (91 runs sampled)
function                             x 26,723,027 ops/sec ±0.14% (88 runs sampled)    x 26,044,516 ops/sec ±0.14% (88 runs sampled)
promise                              x 844,020 ops/sec ±1.74% (90 runs sampled)       x 785,730 ops/sec ±1.36% (89 runs sampled)
arrow function (differing)           x 26,735,630 ops/sec ±0.08% (88 runs sampled)    x 26,745,273 ops/sec ±0.10% (87 runs sampled)
generator func (differing)           x 26,695,777 ops/sec ±1.09% (88 runs sampled)    x 26,744,061 ops/sec ±0.09% (89 runs sampled)

@lucasfcosta
Copy link
Member

Looks reasonable to mee, we didn't lose much in terms of performance.
Excellent work @meeber, you rock!

@keithamus keithamus merged commit 248a5b3 into chaijs:master Oct 18, 2016
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