core(byte-efficiency): use lantern for opportunity estimates #4601
Conversation
| @@ -28,15 +30,14 @@ module.exports = [ | |||
| score: '<100', | |||
| extendedInfo: { | |||
| value: { | |||
| wastedKb: 14, | |||
| wastedKb: 45, | |||
There was a problem hiding this comment.
as discussed, can we add more wastedMs values to attempt to catch regressions on our side or Chrome's?
| * @param {!Audit.HeadingsResult} result | ||
| * @param {number} networkThroughput | ||
| * @return {!AuditResult} | ||
| * @param {!Array<{url: string, wastedByte: number}>} results |
There was a problem hiding this comment.
string to explain method/result?
|
|
||
| const wastedBytes = results.reduce((sum, item) => sum + item.wastedBytes, 0); | ||
| const wastedKb = Math.round(wastedBytes / KB_IN_BYTES); | ||
| const wastedMs = Math.round(wastedBytes / networkThroughput * 100) * 10; | ||
| const wastedMs = this.computeWasteWithTTIGraph(results, graph); |
There was a problem hiding this comment.
results don't have urls? (needed by computeWasteWithTTIGraph)
There was a problem hiding this comment.
but they do? :)
There was a problem hiding this comment.
oh, maybe Audit.HeadingsResult needs to be updated? cause right now it looks like it would be
{{results: number, headings: Audit.Headings, passes: boolean, debugString: string=, wastedKb: number, totalKb: number, potentialSavings: string}}
There was a problem hiding this comment.
yeah results is most definitely not a number, that's the array of items with {url: string, wastedBytes: number}
There was a problem hiding this comment.
Ah, I also missed that it was result.results.map(...)
patrickhulce
force-pushed
the
lantern_opp_estimates
branch
from
ea94556
to
6378f91
Compare
patrickhulce
force-pushed
the
lantern_opp_estimates
branch
from
6378f91
to
90f6651
Compare
|
anything else this is waiting on @brendankenny @paulirish ? |
I think we should do those as well. I'm fine with it being a separate PR, but I'd like to have our opportunities move to lantern at the same time (ie. same release). Also have a feeling that opps-by-metric will be landing a while from now. |
Definitely same release (3.0)
Not as confident 😄we've got 4 weeks 'til due date for this and with this PR inflight ~3 weeks that puts me needing to finish it this sprint 😆 |
closes #3618
This moves all byte efficiency audits to use lantern estimates. Render blocking resources and preload opportunities still use bespoke methods, but IMO anything else we do until we surface opportunities by metric would be wasted work anyhow. If others feel differently that it's important to update those too, happy to take a crack at it.
Overall approach: simulate the pessimistic TTCI graph as-is, modify all the resources to have the specified transfer size, re-simulate and report the difference