-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
report: reframe Fast 3G as Slow 4G #6163
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Like the changes, and moving to more generic phrasing like 'mobile' and 'cellular' seem like a good hedge against doing this again in so many places.
docs/throttling.md
Outdated
@@ -1,13 +1,13 @@ | |||
|
|||
## The 3G network throttling preset | |||
## The network throttling preset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: The mobile network throttling preset
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup I think this does it.
PWA Checklist will need an update. And LH docs. And Audits Panel UI. anything else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no opinion on the policy changes, but source changes LGTM :)
/** Explanation shown to users below performance results to inform them that the test was done with a 3G network connection and to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. 'Lighthouse' becomes link text to additional documentation. */ | ||
lsPerformanceCategoryDescription: '[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.', | ||
/** Explanation shown to users below performance results to inform them that the test was done with a 4G network connection and to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. 'Lighthouse' becomes link text to additional documentation. */ | ||
lsPerformanceCategoryDescription: '[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 4G. Values are estimated and may vary.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this ends up prominent in the report, so is it worth adding the extra "slow" in front of "4G" for this string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the approach that if it wasn't worth qualifying fast
before than why qualify slow
now, but I see both sides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, makes sense. I was thinking in terms of easing in users who will still be thinking in terms of "3G is what I should be testing", rather than specific speeds/network characteristics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that's a good point, I like the more general emulated mobile network
here too actually :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💥 nice
also, maybe add a link to previous discussion to the PR description for future digging into this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the source changes. Shifting the labelling from "3G Fast" to "4G Slow" across our other speed tools is something we should consider for consistency if making this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Framing on this change:
- The goal of a good mobile throttling preset is never to accurately reflect the conditions of a particular cellular telecommunications system. It's to reflect the conditions that real users experience.
- The preset here attempts to capture latency/throughput of ~80 percentile observed connection data from the wild. As people globally acquire faster connections, this preset should be updated to keep pace.
- Right now, we have no changes to make in the latency/throughput numbers.
- However given the recent adoption of 4G, this preset actually now captures a 4G experience moreso than a 3G experience.
docs/throttling.md
Outdated
|
||
- Latency: 150ms | ||
- Throughput: 1.6Mbps down / 750 Kbps up. | ||
- Packet loss: none. | ||
|
||
These exact figures are used as the [WebPageTest "Mobile 3G - Fast" preset](https://github.com/WPO-Foundation/webpagetest/blob/master/www/settings/connectivity.ini.sample) and [Lighthouse's throttling default](https://github.com/GoogleChrome/lighthouse/blob/8f500e00243e07ef0a80b39334bedcc8ddc8d3d0/lighthouse-core/config/constants.js#L19-L26). These values represent roughly the bottom 25% of 4G connections and top 25% of 3G connections. | ||
These exact figures are used as [Lighthouse's throttling default](https://github.com/GoogleChrome/lighthouse/blob/8f500e00243e07ef0a80b39334bedcc8ddc8d3d0/lighthouse-core/config/constants.js#L19-L26) and represent roughly the bottom 25% of 4G connections and top 25% of 3G connections. They are identical to the [WebPageTest "Mobile 3G - Fast" preset](https://github.com/WPO-Foundation/webpagetest/blob/master/www/settings/connectivity.ini.sample) and slightly faster for many pages than [WebPageTest "4G" preset](https://github.com/WPO-Foundation/webpagetest/blob/master/www/settings/connectivity.ini.sample). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and slightly faster for many pages
and, due to a lower latency, slightly faster for some pages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright alright I begrudgingly admit that's better :)
LGTM. Makes sense, and +1 to Addy - would love to see this reflected broadly in our tooling. |
For terminology alignment with CruX. Bug: GoogleChrome/lighthouse#6163 Change-Id: I90ebd73384947114f9b237f4afee3fdd4683db15 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1575823 Reviewed-by: Erik Luo <luoe@chromium.org> Reviewed-by: Paul Irish <paulirish@chromium.org> Commit-Queue: Paul Irish <paulirish@chromium.org> Commit-Queue: Connor Clark <cjamcl@google.com> Cr-Original-Commit-Position: refs/heads/master@{#652602} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 33c7ee83367a4b488d8024ac040d6f0a942db6ab
For terminology alignment with CruX. Bug: GoogleChrome/lighthouse#6163 Change-Id: I90ebd73384947114f9b237f4afee3fdd4683db15 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1575823 Reviewed-by: Erik Luo <luoe@chromium.org> Reviewed-by: Paul Irish <paulirish@chromium.org> Commit-Queue: Paul Irish <paulirish@chromium.org> Commit-Queue: Connor Clark <cjamcl@google.com> Cr-Commit-Position: refs/heads/master@{#652602}
Good summary copied from @paulirish below:
We don't actually reference 3G in that many places, I think this effort will be mostly about docs and how we talk to folks.
The biggest change I'm not 100% sure on how to navigate is the PWA audit and our docs linking to WPT - Fast 3G preset.
re: PWA check
I took the approach of just saying "mobile networks" and "cellular networks" since that is in fact the motivation of the check. The checklist does specifically call out "first visit on a simulated 3G network." though, so not sure if we should update it there or what :)