Performance impact of jsdom 23.2.0

[DoodleBobBuffPants]

Basic info:

• Node.js version: 21.4.0
• jsdom version: 23.2.0

Minimal reproduction case

• Vitest tests with jsdom 23.1.0 (mm:ss):
  • Without coverage: 3:30
  • With coverage: 5:30
• Vitest tests with jsdom 23.2.0 (mm:ss):
  • Without coverage: 7:30
  • With coverage: 15:30

The hit to performance in 23.2.0 is quite significant, to the point that a handful of our unit tests doing no more than clicking and selecting using RTL are consistently timing out too now

I wanted to raise this as an issue as requested in the release notes for 23.2.0

How does similar code behave in browsers?

N/A

[kpala]

Seeing even more dramatic impact on one of our test suites using RTL and mocha:

jsdom 23.1.0:

436 passing (1m)

jsdom 23.2.0:

433 passing (12m)
3 failing                    <-- timed out

That's 12x time spent (and the timeouts).

[domenic]

Thanks both for your reports. It looks likely that we'll have to revert. /cc @asamuzaK

If either of you are able to produce public repro cases, as standalone repositories that we can clone and run, I imagine that will make our work on reintroducing the selection engine in the future easier.

[asamuzaK]

Sorry for the inconvenience.

[DoodleBobBuffPants]

I started looking at a repro but in my case I don't believe it will add anything new - in 23.1.0 our tests which timeout were under the default timeout limit of 5s, and in 23.2.0 our tests exceed that timeout. Increasing the timeout resolves this, so it comes down to performance again

[SamuelWei]

My tests are also failing due to TypeError since 23.2.0

TypeError: Cannot read properties of null (reading 'createTreeWalker')
    at q._setup (/var/www/html/node_modules/@asamuzakjp/dom-selector/src/js/matcher.js:173:29)
    at new selector (/var/www/html/node_modules/@asamuzakjp/dom-selector/src/js/matcher.js:95:67)
    at Object.y (/var/www/html/node_modules/@asamuzakjp/dom-selector/src/index.js:41:31)
    at exports.closest (/var/www/html/node_modules/jsdom/lib/jsdom/living/helpers/selectors.js:22:22)
    at HTMLButtonElementImpl.closest (/var/www/html/node_modules/jsdom/lib/jsdom/living/nodes/Element-impl.js:548:12)
    at HTMLButtonElement.call [as closest] (/var/www/html/node_modules/jsdom/lib/jsdom/living/generated/Element.js:632:58)
    at closest (/var/www/html/node_modules/bootstrap-vue/src/utils/dom.js:130:24)
    at VueComponent.isInModal (/var/www/html/node_modules/bootstrap-vue/src/components/tooltip/helpers/bv-tooltip.js:592:24)
    at VueComponent.setModalListener (/var/www/html/node_modules/bootstrap-vue/src/components/tooltip/helpers/bv-tooltip.js:770:16)
    at VueComponent.setWhileOpenListeners (/var/www/html/node_modules/bootstrap-vue/src/components/tooltip/helpers/bv-tooltip.js:745:12)

[jacquesg]

The performance regression is actually significant, at least for my uses, running on around 14 cores:

23.1.0:

Test Suites: 140 passed, 140 total
Tests:       327 passed, 327 total
Snapshots:   0 total
Time:        18.889 s, estimated 19 s

23.2.0:

Test Suites: 140 passed, 140 total
Tests:       327 passed, 327 total
Snapshots:   0 total
Time:        30.187 s

[domenic]

I started looking at a repro but in my case I don't believe it will add anything new - in 23.1.0 our tests which timeout were under the default timeout limit of 5s, and in 23.2.0 our tests exceed that timeout. Increasing the timeout resolves this, so it comes down to performance again

Any repros would still be useful. We need to know what sorts of cases are slower, so we can optimize them.

My tests are also failing due to TypeError since 23.2.0

A repro would be extremely helpful.

The performance regression is actually significant, at least for my uses, running on around 14 cores:

Can you provide any test cases so we can profile this performance regression and speed up the slow parts?

[asamuzaK]

I've published [email protected] for testing purpose.
Improved performance of matches() and closest().
It is still 6~8x slower than nwsapi, but 2x faster than [email protected]

Latest benchmark results:

benchmark	nwsapi	[email protected]	[email protected]
# selectors/complex-selectors/closest #	jsdom x 126,133 ops/sec ±0.75% (73 runs sampled)	jsdom x 8,078 ops/sec ±15.55% (59 runs sampled)	jsdom x 15,489 ops/sec ±7.70% (61 runs sampled)
# selectors/complex-selectors/matches #	jsdom x 120,369 ops/sec ±8.31% (65 runs sampled)	jsdom x 9,898 ops/sec ±8.28% (60 runs sampled)	jsdom x 18,755 ops/sec ±9.55% (58 runs sampled)
# selectors/complex-selectors/querySelector #	jsdom x 3.12 ops/sec ±2.60% (12 runs sampled)	jsdom x 2,527 ops/sec ±3.31% (55 runs sampled)	jsdom x 3,122 ops/sec ±1.56% (58 runs sampled)
# selectors/complex-selectors/querySelectorAll #	jsdom x 2.93 ops/sec ±1.52% (11 runs sampled)	jsdom x 11.39 ops/sec ±1.65% (29 runs sampled)	jsdom x 11.23 ops/sec ±1.57% (28 runs sampled)
# selectors/sizzle/querySelectorAll #	jsdom x 1.20 ops/sec ±10.21% (7 runs sampled) 18/273 fails.	jsdom x 0.41 ops/sec ±5.93% (6 runs sampled) 9/273 fails.	jsdom x 0.42 ops/sec ±6.31% (5 runs sampled) 9/273 fails.
# selectors/selector/querySelectorAll only nwsapi supports #	jsdom x 1.24 ops/sec ±8.02% (7 runs sampled)	jsdom x 0.42 ops/sec ±1.54% (5 runs sampled)	jsdom x 0.44 ops/sec ±1.48% (6 runs sampled)

