-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
misc(lantern-collect): rebaseline to include new ttfb metric #15069
Conversation
@@ -1,81 +1,81 @@ | |||
{ | |||
"sites": [ | |||
{"url": "http://m.iciba.com", "roughEstimateOfFCP": 1743, "optimisticFCP": 1743, "pessimisticFCP": 1743, "roughEstimateOfFMP": 1743, "optimisticFMP": 1743, "pessimisticFMP": 1743, "roughEstimateOfTTI": 7517, "optimisticTTI": 3337, "pessimisticTTI": 11697, "roughEstimateOfSI": 9842, "optimisticSI": 4764, "pessimisticSI": 5264, "roughEstimateOfLCP": 9781, "optimisticLCP": 9341, "pessimisticLCP": 10222}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this PR because I noticed a delta in this file when running node core/scripts/lantern/update-baseline-lantern-values.js
. It was simply the new TTFB metric being added (nothing else changed, as expected). The other metrics do not show up yet because the necessary data in the trace is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm why is TTFB added but not LCP load start/end? I would imagine that none of the sites had a baseline TTFB before now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's likely no DOMNodeId in these old traces, so LCPImageRecord will fail to determine load start/end.
ttfb doesn't rely on any new trace stuff. so when the lantern baseline values are updated, all the data is there to compute it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, they don't even have LargestImagePaint::Candidate
events :P
@@ -243,6 +243,11 @@ evaluateAndPrintAccuracy('largestContentfulPaint', 'optimisticLCP'); | |||
evaluateAndPrintAccuracy('largestContentfulPaint', 'pessimisticLCP'); | |||
evaluateAndPrintAccuracy('largestContentfulPaint', 'roughEstimateOfLCP'); | |||
|
|||
// TODO: enable when new traces are collected (also, do calls to findAndPrintWorst10Sites) | |||
// evaluateAndPrintAccuracy('timeToFirstByte', 'roughEstimateOfTTFB'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
saving future us some time here
follow up to #14941