core(byte-efficiency): replace pessimistic graph with optimistic #15651

adrianaixba · 2023-11-30T23:25:57Z

notes: go/primoprio-follow-up

adamraine · 2023-12-01T00:05:11Z

core/test/results/sample_v2.json

@@ -4457,7 +4457,7 @@
      "warnings": [],
      "metricSavings": {
        "FCP": 0,
-        "LCP": 300
+        "LCP": 610


Now this is surprising. LCP savings should be going down not up.

I did some investigation using PageDependencyGraph.printGraph and here are the results for uses-text-compression LCP (may want to paste into a different viewer to see better):

######## OPTIMISTIC ====== | http://localhost:10200/dobetterweb/dbw_tester.html = | cpu ===== | http://localhost:10200/dobetterweb/dbw_tester.css?delay=100 ===== | http://localhost:10200/dobetterweb/unknown404.css?delay=200 ================== | http://localhost:10200/dobetterweb/dbw_tester.css?delay=2200 ===== | http://localhost:10200/dobetterweb/dbw_disabled.css?delay=200&isdisabled ======================== | http://localhost:10200/dobetterweb/dbw_tester.css?delay=3000&capped ================ | http://localhost:10200/dobetterweb/dbw_tester.css?delay=2000&async=true ============================ | http://localhost:10200/dobetterweb/dbw_tester.css?delay=3000&async=true ========= | http://localhost:10200/dobetterweb/dbw_tester.js ========= | http://localhost:10200/dobetterweb/empty_module.js?delay=500 = | cpu ================================================ | http://localhost:10200/dobetterweb/fcp-delayer.js?delay=5000 ============ | http://localhost:10200/dobetterweb/third_party/aggressive-promise-polyfill.js ====== | http://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js =========== | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?isr1 = | cpu =========== | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?isr2 =========== | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?isr3 =========== | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg = | cpu ========== | cpu ======= | http://localhost:10200/dobetterweb/dbw_tester.css?scriptActivated&delay=200 ========= | http://localhost:10200/dobetterweb/dbw_tester.html ============================= | http://localhost:10200/dobetterweb/lighthouse-1024x680.jpg?lcp&redirect=lighthouse-1024x680.jpg%3Fre ======= | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?iar2 == | cpu ===== | http://localhost:10200/dobetterweb/empty.css ================ | http://localhost:10200/dobetterweb/lighthouse-1024x680.jpg?redirected-lcp ######## PESSIMISTIC ====== | http://localhost:10200/dobetterweb/dbw_tester.html = | cpu ===== | http://localhost:10200/dobetterweb/dbw_tester.css?delay=100 ===== | http://localhost:10200/dobetterweb/unknown404.css?delay=200 ================= | http://localhost:10200/dobetterweb/dbw_tester.css?delay=2200 ===== | http://localhost:10200/dobetterweb/dbw_disabled.css?delay=200&isdisabled ======================== | http://localhost:10200/dobetterweb/dbw_tester.css?delay=3000&capped ================ | http://localhost:10200/dobetterweb/dbw_tester.css?delay=2000&async=true ============================ | http://localhost:10200/dobetterweb/dbw_tester.css?delay=3000&async=true ========= | http://localhost:10200/dobetterweb/dbw_tester.js ========= | http://localhost:10200/dobetterweb/empty_module.js?delay=500 = | cpu =============================================== | http://localhost:10200/dobetterweb/fcp-delayer.js?delay=5000 ============ | http://localhost:10200/dobetterweb/third_party/aggressive-promise-polyfill.js ====== | http://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js ====== | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?iar1 =========== | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?isr1 = | cpu =========== | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?isr2 =========== | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?isr3 =========== | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg ================================================= | http://localhost:10200/dobetterweb/lighthouse-rotating.gif = | cpu ========= | cpu ======= | http://localhost:10200/dobetterweb/dbw_tester.css?scriptActivated&delay=200 ========= | http://localhost:10200/dobetterweb/dbw_tester.html ============================= | http://localhost:10200/dobetterweb/lighthouse-1024x680.jpg?lcp&redirect=lighthouse-1024x680.jpg%3Fre ======= | http://localhost:10200/dobetterweb/lighthouse-480x318.jpg?iar2 == | blob:http://localhost:10200/3f2bc9df-684b-4541-837c-1590152ef65d == | cpu ===== | http://localhost:10200/dobetterweb/empty.css = | filesystem:http://localhost:10200/temporary/empty-0.43448333410631723.png ================ | http://localhost:10200/dobetterweb/lighthouse-1024x680.jpg?redirected-lcp COMPRESSABLE: http://localhost:10200/dobetterweb/dbw_tester.html -- SAVE % 66.14231358964166 COMPRESSABLE: http://localhost:10200/dobetterweb/third_party/aggressive-promise-polyfill.js -- SAVE % 80.95526868808166 COMPRESSABLE: http://localhost:10200/dobetterweb/dbw_tester.html -- SAVE % 66.14231358964166

The most important difference I spotted was that lighthouse-rotating.gif is missing from the optimistic graph and yet it is the last node to finish. This means the byte savings on the second dbw_tester.html graph shouldn't impact the pessimistic simulation because the second dbw_tester.html is downloaded in parallel with lighthouse-rotating.gif. This just happens to be the way the requests downloaded, and therefore we can't rely on the optimistic graph providing a smaller savings estimate than the pessimistic graph for LCP.

That being said, both optimistic and pessimistic LCP graphs cast very wide nets when deciding which network requests to include and that might just make the results kinda noisy. It might be worth looking into a new optimistic LCP definition that focuses on the specific LCP request.

Took a look at only a handful of sites, seems like pessimistic savings being lower than optimistic savings only occasionally happens and solely for LCP.

We could:

use the min of pessimistic & optimistic

or we could just run with and rely on what optimistic gives us even if it might give us more savings

i'm open to both, though leaning on (1) to prioritize these audits less when sorting.

It might be worth looking into a new optimistic LCP definition that focuses on the specific LCP request.

I agree, worth taking note of this to dive into later

adam convinced me of going with 2. let's leave a comment in there about this weirdness.

adamraine · 2023-12-06T18:23:16Z

core/audits/byte-efficiency/byte-efficiency-audit.js

        results,
        pessimisticLCPGraph,
        simulator,
        {providedWastedBytesByUrl: result.wastedBytesByUrl, label: 'lcp'}
      );