If you can help, could you please add the following to package.json and test it?

package.json:

overrides: {
  "@asamuzakjp/dom-selector": "2.0.2-a.2"
}

Or directly install:

npm i @asamuzakjp/dom-selector@next

[jacquesg]

If you can help, could you please add the following to package.json and test it?

package.json:

overrides: {
  "@asamuzakjp/dom-selector": "2.0.2-a.2"
}

Or directly install:

npm i @asamuzakjp/dom-selector@next

Ran this a couple of times, shaves around a second off the time, going from 30s to 29s.

It does however introduce something new, which looks like a regression:

Unable to find an element with the text: Home. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your
 matcher more flexible.

...

      259 |     expect(lists).toHaveLength(3);
      260 |
    > 261 |     expect(await within(lists[1]).findByText('Home')).toBeInTheDocument();
          |                                   ^
      262 |     expect(await within(lists[2]).findByText('Administration')).toBeInTheDocument();
      263 |   });
      264 | });

      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:162:27)
      at node_modules/@testing-library/dom/dist/query-helpers.js:86:33
      at findByText (src/tests/pages/app/layout.test.tsx:261:35

    const lists = await screen.findAllByRole('list');
    expect(lists).toHaveLength(3);

    expect(await within(lists[1]).findByText('Home')).toBeInTheDocument();
    expect(await within(lists[2]).findByText('Administration')).toBeInTheDocument();

It seems like findAllByRole doesn't return the items in a stable order as before.

[asamuzaK]

It seems like findAllByRole doesn't return the items in a stable order as before.

Thanks for the catch.
Fixed it.
asamuzaK/domSelector@21492b5

[diegohaz]

We observed a roughly 40% increase in execution time in our tests after upgrading to v23.2.0. Some tests even timed out.

[domenic]

We observed a roughly 40% increase in execution time in our tests after upgrading to v23.2.0. Some tests even timed out.

Are you able to provide sample code we can use to reproduce and fix this regression?

[domenic]

So far, none of the reporters here have been able to invest the time in creating a benchmark we can use for the future. (I don't even demand a standalone repro that uses only jsdom; a whole GitHub repository with whatever testing frameworks would be enough!)

Given this, I'm planning not to revert for now. Reverting without such a benchmark would mean we have no ability to judge when dom-selector is safe to reintroduce in the future, and so would set us up for a painful cycle of dom-selector (which has more capabilities, which people might depend on, as seen in e.g. #3618 (comment)) -> nwmatcher (which is faster but more broken), back and forth, based purely on anecdotes.

If people are interested in getting jsdom back to its 23.1.0 performance (at the loss of its new selector capabilities), then please set up a benchmark repository. The more minimal the better, and if it can be done as a pull request against https://github.com/jsdom/jsdom/tree/main/benchmark/selectors, that'd be ideal. If we get one or two such benchmarks, then I'll spend the time on reverting.

[diegohaz]

I don't even demand a standalone repro that uses only jsdom; a whole GitHub repository with whatever testing frameworks would be enough!

Sorry, I initially thought you were asking for a minimal reproduction, which I haven't had time to build.

The performance regression I've noticed originates from a public repository: https://github.com/ariakit/ariakit.

Here's the PR where jsdom is getting updated: ariakit/ariakit#3336

The following link shows an instance of tests failing due to timeout: https://github.com/ariakit/ariakit/actions/runs/7511045254/job/20450195794?pr=3336#step:6:83

As you can see, the tests require 325 seconds to finish.

For comparison, here's an instance of tests from the main branch, where jsdom 23.1.0 is in use: https://github.com/ariakit/ariakit/actions/runs/7510618416/job/20449247363#step:6:73

In this case, the tests are completed in 216 seconds.

[nstepien]

In https://github.com/adazzle/react-data-grid

• npm install
• npm test virt

This will run the virtualization.test.ts tests in Vitest.

With jsdom 23.1.0:

 ✓ test/virtualization.test.ts (8) 6600ms
   ✓ virtualization is enabled 2373ms
   ✓ virtualization is enabled with 4 frozen columns 306ms
   ✓ virtualization is enabled with all columns frozen
   ✓ virtualization is enabled with 2 summary rows
   ✓ zero columns
   ✓ zero rows
   ✓ virtualization is enable with not enough columns or rows to virtualize
   ✓ enableVirtualization is disabled 3325ms

With jsdom 23.2.0 + testTimeout: 50000 in vite.config.ts:

 ✓ test/virtualization.test.ts (8) 27247ms
   ✓ virtualization is enabled 14558ms
   ✓ virtualization is enabled with 4 frozen columns 477ms
   ✓ virtualization is enabled with all columns frozen 556ms
   ✓ virtualization is enabled with 2 summary rows 388ms
   ✓ zero columns
   ✓ zero rows
   ✓ virtualization is enable with not enough columns or rows to virtualize
   ✓ enableVirtualization is disabled 10880ms