1948554 - Nightly is 12.5x slower than Chrome to parse 200K empty <script> elements and 2.5x slower to parse simplistic <script> elements

Status: RESOLVED WONTFIX

(Core :: DOM: HTML Parser, enhancement)

Description

[Mayank Bansal]

Attachment: scripts.HTML

Open attached testcase
It has 200K <script> elements

Nightly: https://share.firefox.dev/3QkqTyb (14s without the profiler)
Chrome: https://share.firefox.dev/411n5qz (6.5s)

Not a realistic testcase, but maybe it highlights some hotspots? As usual, feel free to WONTFIX/INVALID.

Comment 1

[Mayank Bansal]

Attachment: scripts_empty.HTML

200k empty <script> elements

Nightly: https://share.firefox.dev/3EHMYEw (12s)
Chrome: 950ms

Comment 2

[Mayank Bansal]

Attachment: chrome - 950ms.png

Comment 3

[Henri Sivonen (:hsivonen)]

These are document.write-eligible scripts, and Gecko does a bunch of stuff to cater for the possibility of document.write.

It would be interesting to compare with the same number of <script async src="data:text/javascript,a=1;"></script>

Comment 4

[Henri Sivonen (:hsivonen)]

And to set expectations: Unless there's a real-world case where this is a very bad problem, it's probably not worthwhile to try to make the work that caters to the possibility of document.write take less time on the main thread, which this seems to be about. (We already have some safeguards on the parser thread against excessive work on the parser thread in a scenario where actual document.write happens in a bad way excessively many time, and we could, in principle, add safeguards on the parser thread against doing excessive work also in a scenario where there's an excessive number of document.write-eligible scripts even if the don't actually perform bad document.writes. But it seems not useful to do that here, as what shows up on the profiler is the main-thread side.)

Comment 5

[Mayank Bansal]

Attachment: script_comment3_100K.HTML

Testcase based on the comment 3, 100K <script> elements.
Nightly: https://share.firefox.dev/4jYuijV (2.2s)
Chrome: https://share.firefox.dev/4gKDfKE (3.3s)

Comment 6

[Henri Sivonen (:hsivonen)]

As for what we could (I think; someone check my thinking) do to optimize the main thread side here:

Add a new for (;;) loop from https://searchfox.org/mozilla-central/rev/7b3f3fb5fd2cad8f348131498a35a91bef68b47b/parser/html/nsHtml5TreeOpExecutor.cpp#611 to the bottom of the method.

Add a flag hadDocumentWrite and make document.write set the flag to true.

Set the flag to false at https://searchfox.org/mozilla-central/rev/7b3f3fb5fd2cad8f348131498a35a91bef68b47b/parser/html/nsHtml5TreeOpExecutor.cpp#693

After the curly brace at https://searchfox.org/mozilla-central/rev/7b3f3fb5fd2cad8f348131498a35a91bef68b47b/parser/html/nsHtml5TreeOpExecutor.cpp#706 , add a check that if scriptElement is an internal (non-src) classic script and hadDocumentWrite is false continue the loop added above else return.

Comment 7

[Henri Sivonen (:hsivonen)]

Testcase based on the comment 3, 100K <script> elements.

Thanks. This confirms comment 3.

I think implementing comment 6 as written would still flood the event loop with 100000 runnables that would eventually return quickly each. Avoiding that kind of runnable flood would involve more special-casing than what comment 6 says.

Thank you for looking into this, but unless there's a real-world problem that this is a minimized test case of, I think it's not important to prioritize fixing this.

Comment 8

[Mayank Bansal]

Thanks for reviewing the testcase!
I will close this bug as wontfix - plenty other more realistic bugs exist.