core: add time-to-first-byte and lcp-breakdown #14941
Conversation
|
Can you run on WPT from this branch so this doesn't have to land first? |
|
We are getting the metrics from the Lighthouse within WPT so it would be nice to land this first (assuming WPT is pulling the latest release). It might be possible to do the same analysis from the trace artifacts returned by WPT though so it may not be 100% necessary to land this. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
|
||
| # Install dependencies | ||
| sudo apt-get update | ||
| sudo apt-get install -y xvfb nodejs google-chrome-stable google-cloud-sdk git zip | ||
| sudo npm install -g yarn | ||
|
|
||
| # Add a lighthouse user | ||
| sudo useradd -m -s $(which bash) -G sudo lighthouse |
There was a problem hiding this comment.
I couldn't figure out why, but there is already a lighthouse user by this point in the script 🤷
There was a problem hiding this comment.
Were you running on a fresh machine or re-using one?
There was a problem hiding this comment.
Pretty sure it was a fresh machine, I went through a few cycles of creating and removing machines named lantern-collect-instance
There was a problem hiding this comment.
That's weird. Doesn't seem like it could work without this line.
The other gcp script does || echo "Lighthouse user already exists!", let's do that here too.
There was a problem hiding this comment.
weirdly I got a lot of echoed "Lighthouse user already exists!" messages when running the regular gcp collect script as well. I also couldn't figure out why :)
There was a problem hiding this comment.
You were both manually destroying the instance in the GCP dashboard each time? IIRC the script will silently-pass the instance-creation command if it already exists with those parameters.
There was a problem hiding this comment.
Yeah I was manually destroying it
|
I've added tests for everything, and I think this PR is landable now. There might be some flakiness with the smokes but that should be pretty easy to patch. The only thing left is to update the lantern database (this is why CI basics is failing). Once the rest of this PR is settled I will update the database and expectations. Or we could just exclude these new "metrics" from the lantern test for now. |
|
|
||
| const lcpRecord = await LCPImageRecord.request(data, context); | ||
| if (!lcpRecord) { | ||
| return {ttfb, loadStart: ttfb, loadEnd: ttfb}; |
There was a problem hiding this comment.
I think we might want to make loadStart/loadEnd undefined in these cases. The optimize-lcp article mentions making loadDelay/loadTime 0 (so setting these to ttfb), but that's technically only in the case where no resource outside the original HTML is needed for displaying the text. We don't track how fonts or dynamically added text were loaded, so we don't really know if there was another resource needed or not.
FWIW that would follow the approach taken in UKM, where loadStart/loadEnd are null/not recorded when the LCP is text/video (with implicit possible future TODOs for if/when video LCP happens and if text LCP timing is ever worth looking at)
There was a problem hiding this comment.
I was somewhat worried that this would mess with the lantern metric comparisons. This might be reason enough to just leave it out of the lantern tests for this PR. I will keep the test data of course.
There was a problem hiding this comment.
Update, looks like lantern testing handles this just fine, I think I might still remove the lantern test stuff from this PR since we might want to regenerate the WPT again, and should probably use a real device.
| }, | ||
| "roughEstimateOfTTFB": {}, |
There was a problem hiding this comment.
is the test failure that these need to be repopulated?
There was a problem hiding this comment.
Yes, we need to either update the lantern test database for that to happen, or remove these new "metrics" from lantern testing.
This PR adds 3 new "metrics" to Lighthouse: TTFB,
lcpLoadStartandlcpLoadEndlcpLoadStartandlcpLoadEndare not the same as LCP load delay/load time because they are based at the timeOrigin rather than the end of the previous LCP phase.The current simulated throttling variants of
lcpLoadStartandlcpLoadEndare just the observed metric value weighted by the throttled LCP value. As we discussed, this will be our starting point for analysis.The current simulated throttling variant of TTFB is based on this calculation in the network analyzer:
lighthouse/core/lib/dependency-graph/simulator/network-analyzer.js
Line 210 in 2b8caf2
Lantern correlations: