1157924 - Identify root of the dirty subtree during reflow and attach as metadata to reflow markers

Status: RESOLVED INVALID

(DevTools :: Performance Tools (Profiler/Timeline), defect, P3)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Right now, we can see when layout is taking a long time, or there are repeated sync layouts within a JS run-to-completion.

But we don't know which subset of elements on the page are reflowing, or if it the whole page, etc.

I'd like to have a unique selector to

* The root of the dirty subtree that needs to be reflowed

* The element that was modified and triggered the layout invalidation

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Am I operating on false assumptions about how layout works here? Does this make sense? Need rewording? What would be involved in getting this data?

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Nick Fitzgerald [:fitzgen] from comment #0)
> * The root of the dirty subtree that needs to be reflowed

I think you mean "the *narrowest* partially-dirty subtree" (because when we reflow, the whole frame tree is technically a partially-dirty subtree that needs to be reflowed).

I think (?) this "root of the narrowest partially-dirty subtree" would be the root-most frame that satisfies either of the following conditions:
 (1) has the NS_FRAME_IS_DIRTY set
 or:
 (2) has >1 children with NS_FRAME_IS_DIRTY or NS_FRAME_HAS_DIRTY_CHILDREN set.

I haven't worked with these dirty flags a ton; CC'ing dbaron so he can correct me if I'm mistaken.

> * The element that was modified and triggered the layout invalidation

This may be based on a mistaken assumption.  In general, there will be more than one element that was modified.  You can modify many elements, and we'll hold off on actually doing reflow until the next refresh-driver tick, or until you trigger a layout flush by requesting information (like certain layout-dependent computed style values, or .offsetHeight).

So I'm not sure about what you want for that second point.

Comment 3

[Daniel Holbert [:dholbert]]

So I think what you're going for, RE dirty subtrees, is something like the following:
 (In my diagrams, asterisk means "dirty", and single-quote means "has dirty children")

SCENARIO 1:
     A'
    / \
   B'  C
  / \
 D*  E*

"B" would considered the root of the narrowest dirty subtree.


SCENARIO 2: (same as 1, except E is not dirty)
     A'
    / \
   B'  C
  / \
 D*  E

Here, "D" would be considered the root of the narrowest dirty subtree.

Is that what you're going for?

Comment 4

[Daniel Holbert [:dholbert]]

Or alternately, are you just interested in a list of all the frames that were marked as dirty? (i.e. in Scenario 1, do you really want to be reporting information about node "B", or do you want to be reporting information about "D" and "E"?)

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So one of the other things here is that we may well spend substantial time on things that weren't dirty, because the things that were dirty caused layout changes that moved them around (or the layout code wasn't optimized enough to know that they wouldn't move).

After some discussion in person with fitzgen, I was thinking it might be worth getting layout to expose the start and end of reflow on certain elements -- probably not all elements -- so that devtools get notified (when in timeline view!) at the start and end of nsBlockFrame::Reflow, and that it then has UI to show which element was being reflowed when (i.e., the frame's mContent).  (If that looks useful, we'd probably want to add nsTableFrame and nsFlexContainerFrame, but might not want to add everything because it could just get too cluttered.)  If the right side of the timeline view could then display that information in a useful way (e.g., with the element highlighter), that might be a useful start.

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Daniel Holbert [:dholbert] from comment #3)
> Is that what you're going for?

yes, that is exactly what I was going for with the "root of the dirty subtree", thanks for the diagrams :)

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #5)
> So one of the other things here is that we may well spend substantial time
> on things that weren't dirty, because the things that were dirty caused
> layout changes that moved them around (or the layout code wasn't optimized
> enough to know that they wouldn't move).
> 
> After some discussion in person with fitzgen, I was thinking it might be
> worth getting layout to expose the start and end of reflow on certain
> elements -- probably not all elements -- so that devtools get notified (when
> in timeline view!) at the start and end of nsBlockFrame::Reflow, and that it
> then has UI to show which element was being reflowed when (i.e., the frame's
> mContent).

Our plan is to automatically collapse markers that are wholly contained within another marker (eg "parse HTML" within "JS run-to-completion") and let you expand to get more details if you are interested.

With this collapsing, perhaps we could just emit more nested markers for each block frame that gets reflowed and let the collapsing hide the potentially overwhelming dump of extra markers unless someone is debugging a particular reflow and expands.

Comment 8

[Jordan Santell [:jsantell] [@jsantell]]

Scoping this out, and maybe more info here that we can expose: http://www-archive.mozilla.org/newlayout/doc/reflow.html

Comment 10

[Victor Porof [:vporof][:vp]]

Triaging. Filter on ADRENOCORTICOTROPIC (yes).

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

This report is related to the old DevTools Profiler.
The Performance panel now points to the new Firefox profiler available at profiler.firefox.com
Closing as Invalid bug

Comment 12

[Julien Wajsberg [:julienw]]

Note: if you think something similar to this would be useful in the new profiler, please file a new bug :-) thanks