1521931 - Parser should yield if there is a pending high priority event before first contentful paint

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, enhancement, P2)

Description

[Olli Pettay [:smaug][[e-mail]]]

In order get first contentful paint sooner, we shouldn't try to continue synchronously create DOM if there is a pending vsync.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: parser_blocking.diff

https://treeherder.mozilla.org/#/jobs?repo=try&revision=24542ef355fb7fb48adf74af1609d39419d82095

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: parser_blocking_child_only.diff

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a26960b425dab17278e267c96a824fbfefd4e2d

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9038362 [details] [diff] [review]
parser_blocking_child_only.diff

Let's see what Henri thinks about this. The idea is that we really must paint asap if we haven't got first contentful paint yet.
Especially we need to try to paint _before_ some slow scripts.

(bug 1521955 is about after scripts basically, but that seems to cause still more test failures to look at)

Comment 4

[Henri Sivonen (:hsivonen)]

Comment on attachment 9038362 [details] [diff] [review]
parser_blocking_child_only.diff

Review of attachment 9038362 [details] [diff] [review]:
-----------------------------------------------------------------

r+ if the requested comments are added to the code

::: parser/html/nsHtml5StreamParser.cpp
@@ +127,5 @@
> +                doc->Dispatch(TaskCategory::Network, flusher.forget()))) {
> +          PROFILER_ADD_MARKER("HighPrio blocking parser flushing(1)", DOM);
> +          return NS_OK;
> +        }
> +      }

Please add a comment that the code is here and not at the top of RunFlushLoop() in order to reuse the runnable.

::: parser/html/nsHtml5TreeOpExecutor.cpp
@@ +61,5 @@
> +              doc->Dispatch(TaskCategory::Network, flusher.forget()))) {
> +        PROFILER_ADD_MARKER("HighPrio blocking parser flushing(2)", DOM);
> +        return NS_OK;
> +      }
> +    }

Please add a comment that the code is here and not at the top of RunFlushLoop() in order to reuse the runnable.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

ok, will do, thanks.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: parser_blocking_child_only_2.diff

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/869a4e8b520b
Parser should yield if there is a pending high priority event before first contentful paint, r=hsivonen

Comment 8

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/869a4e8b520b

Comment 9

[Ionuț Goldan [:igoldan]]

We won some very big performance improvements thanks to this bug! \0/

== Change summary for alert #19075 (as of Wed, 30 Jan 2019 13:06:56 GMT) ==

Improvements:

37% raptor-tp6-docs-firefox raptor-tp6-docs-firefox-fcp osx-10-10 opt 2,135.67 -> 1,356.25
36% raptor-tp6-slides-firefox raptor-tp6-slides-firefox-fcp osx-10-10 opt 2,424.92 -> 1,543.17
32% raptor-tp6-docs-firefox osx-10-10 opt 2,453.17 -> 1,669.10
21% raptor-tp6-slides-firefox osx-10-10 opt 3,114.61 -> 2,475.20
15% raptor-tp6-docs-firefox windows10-64-qr opt 1,447.98 -> 1,232.27
15% raptor-tp6-docs-firefox windows10-64 opt 1,428.34 -> 1,220.77
13% raptor-tp6-docs-firefox windows10-64 pgo 1,342.19 -> 1,172.79
12% raptor-tp6-docs-firefox windows7-32 opt 1,349.17 -> 1,182.40
12% raptor-tp6-docs-firefox windows7-32 pgo 1,281.96 -> 1,128.11

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=19075