Open Bug 1388157 Opened 8 years ago Updated 3 years ago

Time to non-blank Paint should be aligned with the proposed "first-contentful-paint"

Categories

(Core :: Layout, enhancement, P3)

Product:
Core
Component:
Layout
Type:
enhancement

Tracking

()

Tracking Flags:
Tracking Status
firefox57 --- wontfix

People

(Reporter: zbraniecki, Unassigned)

Details

In bug 1307242 :mstange implemented Time to non-blank Paint timestamp, which later has been exposed as `performance.timing.timeToNonBlankPaint` in bug 1377251. We increasingly use this timestamp in our benchmarks like tp6 and soon tpaint/ts_paint. At the same time, there's a spec proposal "Time To First Paint" - https://github.com/WICG/paint-timing This proposal currently brings two events: 1) "first-paint" 2) "first-contentful-paint" The difference between them, as far as I understand, is that (1) should be recorded when any painting happens in reaction to the new website being loaded - for example new background color. The (2) should be recoreded when the first paint with DOM elements is completed. :smaug looked into this to give us an expert feedback in bug 1381988 and he concluded that currently, `timeToNonBlank` measures too early compared to what we want to measure (it is recorded before composition/rendering step) and it measures earlier than in the spec proposal. According to :smaug the closes thing we have is MozAfterPaint's event.paintTimeStamp and :jmaher pointed out that we need to be careful to record only if the paint is not empty. I'd like to suggest that we align the `performance.timing.timeToNonBlankPaint` (and firstNonBlankPaint event) with the spec's `first-contentful-paint`.
NI on :mstange who said he'll look at it.
Flags: needinfo?(mstange)
There are two orthogonal issues here: (1) Capturing the timestamp after rasterization + compositing, and (2) triggering it for "contentful" paints instead of "non-blank" paints. The first one seems worthwhile. I can guide anybody who wants to implement this, but I don't have cycles to do it myself at the moment. The second one needs a product / metrics decision. For Quantum release criteria, it was decided that nonBlankPaint is what we want to measure.
Flags: needinfo?(mstange)
I would be interested in moving the timestamp into the right location, if you can mentor me through this. If I understand correctly (2) means that timeToNonBlankPaint will be the `firstPaint` from the spec proposal, rather than firstContentfulPaint, is that correct?
Flags: needinfo?(mstange)
That seems to be the case, yes. And the spec also calls for (1) to be done, "the timestamp should be the latest timestamp the browser is able to note in this pipeline (best effort)."
Flags: needinfo?(mstange)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #3) > I would be interested in moving the timestamp into the right location, if > you can mentor me through this. That's great. The main challenge here is tracking the first paint from display list building through the layers transaction to the compositor and then back to the right document in the right content process. A composition can include layer trees from multiple process, and each of those layer trees can include painted contents from multiple documents. I think something like this would work: - When building the display list, keep track of a list of documents (nsTArray<nsCOMPtr<nsIDocument>>) for which this paint is the first non-blank paint. This list can be stored on the nsDisplayListBuilder. - At this point we also know the transaction ID of the layers transaction that will be used for the current paint. - Store the list in some global place, associated with the transaction ID, because the nsDisplayListBuilder will go away at the end of this paint. - When the content process is notified about a completed composition, it gets the composition timestamp and the latest transaction ID whose effects where visible in that composition. http://searchfox.org/mozilla-central/rev/c329d562fb6c6218bdb79290faaf015467ef89e2/view/nsView.cpp#1089 Here we need to notify all documents whose first non-blank-paint was during a transaction that's <= aTransaction. The "global place" where you could store the list of documents could be the root nsPresContext of the painted document. Because the root pres context is what gets notified of composites.
Changing where we collect the non-blank paint timestamp is going to affect perf numbers. How well equipped are we to handle that change? Do we even want to make that change? Or should we have both the "displaylist" timestamp and the "composition" timestamp, use the former for all the things that currently use it, and the latter for Talos?
Flags: needinfo?(benjamin)
Thanks Markus. Unfortunately, I don't believe I'm the right person to tackle that. I've never worked with display lists and have little understanding of the code you're talking about. I believe we need to prioritize it (I'd argue it would be good to get it soon to improve our talos testing infra) and find someone who understands the code. Lastly, I'm wondering if when we work on that, it would be possible to also add the "contentful" timestamp.
I am for implementing the paint-timing metrics and eventually exposing them to content. If I understand your argument correctly, you argue that contentful-paint is closer to what tp6 is expecting as "first paint" milestone; correct? I would not change TTNBP as it is something between first-paint or contentful-paint spec but simply add new metrics. It doesn't include non-default background-color, which first-paint does; and it has no sense of text/images/svg content as contentful-paint should. TTNBP is still good for telemetry to track it in relation to any new page load metrics. As Markus pointed out, the biggest challenge for paint timing is to include the compositing time; which I would love to see solved in an efficient way. Knowing when paint happens can help with a lof of other metrics; including getting end-to-end input latency measures.
TP6 has so little historical data that a change to any metric is acceptable.
Flags: needinfo?(benjamin)
so, the challenge now is that we have tpaint/tspaint calculating the wrong thing and we don't have a good replacement for it because: a) TTNBP is too early (lacks composition) b) MozAfterPaint.paintTimeStamp is not exposed to content In order to get the new metrics we'll need someone with good understanding of the code, and I'm not sure if we can hope to get anyone to work on that now :( Without that, we're stuck with making decisions based on flawed metrics when working on patches for UI chrome since none of tpaint, ts_paint nor sessionrestore measures the things we should be measuring against - when does the chrome become visible to the user. I agree that we can do this as a new metric, but I'm not sure how to move forward to get it prioritized.
> so, the challenge now is that we have tpaint/tspaint calculating the wrong thing I'd say it just trades accuracy for simplicity. For best accuracy a high speed camera would need to be pointed at the test machine's display. Apart from the goal on improving the numbers, are there known issues that this addresses, like excessive composite times visible in tests?
> For best accuracy a high speed camera would need to be pointed at the test machine's display. I doubt that many engineers working on a patch that affects browser.xul would go this route. But they will test against talos. > Apart from the goal on improving the numbers, are there known issues that this addresses, like excessive composite times visible in tests? Not that I know of. We just got the FNBP and wanted to switch talos to it to measure the right thing but it seems that it's a bit more complicated.
> I doubt that many engineers working on a patch that affects browser.xul would go this route. But they will test against talos. I hoped this illustrated the other extreme, where the effort for accuracy needs very elaborate setups. > Not that I know of. We just got the FNBP and wanted to switch talos to it to measure the right thing but it seems that it's a bit more complicated. It doesn't need to be necessarily more complicated. If there are no known issues with expensive compositing I'd consider TTFNBP being enough while we figure out a metric that is more accurate.
(In reply to :Harald Kirschner :digitarald from comment #8) > I am for implementing the paint-timing metrics and eventually exposing them > to content. > > If I understand your argument correctly, you argue that contentful-paint is > closer to what tp6 is expecting as "first paint" milestone; correct? Oh, you're right, that's what he is arguing. Which means I misunderstood what he was asking for. I thought he was arguing to align our TTNBP with first-paint from that spec, especially the part about grabbing the timestamp when the pixels arrive on the screen, instead of during display list construction. > I would not change TTNBP as it is something between first-paint or > contentful-paint spec but simply add new metrics. It doesn't include > non-default background-color, which first-paint does; and it has no sense of > text/images/svg content as contentful-paint should. TTNBP is still good for > telemetry to track it in relation to any new page load metrics. I agree with this part. > As Markus pointed out, the biggest challenge for paint timing is to include > the compositing time; which I would love to see solved in an efficient way. > Knowing when paint happens can help with a lof of other metrics; including > getting end-to-end input latency measures. Ok, so I'd like to rephrase my question: If we change TTNBP to include composition, how will that impact everything that already makes use of TTNBP? I don't want to throw a wrench into our release criteria, for example.
(In reply to :Harald Kirschner :digitarald from comment #13) > It doesn't need to be necessarily more complicated. If there are no known > issues with expensive compositing I'd consider TTFNBP being enough while we > figure out a metric that is more accurate. The problem with excluding painting + compositing from the measurement is that any impact from webrender (positive or negative) won't be measured, once it lands.
> Oh, you're right, that's what he is arguing. Which means I misunderstood what he was asking for. > I thought he was arguing to align our TTNBP with first-paint from that spec, especially the part about grabbing the timestamp when the pixels arrive on the screen, instead of during display list construction. Was I? :) Here's my assumption: tp6, tpaint and ts_paint are aiming to measure the time it takes from the invocation of a command (load website, open window, start browser respectively) to the moment the pixels with the content are drawn on the screen. The automation should be as close as possible to the values we'd get from external testing (via hi-speed camera) Two notes: a) The testing will always be slightly worse than external measuring because it won't include the time it takes from the moment we send the pixels to the driver to the moment it gets painted on the screen. And it's ok, since it should be insignificant and is outside of our control. b) There is a different between when we paint *any* of the DOM and when the website is visually-complete. The website may be still loading, and above-the-fold content may be still loading (images?), and maybe the website's UI is not interactive yet (the event handlers are not hooked). It's ok as well. It doesn't affect tpaint/ts_paint since we paint the whole UI together and there's no easy way to measure that yet, and the WICG is planning for "visually-complete" timestamp later, which we could add as well. When they do, both will be useful for us. The way I understand it is: 1) MozAfterPaint - randomly (due to async) fired event callback after a paint event is completed. Pretty unreliable. Used by tpaint, ts_paint. 2) TimeToNonBlankPaint - consistent timestamp that captures several stages of painting, but misses others. Used by tp6. 3) MozAfterPaint's event.paintTimeStamp - consistent timestamp that measures all stages of painting, but is chrome-only and not-standardized, and may be fired for blank image before anything is ready to be painted, so the user has to know which MozAfterPaint is for the paint with anything in it. 4) WICG's PaintTiming 'firstPaint' - very similar timestamp to (3) - captures the timestamp when the browser finished preparing the image and submitted it for painting to the driver, and it's the first paint that contains anything based on the website data (like, background color). 5) WICG's PaintTiming 'firstContentfulPaint' - simlar to (4) but measures the timestamp of the first completed paint that includes any of the DOM in it. I believe that in most cases (4) and (5) will be the same paint, unless on a slow connection. If I understand correctly, TTNBP is currently closer to 'first-paint', since it doesn't care if any DOM got laid out for the paint it takes the timestamp for, but it does measure the first paint for the new document, so doesn't have the risk of measuring a blank paint. I believe that for tp6, tpaint and ts_paint (and possibly others), the most important one would be (5), but it seems to me also that in our talos infra when loading local websites, it's very unlikely that (5) and (4) will be different, so if (4) is easier to get, we should start with settling around (4). My point is also, that (2) is inherently inferior for measuring the tp6, tpaint and ts_paint. > The problem with excluding painting + compositing from the measurement is that any impact from webrender (positive or negative) won't be measured, once it lands. That's true, but I believe there's more. We basically black ourselves out of any changes that impact the perceived performance if it afffects stages not covered by the TTNBP. WebRender is of course the most drastic one, but I can see other changes (layers, costly CSS translucent compositions?) that we are and will be landing as part of photon that are completely missed in our talos testing. And of course any other optimizations that we may do in composition stages that target 57 and may improve/regress performance on tp6 will also be missed. I'm inclined to try to use the SuperPowers thing within the tpaint (which is in content) to at least capture MozAfterPaint.paintTimeStamp which :smaug indicates is better, but that doesn't help tp6, doesn't unify our measurements and is still off.
Priority: -- → P3

(In reply to Markus Stange [:mstange] from comment #2)

There are two orthogonal issues here:
(1) Capturing the timestamp after rasterization + compositing, and
(2) triggering it for "contentful" paints instead of "non-blank" paints.

In the meantime, bug 1298381 has implemented "contentful" paint detection. And in bug 1506976 I'm adding a measurement for the "compositeEnd" timestamp of the contentful paint.

The resulting value will be usable for any performance tests that measure pageload. They should not be used for startup or window opening tests; for those, please continue to use MozAfterPaint event listeners and event.paintTimeStamp.

Mass-removing myself from cc; search for 12b9dfe4-ece3-40dc-8d23-60e179f64ac1 or any reasonable part thereof, to mass-delete these notifications (and sorry!)

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.