Repository navigation
Web platform tests that are worryingly slow #3154
Description
Activity
- added 2 commits that reference this issue
on Mar 23, 2021 One issue I've found is that selections that use setEndBefore (and probably also setStartBefore) iterate over the whole DOM in some cases.
Consider the below example, where the range ends before the second paragraph.
The end node of this range is the body element, which kind of makes sense because the end is between two nodes.
However,
nextNodeDescendantmakes it so that the endNode isjsdom/lib/jsdom/living/range/Range-impl.js
Lines 394 to 401 in 04f6c13
const endNode = nextNodeDescendant(_end.node); while (currentNode && currentNode !== endNode) { if (currentNode.nodeType === NODE_TYPE.TEXT_NODE && isContained(currentNode, this)) { s += currentNode.data; } currentNode = domSymbolTree.following(currentNode); } nullwhich means the loop can never terminate and iterates over all nodes in the DOM. This isn't an issue with small documents, but big documents become very slow.import { JSDOM } from 'jsdom'; const html = `<!DOCTYPE HTML> <html> <head> <meta charset="utf-8"> <title>AHTML 2.1 Document</title> <style type="text/css"></style> </head> <body id="0"> <p id="1"><span id="start" class="cs1" lang="und" dir="ltr">text</span> <span id="2601" class="cs2" lang="und" dir="ltr">inside</span> <span id="2602" class="cs1" lang="und" dir="ltr">selection</span> </p> <p id="end"> <span id="2612" class="cs1" lang="und" dir="ltr">text</span> <span id="2613" class="cs2" lang="und" dir="ltr">outside</span> <span id="2614" class="cs1" lang="und" dir="ltr">selection</span></p> </body> `; const dom = new JSDOM(html); const bodyEl = dom.window.document.body; const domRange = bodyEl.ownerDocument.createRange(); const startNode = dom.window.document.querySelector('#start'); domRange.setStart(startNode, 0); const endNode = dom.window.document.querySelector('#end'); domRange.setEndBefore(endNode); const selection = bodyEl.ownerDocument.getSelection(); selection.removeAllRanges(); selection.addRange(domRange); console.log(selection.toString());To me, the obvious solution would be to save the "endBeforeNode" and use it as an additional terminator for the while loop:
setEndBefore(node) { this._endBeforeNode = node; const parent = domSymbolTree.parent(node); ...while (currentNode && currentNode !== endNode && currentNode !== this._endBeforeNode) { ... }- addedmetr upliftA candidate for the METR Uplift experimentA candidate for the METR Uplift experiment
on Mar 13, 2025 - added 8 commits that reference this issue
on Mar 15, 2025 Implementation notes, at least for today:
Part 1
AI use
I am very happy AI was allowed for this task. It was an invaluable partner on brainstorming and learning about various profiling tools. I used Cursor for coding (with "auto" mode selected), I used Claude Sonnet 3.7 and ChatGPT o3-mini-high for learning about profiling tools, and I used Claude Sonnet 3.7 extended thinking + ChatGPT o3-mini-high for iterating on faster versions of various functions, or trying to find performance issues.
I focused all my attention today on
Range-mutations-dataChange.html, but I suspect the improvements I've made so far will benefit most jsdom tests. I'm not sure it's really enough though to stop the timeouts...My first attempt was to get Cursor to implement @Narretz's suggestion and see if that magically fixed things. Sadly, it did not; the test took about the same 7 seconds before and after on my machine. I decided I'd need to actually profile things.
(Note that this mostly invalidates my initial guess of 60 minutes without AI / 30 minutes with AI to fix this problem, as it greatly increased the scope of the work.)
Claude Sonnet 3.7 and me had a frustrating time getting profiling tools running on my setup, because of issues like being on Windows, having spaces in my path, and the many levels of indirection between our test setup and the actual Node binary. It over-eagerly tried to write wrapper scripts and
cmdfiles. Eventually we arrived on something much simpler that worked:0x -- node node_modules/mocha/bin/mocha.js test/web-platform-tests/run-wpts.js --fgrep Range-mutations-dataChangeStaring at this allowed me to produce a few good results:
- Optimize string formatting web-platform-tests/wpt#51373
- Cache the document's base URL and serialized base URL #3837
- Add a cache for all [ReflectURL] IDL attributes #3838
- The first part of Some micro-optimizations centered around tagName/nodeName #3839
Eventually I felt like 0x's flamegraph viewer was not giving me enough insight, so I asked o3-mini-high what would be better, and it suggested
clinic flame. Armed with the knowledge of how to get at the Node binary, I was able to just adapt the command and it worked first try:clinic flame -- node node_modules/mocha/bin/mocha.js test/web-platform-tests/run-wpts.js --fgrep Range-mutations-dataChangeThis helped me create the rest of the changes in #3839. The viewer is a decent bit better, and the clinic flame docs (which I ended up going through with no AI assistance due to their onboarding in-product help) suggested adding
--no-turbo-inlining, which was somewhat helpful.For code-writing, for both web-platform-tests/wpt#51373 and the ASCII-uppercase/lowercase portions of #3839, I concurrently asked Claude Sonnet 3.7 thinking and ChatGPT o3-mini-high to generate me optimized versions of these functions. They both came up with basically the same code, and curiously both had an initial version which had a redundancy carried over from the input code, which they mentioned. I prompted them to eliminate the redundancy and they did so nicely.
Note that for the latter, Claude Sonnet 3.7 fell flat on its face at first, suggesting just minor syntax changes. After I chided it, it produced the correct optimized code (basically identical to what ChatGPT ended up producing in the other tab).
#3837 benefited from Cursor (auto mode)'s autocomplete and agent framework a good amount. After I refactored some code from the helper file into
Document-impl.jsmanually, it got the rest of the idea and I was mostly able to tab through it. I then used agent mode to update all the call sites, which worked nicely.However, Cursor screwed me over here for an extra 5-10 minutes as it suggested a bad import for the lib/jsdom/living/domparsing/serialization.js change, and it took me that long to debug what was going on. (Because serialization is used in the test harness output, so even error messages were broken.) We'll put that in the column of "errors that would be caught if jsdom was written in TypeScript".
#3838 did not really benefit from AI, as it was small and the context for that code (JS generating other JS) is pretty weird, so I suspect if I'd tried anything it would have gotten confused.
Where to from here
I'm still not seeing huge speedups on my local machine, despite zapping away big chunks of the flamegraph. Maybe CI is more evident, though, since my local machine was only in the 6-8 second range whereas CI sometimes exceeds the 60 second (90 second?) timeout.
At this point some of the more low-hanging fruit in the
Range-mutations-dataChange.htmlprofile is aroundtestharness.js'sget_stack()function. It seems like it should be possible to avoid getting the stack unless there's an assertion failure? That might be a pretty intrusive change totestharness.jsthough.I'm a little disappointed that we didn't find anything range-specific in this test, along the lines of what @Narretz found. Instead, the flamegraph seemed to indicate that the problem was just lots of assertions, or maybe lots of DOM node creation.
I wonder what profiling
Node-cloneNode.htmlwould show. It's currently disabled into-run.yaml. Maybe worth checking out in another round._preInsertValidity()was still relatively high on the stack, and so I just tried throwing that into ChatGPT and Claude. https://chatgpt.com/share/67d5602a-4aac-800c-8388-be764f812497 suggests some optimizations, but I wasn't convinced they would help, as the case that I believe we're dealing with is the element addition case, whereas most of the optimizations are around the document fragment case. (Or maybe I'm wrong and we're inserting elements into document fragments? The range tests are a little crazy.) It's probably worth trying a bit harder to see if there's juice there to squeeze.Part 2
After dinner I couldn't resist and got sucked into another 20 minutes of optimization, producing #3840. Again it was just a matter of staring at the flame graph. This time the work was pretty obvious, but Cursor was helpful in autocompleting the refactors. (Not using agent mode, just once I deleted the initialization from one place, it figured out easily that I wanted to move it to the getter.)
- added 2 commits that reference this issue
on Mar 15, 2025 5 remaining items
- added a commit that references this issue
on Mar 16, 2025 - added a commit that references this issue
on Mar 18, 2025 - added 2 commits that reference this issue
on Apr 13, 2025 In #3854 (comment) @asamuzaK suggests these timeouts on CI are due to memory problems.
This seems very likely to me. It explains why even locally, these tests run quickly but when run as part of the larger
npm run test:wptrun they take much longer.The problem might just be a lot of memory usage and fragmentation over time. But I suspect it is worse. I suspect that each
JSDOMinstance created leaks memory that is never fully released. We have seen this various times throughout jsdom's history, and it sometimes gets better and sometimes worse, often due to underlying fixes in the Node.js vm module. I most recently noticed it myself back in #1122 but we have a lot of reports.It's also notable that at least in one case we've had a bug that we squashed and added a test for:
jsdom/test/api/from-outside.js
Lines 45 to 55 in 1911c80
it("frees up callback handles passed to setTimeout", { timeout: 5000 }, () => { const timeoutWithGcFixturePath = path.resolve(__dirname, "./fixtures/timeout-with-gc.js"); const { status, stdout } = spawnSync("node", ["--expose-gc", timeoutWithGcFixturePath], { encoding: "utf-8" }); assert.equal(status, 0); const ratio = Number(stdout); assert(!Number.isNaN(ratio)); // At least 70% of the memory must be freed up. assert(ratio < 0.3); }); We could paper over this by restarting the test process every once in a while, but it would be better to find and fix the root cause.
Reacted by ExE Boss- added a commit that references this issue
on Apr 14, 2025 - added a commit that references this issue
on Sep 16, 2025 - added 2 commits that reference this issue
on Jan 15, 2026
We've been getting several CI failures recently from
Node-cloneNode.htmltaking longer than 70 seconds to pass. (Even on my local machine which is pretty beefy, it takes around 45 seconds.) I will disable it for now.Similarly, many tests involving ranges (e.g. under
dom/rangesorselection) take a very long time to pass. I will not be disabling those yet, although we might have to at some point.It'd be lovely if we could dig in to what is slow about these tests. I suspect that jsdom could be faster; maybe we have some O(n^2) algorithm or something. This seems especially likely for ranges.
If we think a given case is just a very slow test, then we could try to split it up by modifying the upstream WPT repo, e.g. like web-platform-tests/wpt#21881 did.