+      // Use the min savings between the two graphs for LCP metricSavings.
+      const lcpMinGraphSavings = Math.min(lcpOptimisticGraphSavings, lcpPessimisticGraphSavings);


Thinking about this a little more. I think it was a mistake for our modus operandi to be "use the smallest estimate for savings possible". We should still use the optimistic graph because it is less noisy and focuses on the requests that definitely impacted LCP (at least in principle), but I don't think we need to add this arbitrary Math.min constraint just to get the lowest estimate. If a handful of edge cases have a higher savings estimate as a result that's fine with me but...

As I said in my previous comment, I think we should explore an LCP optimistic graph that is modeled on the LCP critical path which should further reduce the noise in our LCP savings estimate here. As an initial thought, the optimistic graph could include just the LCP resource and any render blocking requests.

core/audits/byte-efficiency/byte-efficiency-audit.js

adamraine · 2023-12-06T18:30:39Z

core/test/results/sample_v2.json

      "scoreDisplayMode": "metricSavings",
      "numericValue": 3450,
      "numericUnit": "millisecond",
      "displayValue": "Potential savings of 666 KiB",
      "metricSavings": {
        "FCP": 0,
-        "LCP": 3300
+        "LCP": 0


This is a good change. Byte savings on the large gif are good, but it isn't the LCP resource so it's LCP impact is superficial.

core/audits/byte-efficiency/byte-efficiency-audit.js

replace pessimistic graph with optimistic

3b3f4fb

adamraine reviewed Dec 1, 2023

View reviewed changes

Merge branch 'main' into optimistic-byte-efficiency

43011b0

vercel bot deployed to Preview December 5, 2023 23:38 View deployment

use min of pessimistic and optimistic

9c24cf9

vercel bot deployed to Preview December 6, 2023 00:53 View deployment

adrianaixba requested a review from adamraine December 6, 2023 00:53

adrianaixba marked this pull request as ready for review December 6, 2023 00:53

adrianaixba requested a review from a team as a code owner December 6, 2023 00:53

devtools-bot assigned adamraine Dec 6, 2023

devtools-bot added the waiting4reviewer label Dec 6, 2023

adamraine reviewed Dec 6, 2023

View reviewed changes

adrianaixba added 2 commits December 7, 2023 15:06

rely on the optimistic graph instead of taking the min

7787944

fix

3c710b0

vercel bot deployed to Preview December 7, 2023 23:08 View deployment

adamraine approved these changes Dec 7, 2023

View reviewed changes

core/audits/byte-efficiency/byte-efficiency-audit.js Outdated Show resolved Hide resolved

note

24b39a7

vercel bot deployed to Preview December 7, 2023 23:13 View deployment

nit

a9464c3

vercel bot deployed to Preview December 7, 2023 23:15 View deployment

Merge branch 'main' into optimistic-byte-efficiency

200d672

vercel bot deployed to Preview December 7, 2023 23:19 View deployment

adrianaixba merged commit c4d02c6 into main Dec 8, 2023
29 checks passed

adrianaixba deleted the optimistic-byte-efficiency branch December 8, 2023 23:38

adamraine mentioned this pull request Jan 9, 2024

Improve LCP lantern graphs #15737

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core(byte-efficiency): replace pessimistic graph with optimistic #15651

core(byte-efficiency): replace pessimistic graph with optimistic #15651

adrianaixba commented Nov 30, 2023 •

edited

Loading

adamraine Dec 1, 2023

adamraine Dec 5, 2023

adrianaixba Dec 6, 2023 •

edited

Loading

paulirish Dec 7, 2023

adamraine Dec 6, 2023

adamraine Dec 6, 2023

core(byte-efficiency): replace pessimistic graph with optimistic #15651

core(byte-efficiency): replace pessimistic graph with optimistic #15651

Conversation

adrianaixba commented Nov 30, 2023 • edited Loading

adamraine Dec 1, 2023

Choose a reason for hiding this comment

adamraine Dec 5, 2023

Choose a reason for hiding this comment

adrianaixba Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

paulirish Dec 7, 2023

Choose a reason for hiding this comment

adamraine Dec 6, 2023

Choose a reason for hiding this comment

adamraine Dec 6, 2023

Choose a reason for hiding this comment

adrianaixba commented Nov 30, 2023 •

edited

Loading

adrianaixba Dec 6, 2023 •

edited

Loading