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.

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

Lantern overestimates image savings #14166

Open
connorjclark opened this issue Jun 28, 2022 · 6 comments
Open

Lantern overestimates image savings #14166

connorjclark opened this issue Jun 28, 2022 · 6 comments

Comments

@connorjclark
Copy link
Collaborator

connorjclark commented Jun 28, 2022

From @philipwalton:

Untitled

A command to quickly generate the faulty traces:

LANTERN_DEBUG=1 lighthouse "https://next.webpagetest.org/themetrictimes/index.php" --only-audits="modern-image-formats" --save-assets --throttlingMethod="devtools"

Either an issue with the graph generation, or the simulator. I think the former.

@paulirish
Copy link
Member

Actual trace (devtools throttling):

image

modern-image-formats-before.trace.json (which should "match" the actual trace):

(but doesn't)

image

modern-image-formats-after.trace.json (which should be the same except some of the green image blocks are shorter)

(which seems true)

image

@brendankenny
Copy link
Member

brendankenny commented Jun 28, 2022

modern-image-formats-before.trace.json (which should "match" the actual trace):
(but doesn't)

I think the Lantern debug trace generator is broken (check those duration details :)

Information about a network request, showing 'Duration6.31 s (4.7 hrs network transfer + -17010144827 μs resource loading)'

The output from printGraph seems more reasonable and appears to resemble the "real" waterfall pretty well.

Lantern dependency graph
==                                                 | https://next.webpagetest.org/themetrictimes/index.php
 =                                                 | cpu
 ==                                                | https://fonts.googleapis.com/css2?family=Abril+Fatface&family=Roboto&family=Merriweather:wght@300;70
 ==                                                | https://next.webpagetest.org/themetrictimes/site.css
 ==                                                | https://next.webpagetest.org/themetrictimes/site.js
 ==                                                | https://next.webpagetest.org/themetrictimes/site2.js
 ==                                                | https://code.jquery.com/jquery-2.0.0.js
 ==                                                | https://code.jquery.com/ui/1.13.0-rc.3/themes/smoothness/jquery-ui.css
 ==                                                | https://next.webpagetest.org/themetrictimes/foo/bar/baz.css
 =                                                 | cpu
   ==                                              | https://unpkg.com/react/umd/react.development.js
   =                                               | cpu
   ==                                              | https://unpkg.com/react-dom/umd/react-dom.development.js
   =                                               | cpu
   =                                               | http://example.com/this-will-fail.js
   ==                                              | https://next.webpagetest.org/themetrictimes/site3.js
   =======================                         | https://next.webpagetest.org/themetrictimes/delays.jpg
   ===================                             | https://next.webpagetest.org/themetrictimes/serviceworkers.jpg
   ==============================================  | https://next.webpagetest.org/themetrictimes/dog.jpg?again
   =======================                         | https://next.webpagetest.org/themetrictimes/delays.jpg?again
   ====================                            | https://next.webpagetest.org/themetrictimes/serviceworkers.jpg?again
    =                                              | cpu
    =                                              | https://next.webpagetest.org/embed.php?url=https%3A%2F%2Fnext.webpagetest.org%2Fthemetrictimes%2Find
    ====                                           | https://unpkg.com/react@18.2.0/umd/react.development.js
    ==================                             | https://unpkg.com/react-dom@18.2.0/umd/react-dom.development.js
     ============================================= | https://next.webpagetest.org/themetrictimes/dog.jpg
     =                                             | cpu
     =                                             | cpu
     =====================                         | https://next.webpagetest.org/pagestyle2.css?v=1e360b2b5eb25b1a86cf2598b53cff87
     ====================                          | https://next.webpagetest.org/wpt-header.css?v=7bd794c87570eaa8918316cce96a0235
     =========================                     | https://next.webpagetest.org/images/wpt_home_featureimg.jpg
       =                                           | cpu
                      =                            | cpu
                       =                           | cpu
                       =                           | cpu
                       =                           | cpu
                          =                        | cpu
                          ======                   | https://next.webpagetest.org/js/site.js?v=41
                          =                        | cpu
                          ==                       | https://g.3gl.net/jp/8627/v3.3.4/M
                          =                        | cpu
                          ==                       | https://next.webpagetest.org/js/test.js?v=47
                          ======                   | https://next.webpagetest.org/wpt-header.js?v=41
                          =======                  | https://next.webpagetest.org/images/TwitchGlitchPurple.svg
                          =                        | cpu
                          ===                      | https://next.webpagetest.org/images/wpt-logo.svg
                          =====                    | https://next.webpagetest.org/images/wpt-header-menubtn.svg
                          =                        | cpu
                          ====                     | https://next.webpagetest.org/fonts/opensans-regular-webfont.woff2
                          =                        | cpu
                          =                        | data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='100%25' height='100%25' viewBox=
                          =                        | data:image/svg+xml,%3csvg width='16' height='10' xmlns='http://www.w3.org/2000/svg'%3e%3cpath d='M1.
                          =                        | data:image/svg+xml,%3csvg width='16' height='10' xmlns='http://www.w3.org/2000/svg'%3e%3cpath d='M1.
                          ===                      | https://fonts.gstatic.com/s/merriweather/v30/u-4n0qyriQwlOrhSvowK_l52xwNZWMf6hPvhPQ.woff2
                          ===                      | https://fonts.gstatic.com/s/merriweather/v30/u-4n0qyriQwlOrhSvowK_l521wRZWMf6hPvhPQ.woff2
                          =====                    | https://next.webpagetest.org/fonts/opensans-bold-webfont.woff2
                          =====                    | https://next.webpagetest.org/fonts/opensans-light-webfont.woff2
                          =                        | cpu
                          =                        | cpu
                          =                        | cpu
                            =                      | cpu
                            =                      | cpu
                            =                      | cpu
                             =                     | cpu
                               =                   | cpu
                               ==                  | https://next.webpagetest.org/images/wpt-logo-pro-dark.svg
                               =                   | cpu
                               =                   | cpu
                               ==                  | https://webpagetest.org/images/twitter.svg
                               ==                  | https://webpagetest.org/images/youtube.svg
                               ==                  | https://webpagetest.org/images/linkedin.svg
                                ==                 | https://webpagetest.org/images/github.svg
                                =                  | cpu
                                ==                 | https://next.webpagetest.org/images/wpt-logo-dark.svg
                                 ==                | https://next.webpagetest.org/images/wpt-logo-pro.svg
                                 ==                | https://www.webpagetest.org/images/twitter.svg
                                 ==                | https://www.webpagetest.org/images/youtube.svg
                                 ==                | https://www.webpagetest.org/images/linkedin.svg
                                 ==                | https://www.webpagetest.org/images/github.svg
                                  ========         | https://next.webpagetest.org/images/pro-intro.jpg
                                  ==               | https://next.webpagetest.org/images/test_icons/chrome.svg
                                  ==               | https://next.webpagetest.org/images/test_icons/flags/US.svg
                                  ==               | https://next.webpagetest.org/images/test_icons/flags/IN.svg
                                  ===              | https://next.webpagetest.org/images/test_icons/edge.svg
                                  ==               | https://next.webpagetest.org/images/test_icons/flags/CA.svg
                                   ===             | https://next.webpagetest.org/images/test_icons/firefox.svg
                                   ===             | https://next.webpagetest.org/images/test_icons/flags/DE.svg
                                   ==              | https://next.webpagetest.org/images/real-experience-icon.svg
                                    ===            | https://next.webpagetest.org/images/video-capture-icon.svg
                                    ===            | https://next.webpagetest.org/images/indepth-metrics-icon.svg
                                     ==            | https://next.webpagetest.org/images/tiles/blog-throttling.png
                                     ===           | https://next.webpagetest.org/images/tiles/CSS.png
                                     ===           | https://next.webpagetest.org/images/tiles/blog-renderblock.png
                                      ==           | https://next.webpagetest.org/images/github.svg
                                      ===          | https://next.webpagetest.org/images/twitter.svg
                                       ===         | https://next.webpagetest.org/images/youtube.svg
                                       ==          | https://next.webpagetest.org/images/linkedin.svg
                                        ==         | https://next.webpagetest.org/images/wpt_home_swoop.svg
                                        ===        | https://next.webpagetest.org/images/src/icon_play.svg
                                         ==        | https://next.webpagetest.org/images/test_icons/comment.svg
                                          =        | cpu
                                          ==       | https://r.3gl.net/hawklogserver/r.p
                                                == | https://next.webpagetest.org/images/favicon-32x32.png

@connorjclark
Copy link
Collaborator Author

connorjclark commented Jun 28, 2022

(comment was before reading brendan's post)

Noticed that I'd get 3-4 concurrent requests max in simulator even though 10 was the max concurrent connections. It ignores all the extra connections because of how it simulates the connection pool.

Setting flexibleOrdering gives a better graph (for modern-image-formats). Need time to figure out how connections work / what is intended by this option.

image

@brendankenny
Copy link
Member

A big thing with the image audits (and almost all the ByteEfficiency audits) is that they're savings against end-of-trace, not LCP (or even TTI).

It's less obvious in https://next.webpagetest.org/themetrictimes/index.php—in my report the image opportunities are still less than LCP—but in Phil's example report where they exceed LCP it's possible a slow loading image kept waitForLoad going well past LCP and the savings are still accurate for the end of the trace.

@connorjclark
Copy link
Collaborator Author

connorjclark commented Jun 28, 2022

I think the Lantern debug trace generator is broken (check those duration details :)

The output from printGraph seems more reasonable and appears to resemble the "real" waterfall pretty well.

I read this comment as you saying that the bug is just in the debugging trace generation, not an indicator of simulator doing something wrong. (maybe you didn't mean this?)

However, the debugging trace generation is made directly from the node timings in the simulator (while printGraph doesn't use simulator at all), and I can confirm the simulator is only doing a handful of requests at a time for this particular image audit. This does look like an actual error in the simulator.

A big thing with the image audits (and almost all the ByteEfficiency audits) is that they're savings against end-of-trace, not LCP (or even TTI).

Yeah there's that, I omitted those details. Just focusing on the wildly unexpected trace / number we're coming up with. No sensible reason these images would be requested in series.

@brendankenny
Copy link
Member

brendankenny commented Jul 7, 2022

Yeah, I was making a big assumption that trace emit was more broken than it was. I see the same serial image requests you did (and the trace shows), which does seem broken.

  • is this on every site and every byte efficiency (and presumably more) simulation?
  • did something change in Chrome?

Setting flexibleOrdering gives a better graph (for modern-image-formats). Need time to figure out how connections work / what is intended by this option

it allows ignoring connectionReused from the actual requests. That makes sense for the altered graph in some cases, but if ignoring it is required to approximate the original load graph, something is pretty wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants