-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Update network throttling to simulate more realistic network conditions #2238
Update network throttling to simulate more realistic network conditions #2238
Conversation
lighthouse-core/lib/emulation.js
Outdated
|
||
const TARGET_LATENCY = 150; // 150ms | ||
const TARGET_DOWNLOAD_THROUGHPUT = Math.floor(1.6 * 1024 * 1024 / 8); // 1.6Mbps | ||
const TARGET_UPLOAD_THROUGHPUT = Math.floor(750 * 1024 / 8 * THROUGHPUT_FACTOR); // 750Kbps |
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.
why is the throughput factor included in the target here?
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.
whoops - fixed.
lighthouse-core/lib/emulation.js
Outdated
@@ -41,10 +41,24 @@ const NEXUS5X_USERAGENT = { | |||
'(KHTML, like Gecko) Chrome/59.0.3033.0 Mobile Safari/537.36' | |||
}; | |||
|
|||
/** | |||
* Adjustments needed for DevTools network throttling to simulate | |||
* more realistic network throttling conditions |
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.
should we link off to any explanations or references?
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.
Added a link to the crbug (not sure whether to make the analysis doc public yet)
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.
Thanks. Updated.
lighthouse-core/lib/emulation.js
Outdated
|
||
const TARGET_LATENCY = 150; // 150ms | ||
const TARGET_DOWNLOAD_THROUGHPUT = Math.floor(1.6 * 1024 * 1024 / 8); // 1.6Mbps | ||
const TARGET_UPLOAD_THROUGHPUT = Math.floor(750 * 1024 / 8 * THROUGHPUT_FACTOR); // 750Kbps |
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.
whoops - fixed.
lighthouse-core/lib/emulation.js
Outdated
@@ -41,10 +41,24 @@ const NEXUS5X_USERAGENT = { | |||
'(KHTML, like Gecko) Chrome/59.0.3033.0 Mobile Safari/537.36' | |||
}; | |||
|
|||
/** | |||
* Adjustments needed for DevTools network throttling to simulate | |||
* more realistic network throttling conditions |
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.
Added a link to the crbug (not sure whether to make the analysis doc public yet)
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.
looks good! get ready for some really slow runs :)
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.
targetDownloadThroughput etc arent used right now but otherwise. sgtm.
lets land now so we can see how sloooooooow this is!
Based on the work in #2183 which I'll land as a follow-up.