Open Bug 1157924 Opened 6 years ago Updated 2 years ago

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

Categories

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

defect

Tracking

(Not tracked)

People

(Reporter: fitzgen, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [devtools-platform])

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
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?
Flags: needinfo?(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.
Flags: needinfo?(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?
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"?)
Flags: needinfo?(nfitzgerald)
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.
(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 :)
Flags: needinfo?(nfitzgerald)
(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.
See Also: → 1158616
See Also: → 929469
Scoping this out, and maybe more info here that we can expose: http://www-archive.mozilla.org/newlayout/doc/reflow.html
Summary: Identify root of the dirty subtree during reflow and attach as metadata to reflow markers → [marker] Identify root of the dirty subtree during reflow and attach as metadata to reflow markers
See Also: → 1131661
Summary: [marker] Identify root of the dirty subtree during reflow and attach as metadata to reflow markers → [marker] Reflow: Identify root of the dirty subtree during reflow and attach as metadata to reflow markers
Duplicate of this bug: 1169564
Whiteboard: [devtools-platform]
Triaging. Filter on ADRENOCORTICOTROPIC (yes).
Priority: -- → P3
Summary: [marker] Reflow: Identify root of the dirty subtree during reflow and attach as metadata to reflow markers → Identify root of the dirty subtree during reflow and attach as metadata to reflow markers
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.