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

Test: Add benchmark for QUnit.equiv() #1704

Merged
merged 1 commit into from
Sep 29, 2022
Merged

Test: Add benchmark for QUnit.equiv() #1704

merged 1 commit into from
Sep 29, 2022

Conversation

Krinkle
Copy link
Member

@Krinkle Krinkle commented Sep 21, 2022

Ref #1327.

\cc @izelnakri

@izelnakri
Copy link
Contributor

This is amazing! I've skimmed it through, I'm about to read it in-depth soon!

@izelnakri
Copy link
Contributor

izelnakri commented Sep 29, 2022

@Krinkle Deno recently introduced a benchmarking API, should we eventually build a QUnit.bench() API that mimicks the deno's API and makes it usable in browser UI(as a failing test red label if a threshold is given or a different colored label) in the future? What do you think? https://deno.land/[email protected]/tools/benchmarker

@izelnakri
Copy link
Contributor

izelnakri commented Sep 29, 2022

I just tested this on my machine and it works well. This is LGTM, as a first step I think. Eventually we can do the following as the next steps:

  • On index-node.js finalization, build an CI artifact file so we can easily check/download the ops/sec of each build and see if theres a performance difference.
  • Put a threshold and make the benchmark fail if ops/sec is lower than the threshold for each group(probably no need to build directly as CI machine resources are dynamically provisioned 3rd party thus could make this CI task flaky.
  • Make npm run benchmark runs a benchmark comparison of QUnit.equiv versus other deep comparison libraries.
  • Implement QUnit.bench() JS API and a cli command in qunit-cli.

@Krinkle Krinkle merged commit 56689ae into main Sep 29, 2022
@Krinkle Krinkle deleted the bench branch September 29, 2022 15:39
Krinkle added a commit that referenced this pull request Oct 4, 2022
…equal`

Follows #1704.

== Exclude ==

Exclude Map/Set tests from the comparison because, unlike QUnit,
fast-deep-equal does not perform a deep equal but a shallow strict
equal when it comes to Map and Set.

```js
const QUnit = require('qunit');
const fde = require('fast-deep-equal/es6');

a = new Set([ { x: 1 }, { y: 2 } ]);
b = new Set([ { x: 1 }, { y: 2 } ]);
QUnit.equiv(a, b); // true
fde(a, b); // false
```

== Local alias ==

Assign local `equiv` in the bench.js file. Without this, even when a
dummy implementation like `QUnit.equiv = (a, b) { return a === b; }`
was still reported as 3X slower than fast-deep-equal holding the
identical function.

```
require('fast-deep-equal/es6').toString().slice(0, 100);
//> 'function equal(a, b) { if (a === b) return true; // …'

require('qunit').equiv+'';
//> 'function equiv(a, b) { if (a === b) return true; // …'
```

For example (with the above fake implementation):

```
fast-deep-equal (primitives) x 648,076 ops/sec ±0.27% (89 runs sampled)
QUnit.innerEquiv (primitives) x 255,011 ops/sec ±0.22% (90 runs sampled)
```

I eventually figured out this is because without the alias, our
benchmark case has to resolve "QUnit" of `QUnit.equiv` through every
lexical scope up to the global sope, and then access the (theoretically
mutable) "equiv" property, and so this every time. For very sensitive
workloads (like the "QUnit.equiv primitives" fixture), which basically
just hit the first `a === b` condition and return early, this is visible
in the benchmark results.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants