[Feature] Restore polling: 'mutation' for the same level of waitForSelector efficiency as Puppeteer

[naruaway]

I realized that Puppeteer has much faster waitForSelector than Playwright since Puppeteer utilizes MutationObserver.
I created a repro here, which demonstrates that Puppeteer is around 20x faster than Playwright in this case.

This repro also includes some description about why Playwright is slow in this case.

Is there any plan to get back polling: 'mutation' removed in this PR? It's really unfortunate to see this inefficiency just to deal with Shadow DOM, which is not used everywhere.

I haven't investigated why Puppeteer can use MutationObserver but we might steal their strategy if they properly also support Shadow DOM with MutationObserver somehow.

[yury-s]

Your test is measuring time from the browser _appearanceTimeStamp and time from the node process waitFinishTimeStamp which are not aligned, you should use one or the other. If you want measure node side timing, just measure the time on the node side. Moreover, the test uses 3000 timeout that increases its nondeterminism. Taking all of that into account we can't trust the test result as a measure of the waitForSelector performance.

[naruaway]

@yury-s, thanks for checking this out, I think your concern about potential issue of using time in browser and Node.js is valid.
I fixed my repro to only use performance.now() in Node.js and confirmed that still Puppeteer reacts much faster.

Moreover, the test uses 3000 timeout that increases its nondeterminism.

This is very intentional, the performance difference happens only when we do enough sleep, which is coming to the hard coded retry intervals in Playwright

we can't trust the test result as a measure of the waitForSelector performance.

I would like to clarify that, even if we cannot trust my benchmark, I think we know waitForSelector in Playwright cannot be more efficient than Puppeteer since the latter is using MutationObserver and the former is relying on periodic retries.
My question is that the removal of MutationObserver in Playwright was very intentional or not. If Playwright team thinks that repeated intervals is fine for most of use cases, I think it's fine but I could not find any discussions around this (I mean this PR) so I suspected this is unintentional even for Playwright team. I really love Playwright and I was using Playwright over Puppeteer since I believed Playwright has the same efficiency as Puppeteer for this kind of low level methods but I just found it's not. I think additional 300ms of Playwright can be visible for some use cases of Playwright although most of E2E test use cases it should be fine since the target DOM element should appear relatively faster anyway

[dgozman]

@naruaway Thank you for the clarifications.

Playwright uses a backoff strategy for slow pages and does not retry your selector in a loop or upon every DOM mutation. This way, your tests can better utilize CPU and run in parallel, instead of a single test pretty much fully occupying a single core.

We believe this is a better strategy, because it only marginally affects the amortized time of the test suite, while allowing for better parallelism.

[naruaway]

Yeah I see that there is interesting trade offs here (pull or push reactivity!) and your reasoning makes sense 👍

Playwright might be more efficient for some tests and Puppeteer might be more efficient for other tests but I can imagine the backoff strategy will not slow down tests for typical E2E test cases scenario since when multiple retries are happening, that part is already slow in app side anyway.

Thanks for the clarification