GoogleChrome / GoogleChrome/lighthouse

trace processor - cleanup and fixes ☂️

Open
#14,708 2 comments 0 reactions 1 assignee Claimed by @paulirish View on GitHub
P2
Dominant language
JavaScript
Stars
30.8k
Forks
9.8k
Avg merge
1d 20h
Merged PRs (30d)
19

Description

Forking off from #14287 …

## arch
* Refactor the traceprocessor flow. In short: determine the "inspected" pids/frames in one pass before doing all the subsetting.
* Verify we handle metric calculation of multiple navigations correctly. (For timespan mode)
* While trace-processor could organize all processes found in the trace, I think it's better to just instantly whittle down the events to the "inspected" process tree and frame tree. Drop everything else, so no metric calculation code needs to filter for themselves.
- or... _don't_ do this since [the processFilter](https://source.chromium.org/chromium/chromium/src/+/main:content/browser/devtools/protocol/tracing_handler.cc;l=819-841;drc=b3bdc5a24a0d87b4de66320c6d187aafd55840ca;bpv=1;bpt=1) is already doing this for us.
* handle the `pid` reuse case (however unlikely that is while tracing). Having just a map of `pid->tid` says nothing about the timing. Seems like we might need a temporal aspect to the tracking as well? Or just step though the trace, subsetting in chunks between any `FrameCommittedInBrowser` events.

## frames
* Clarify that frameEvents/frameTreeEvents are a subset of all events from that frame.
* Adopt [`isOutermostMainFrame`](https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3714815)?

### allframes metrics
* Nearly certain that [this fcpAllFrames calculation](https://github.com/GoogleChrome/lighthouse/blob/6909c6cc3a6b72b421fbedeabf06974734f7845d/core/lib/tracehouse/trace-processor.js#L723-L726) doesnt return the right value.
* #14760 is relevant..

## audit traceEvent usages
* Review all uses of `trace.traceEvents` as there's potentially a mistake handling pids/frames.
* Audit all uses of .args[.data].frame to see if there's a better way to ensure the data is reliably there.
* Validate all uses of startingPid are using it correctly. (They're probably not)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.