fix: do not enable invalidation tracking in performance traces - #2500
Open
thomasbachem wants to merge 1 commit into
Open
fix: do not enable invalidation tracking in performance traces#2500thomasbachem wants to merge 1 commit into
thomasbachem wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
performance_start_traceenablesdisabled-by-default-devtools.timeline.invalidationTracking. On a page that invalidates style heavily, that category fills the trace buffer within a few hundred milliseconds, so the recording stops before the interaction being measured finishes, and nothing in the result says so.The comment above the list says to keep it in sync with
TimelineController.tsand Lighthouse's trace gatherer. Neither enables it: DevTools annotates itdefault: off, experiment: timelineInvalidationTracking, Lighthouse does not carry it at all. This drops it from the default list to match.On the same interaction, before and after: the trace goes from covering the first 192 ms to all 1663 ms of it,
dataLossOccurredstops being set onTracing.tracingComplete, and the payload drops from 199 MB to 38 MB. The issue has a self-contained page that shows the same in stock Chrome, 219 MB with data loss against 0.6 MB without the category. The traced page also runs roughly 5x slower with the category on, so the timings that do survive are distorted.Worth weighing before merging: the events are not dead.
LayoutShiftsHandlerfolds their node ids intobackendNodeIds, whichDetailsViewresolves to make the Performance panel's invalidation list clickable. No insight reads the arrays orbackendNodeIds, so no tool output changes, but a trace saved withfilePathand reopened in DevTools would lose its invalidations view. A default DevTools recording does not have that view either, it is experiment-gated there. If keeping it reachable from here matters, an opt-in parameter would suit better than this removal and I am happy to redo it that way.No test covers this. The failure needs a real page under enough invalidation churn to fill the buffer, and asserting the array's contents would only restate the implementation, so I left the tests alone. Nothing currently pins the category list.
Fixes #2499