Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from
Dec 8, 2023
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3b3f4fb
replace pessimistic graph with optimistic
adrianaixba Nov 30, 2023
43011b0
Merge branch 'main' into optimistic-byte-efficiency
adrianaixba Dec 5, 2023
9c24cf9
use min of pessimistic and optimistic
adrianaixba Dec 6, 2023
7787944
rely on the optimistic graph instead of taking the min
adrianaixba Dec 7, 2023
3c710b0
fix
adrianaixba Dec 7, 2023
24b39a7
note
adrianaixba Dec 7, 2023
a9464c3
nit
adrianaixba Dec 7, 2023
200d672
Merge branch 'main' into optimistic-byte-efficiency
adrianaixba Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/audits/byte-efficiency/byte-efficiency-audit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,10 @@ class ByteEfficiencyAudit extends Audit {
if (metricComputationInput.gatherContext.gatherMode === 'navigation') {
const graph = await PageDependencyGraph.request(metricComputationInput, context);
const {
pessimisticGraph: pessimisticFCPGraph,
optimisticGraph: optimisticFCPGraph,
} = await LanternFirstContentfulPaint.request(metricComputationInput, context);
const {
pessimisticGraph: pessimisticLCPGraph,
optimisticGraph: optimisticLCPGraph,
} = await LanternLargestContentfulPaint.request(metricComputationInput, context);

wastedMs = this.computeWasteWithTTIGraph(results, graph, simulator, {
Expand All @@ -260,13 +260,13 @@ class ByteEfficiencyAudit extends Audit {

const {savings: fcpSavings} = this.computeWasteWithGraph(
results,
pessimisticFCPGraph,
optimisticFCPGraph,
simulator,
{providedWastedBytesByUrl: result.wastedBytesByUrl, label: 'fcp'}
);
const {savings: lcpGraphSavings} = this.computeWasteWithGraph(
results,
pessimisticLCPGraph,
optimisticLCPGraph,
simulator,
{providedWastedBytesByUrl: result.wastedBytesByUrl, label: 'lcp'}
);
Expand Down
24 changes: 12 additions & 12 deletions core/test/results/sample_v2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4457,7 +4457,7 @@
"warnings": [],
"metricSavings": {
"FCP": 0,
"LCP": 300
"LCP": 610
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@adrianaixba adrianaixba Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. use the min of pessimistic & optimistic
  2. 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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

},
"details": {
"type": "opportunity",
Expand Down Expand Up @@ -4495,7 +4495,7 @@
"type": "debugdata",
"metricSavings": {
"FCP": 0,
"LCP": 300
"LCP": 610
}
}
},
Expand Down Expand Up @@ -4544,7 +4544,7 @@
"displayValue": "Potential savings of 64 KiB",
"metricSavings": {
"FCP": 0,
"LCP": 450
"LCP": 300
},
"details": {
"type": "opportunity",
Expand Down Expand Up @@ -4598,7 +4598,7 @@
"type": "debugdata",
"metricSavings": {
"FCP": 0,
"LCP": 450
"LCP": 300
}
}
},
Expand Down Expand Up @@ -4767,7 +4767,7 @@
"displayValue": "Potential savings of 143 KiB",
"metricSavings": {
"FCP": 150,
"LCP": 750
"LCP": 1060
},
"details": {
"type": "opportunity",
Expand Down Expand Up @@ -4809,7 +4809,7 @@
"type": "debugdata",
"metricSavings": {
"FCP": 150,
"LCP": 750
"LCP": 1060
}
}
},
Expand Down Expand Up @@ -4851,14 +4851,14 @@
"id": "efficient-animated-content",
"title": "Use video formats for animated content",
"description": "Large GIFs are inefficient for delivering animated content. Consider using MPEG4/WebM videos for animations and PNG/WebP for static images instead of GIF to save network bytes. [Learn more about efficient video formats](https://developer.chrome.com/docs/lighthouse/performance/efficient-animated-content/)",
"score": 0,
"score": 0.5,
"scoreDisplayMode": "metricSavings",
"numericValue": 3450,
"numericUnit": "millisecond",
"displayValue": "Potential savings of 666 KiB",
"metricSavings": {
"FCP": 0,
"LCP": 3300
"LCP": 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

},
"details": {
"type": "opportunity",
Expand Down Expand Up @@ -4895,7 +4895,7 @@
"type": "debugdata",
"metricSavings": {
"FCP": 0,
"LCP": 3300
"LCP": 0
}
}
},
Expand Down Expand Up @@ -4937,14 +4937,14 @@
"id": "legacy-javascript",
"title": "Avoid serving legacy JavaScript to modern browsers",
"description": "Polyfills and transforms enable legacy browsers to use new JavaScript features. However, many aren't necessary for modern browsers. For your bundled JavaScript, adopt a modern script deployment strategy using module/nomodule feature detection to reduce the amount of code shipped to modern browsers, while retaining support for legacy browsers. [Learn how to use modern JavaScript](https://web.dev/articles/publish-modern-javascript)",
"score": 0.5,
"score": 0,
"scoreDisplayMode": "metricSavings",
"numericValue": 450,
"numericUnit": "millisecond",
"displayValue": "Potential savings of 26 KiB",
"metricSavings": {
"FCP": 0,
"LCP": 0
"LCP": 150
},
"details": {
"type": "opportunity",
Expand Down Expand Up @@ -5013,7 +5013,7 @@
"type": "debugdata",
"metricSavings": {
"FCP": 0,
"LCP": 0
"LCP": 150
}
}
},
Expand Down
Loading