-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
core(config): switch to throttling settings object #4879
Changes from 4 commits
917d48d
9fb8863
2aa8527
967edc2
d0ee1cb
bbf65b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,13 +16,13 @@ const emulation = require('../../emulation').settings; | |
const DEFAULT_MAXIMUM_CONCURRENT_REQUESTS = 10; | ||
|
||
// Fast 3G emulation target from DevTools, WPT 3G - Fast setting | ||
const DEFAULT_RTT = emulation.TYPICAL_MOBILE_THROTTLING_METRICS.targetLatency; | ||
const DEFAULT_THROUGHPUT = emulation.TYPICAL_MOBILE_THROTTLING_METRICS.targetDownloadThroughput * 8; // 1.6 Mbps | ||
const DEFAULT_RTT = emulation.MOBILE_3G_THROTTLING.targetLatencyMs; | ||
const DEFAULT_THROUGHPUT = emulation.MOBILE_3G_THROTTLING.targetDownloadThroughputKbps * 1024; // 1.6 Mbps | ||
|
||
// same multiplier as Lighthouse uses for CPU emulation | ||
const DEFAULT_CPU_TASK_MULTIPLIER = emulation.CPU_THROTTLE_METRICS.rate; | ||
// layout tasks tend to be less CPU-bound and do not experience the same increase in duration | ||
const DEFAULT_LAYOUT_TASK_MULTIPLIER = DEFAULT_CPU_TASK_MULTIPLIER / 2; | ||
const DEFAULT_LAYOUT_TASK_MULTIPLIER = .5; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. didn't we agree earlier we prefer a leading 0? VERY BIG DEAL. not a nit whatsoever. 馃槧 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. haha sure done |
||
// if a task takes more than 10 seconds it's usually a sign it isn't actually CPU bound and we're overestimating | ||
const DEFAULT_MAXIMUM_CPU_TASK_DURATION = 10000; | ||
|
||
|
@@ -45,7 +45,7 @@ class Simulator { | |
rtt: DEFAULT_RTT, | ||
throughput: DEFAULT_THROUGHPUT, | ||
maximumConcurrentRequests: DEFAULT_MAXIMUM_CONCURRENT_REQUESTS, | ||
cpuTaskMultiplier: DEFAULT_CPU_TASK_MULTIPLIER, | ||
cpuSlowdownMultiplier: DEFAULT_CPU_TASK_MULTIPLIER, | ||
layoutTaskMultiplier: DEFAULT_LAYOUT_TASK_MULTIPLIER, | ||
}, | ||
options | ||
|
@@ -57,8 +57,8 @@ class Simulator { | |
TcpConnection.maximumSaturatedConnections(this._rtt, this._throughput), | ||
this._options.maximumConcurrentRequests | ||
); | ||
this._cpuTaskMultiplier = this._options.cpuTaskMultiplier; | ||
this._layoutTaskMultiplier = this._options.layoutTaskMultiplier; | ||
this._cpuSlowdownMultiplier = this._options.cpuSlowdownMultiplier; | ||
this._layoutTaskMultiplier = this._cpuSlowdownMultiplier * this._options.layoutTaskMultiplier; | ||
|
||
this._nodeTiming = new Map(); | ||
this._numberInProgressByType = new Map(); | ||
|
@@ -206,7 +206,7 @@ class Simulator { | |
const timingData = this._nodeTiming.get(node); | ||
const multiplier = (/** @type {CpuNode} */ (node)).didPerformLayout() | ||
? this._layoutTaskMultiplier | ||
: this._cpuTaskMultiplier; | ||
: this._cpuSlowdownMultiplier; | ||
const totalDuration = Math.min( | ||
Math.round((/** @type {CpuNode} */ (node)).event.dur / 1000 * multiplier), | ||
DEFAULT_MAXIMUM_CPU_TASK_DURATION | ||
|
@@ -360,7 +360,7 @@ module.exports = Simulator; | |
* @property {number} [throughput] | ||
* @property {number} [fallbackTTFB] | ||
* @property {number} [maximumConcurrentRequests] | ||
* @property {number} [cpuTaskMultiplier] | ||
* @property {number} [cpuSlowdownMultiplier] | ||
* @property {number} [layoutTaskMultiplier] | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,12 @@ | |
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. | ||
*/ | ||
// @ts-nocheck | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 馃帀 |
||
'use strict'; | ||
|
||
const Driver = require('../gather/driver'); // eslint-disable-line no-unused-vars | ||
|
||
const NBSP = '\xa0'; | ||
|
||
/** | ||
* Nexus 5X metrics adapted from emulated_devices/module.json | ||
*/ | ||
|
@@ -40,17 +43,16 @@ const LATENCY_FACTOR = 3.75; | |
const THROUGHPUT_FACTOR = 0.9; | ||
|
||
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); // 750Kbps | ||
|
||
const TYPICAL_MOBILE_THROTTLING_METRICS = { | ||
targetLatency: TARGET_LATENCY, | ||
latency: TARGET_LATENCY * LATENCY_FACTOR, | ||
targetDownloadThroughput: TARGET_DOWNLOAD_THROUGHPUT, | ||
downloadThroughput: TARGET_DOWNLOAD_THROUGHPUT * THROUGHPUT_FACTOR, | ||
targetUploadThroughput: TARGET_UPLOAD_THROUGHPUT, | ||
uploadThroughput: TARGET_UPLOAD_THROUGHPUT * THROUGHPUT_FACTOR, | ||
offline: false, | ||
const TARGET_DOWNLOAD_THROUGHPUT = Math.floor(1.6 * 1024); // 1.6Mbps | ||
const TARGET_UPLOAD_THROUGHPUT = Math.floor(750); // 750Kbps | ||
|
||
const MOBILE_3G_THROTTLING = { | ||
targetLatencyMs: TARGET_LATENCY, | ||
adjustedLatencyMs: TARGET_LATENCY * LATENCY_FACTOR, | ||
targetDownloadThroughputKbps: TARGET_DOWNLOAD_THROUGHPUT, | ||
adjustedDownloadThroughputKbps: TARGET_DOWNLOAD_THROUGHPUT * THROUGHPUT_FACTOR, | ||
targetUploadThroughputKbps: TARGET_UPLOAD_THROUGHPUT, | ||
adjustedUploadThroughputKbps: TARGET_UPLOAD_THROUGHPUT * THROUGHPUT_FACTOR, | ||
}; | ||
|
||
const OFFLINE_METRICS = { | ||
|
@@ -75,34 +77,10 @@ const CPU_THROTTLE_METRICS = { | |
rate: 4, | ||
}; | ||
|
||
/** | ||
* @param {Driver} driver | ||
*/ | ||
function enableNexus5X(driver) { | ||
// COMPAT FIMXE | ||
// Injecting this function clientside is no longer neccessary as of m62. This is done | ||
// on the backend when `Emulation.setTouchEmulationEnabled` is set. | ||
// https://bugs.chromium.org/p/chromium/issues/detail?id=133915#c63 | ||
// Once m62 hits stable (~Oct 20th) we can nuke this entirely | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :) |
||
/** | ||
* Finalizes touch emulation by enabling `"ontouchstart" in window` feature detect | ||
* to work. Messy hack, though copied verbatim from DevTools' emulation/TouchModel.js | ||
* where it's been working for years. addScriptToEvaluateOnLoad runs before any of the | ||
* page's JavaScript executes. | ||
*/ | ||
/* eslint-disable no-proto */ /* global window, document */ /* istanbul ignore next */ | ||
const injectedTouchEventsFunction = function() { | ||
const touchEvents = ['ontouchstart', 'ontouchend', 'ontouchmove', 'ontouchcancel']; | ||
const recepients = [window.__proto__, document.__proto__]; | ||
for (let i = 0; i < touchEvents.length; ++i) { | ||
for (let j = 0; j < recepients.length; ++j) { | ||
if (!(touchEvents[i] in recepients[j])) { | ||
Object.defineProperty(recepients[j], touchEvents[i], { | ||
value: null, writable: true, configurable: true, enumerable: true, | ||
}); | ||
} | ||
} | ||
} | ||
}; | ||
/* eslint-enable */ | ||
|
||
return Promise.all([ | ||
driver.sendCommand('Emulation.setDeviceMetricsOverride', NEXUS5X_EMULATION_METRICS), | ||
// Network.enable must be called for UA overriding to work | ||
|
@@ -112,55 +90,135 @@ function enableNexus5X(driver) { | |
enabled: true, | ||
configuration: 'mobile', | ||
}), | ||
driver.sendCommand('Page.addScriptToEvaluateOnLoad', { | ||
scriptSource: '(' + injectedTouchEventsFunction.toString() + ')()', | ||
}), | ||
]); | ||
} | ||
|
||
function enableNetworkThrottling(driver) { | ||
return driver.sendCommand('Network.emulateNetworkConditions', TYPICAL_MOBILE_THROTTLING_METRICS); | ||
/** | ||
* @param {Driver} driver | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for type checking in this file we'll have to OTOH there's exposing the major types on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah sg, I've used that before 馃憤 |
||
* @param {LH.ThrottlingSettings|undefined} throttlingSettings | ||
* @return {Promise<void>} | ||
*/ | ||
function enableNetworkThrottling(driver, throttlingSettings) { | ||
/** @type {LH.Crdp.Network.EmulateNetworkConditionsRequest} */ | ||
let conditions; | ||
if (throttlingSettings) { | ||
conditions = { | ||
offline: false, | ||
latency: throttlingSettings.requestLatencyMs || 0, | ||
downloadThroughput: throttlingSettings.downloadThroughputKbps || 0, | ||
uploadThroughput: throttlingSettings.uploadThroughputKbps || 0, | ||
}; | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what case is this handling? if throttlingSettings are provided via config now, what is this fallback case for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for configs that don't expressly set throttling or extend the default, also tests :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this could be one of the things we (eventually) pull into config.js so once it comes out |
||
conditions = { | ||
offline: false, | ||
latency: MOBILE_3G_THROTTLING.adjustedLatencyMs, | ||
downloadThroughput: MOBILE_3G_THROTTLING.adjustedDownloadThroughputKbps, | ||
uploadThroughput: MOBILE_3G_THROTTLING.adjustedUploadThroughputKbps, | ||
}; | ||
} | ||
|
||
// DevTools expects throughput in bytes per second rather than kbps | ||
conditions.downloadThroughput = conditions.downloadThroughput * 1024 / 8; | ||
conditions.uploadThroughput = conditions.uploadThroughput * 1024 / 8; | ||
return driver.sendCommand('Network.emulateNetworkConditions', conditions); | ||
} | ||
|
||
function disableNetworkThrottling(driver) { | ||
/** | ||
* @param {Driver} driver | ||
* @return {Promise<void>} | ||
*/ | ||
function clearAllNetworkEmulation(driver) { | ||
return driver.sendCommand('Network.emulateNetworkConditions', NO_THROTTLING_METRICS); | ||
} | ||
|
||
/** | ||
* @param {Driver} driver | ||
* @return {Promise<void>} | ||
*/ | ||
function goOffline(driver) { | ||
return driver.sendCommand('Network.emulateNetworkConditions', OFFLINE_METRICS); | ||
} | ||
|
||
function enableCPUThrottling(driver) { | ||
return driver.sendCommand('Emulation.setCPUThrottlingRate', CPU_THROTTLE_METRICS); | ||
/** | ||
* @param {Driver} driver | ||
* @param {LH.ThrottlingSettings|undefined} throttlingSettings | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @brendankenny for some reason type checking was refusing to allow undefined as an input to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like this is a temporary bug in the version of TS we're using. I was actually pleased that it started completely differentiating between optional and Later releases have that fixed, though. I'm about to make a PR bumping us to |
||
* @return {Promise<void>} | ||
*/ | ||
function enableCPUThrottling(driver, throttlingSettings) { | ||
const rate = throttlingSettings | ||
? throttlingSettings.cpuSlowdownMultiplier | ||
: CPU_THROTTLE_METRICS.rate; | ||
return driver.sendCommand('Emulation.setCPUThrottlingRate', {rate}); | ||
} | ||
|
||
/** | ||
* @param {Driver} driver | ||
* @return {Promise<void>} | ||
*/ | ||
function disableCPUThrottling(driver) { | ||
return driver.sendCommand('Emulation.setCPUThrottlingRate', NO_CPU_THROTTLE_METRICS); | ||
} | ||
|
||
function getEmulationDesc() { | ||
const {latency, downloadThroughput, uploadThroughput} = TYPICAL_MOBILE_THROTTLING_METRICS; | ||
const byteToMbit = bytes => (bytes / 1024 / 1024 * 8).toFixed(1); | ||
/** | ||
* @param {LH.ConfigSettings} settings | ||
* @return {{deviceEmulation: string, cpuThrottling: string, networkThrottling: string}} | ||
*/ | ||
function getEmulationDesc(settings) { | ||
let cpuThrottling; | ||
let networkThrottling; | ||
|
||
/** @type {function(number|undefined):string} */ | ||
const byteToKbit = (bytes = 0) => (bytes / 1024 * 8).toFixed(); | ||
/** @type {LH.ThrottlingSettings} */ | ||
const throttling = settings.throttling || {}; | ||
|
||
switch (settings.throttlingMethod) { | ||
case 'provided': | ||
cpuThrottling = 'Provided by environment'; | ||
networkThrottling = 'Provided by environment'; | ||
break; | ||
case 'devtools': { | ||
const {cpuSlowdownMultiplier, requestLatencyMs} = throttling; | ||
const down = byteToKbit(throttling.downloadThroughputKbps); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wait, shouldn't these already be Kbps? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whoops yes good catch forgot to update :) |
||
const up = byteToKbit(throttling.uploadThroughputKbps); | ||
|
||
cpuThrottling = `${cpuSlowdownMultiplier}x slowdown (DevTools)`; | ||
networkThrottling = `${requestLatencyMs}${NBSP}ms HTTP RTT, ` + | ||
`${down}${NBSP}Kbps down, ${up}${NBSP}Kbps up (DevTools)`; | ||
break; | ||
} | ||
case 'simulate': { | ||
const {cpuSlowdownMultiplier, rttMs} = throttling; | ||
const throughput = byteToKbit(throttling.throughputKbps); | ||
cpuThrottling = `${cpuSlowdownMultiplier}x slowdown (Simulated)`; | ||
networkThrottling = `${rttMs}${NBSP}ms TCP RTT, ` + | ||
`${throughput}${NBSP}Kbps throughput (Simulated)`; | ||
break; | ||
} | ||
default: | ||
cpuThrottling = 'Unknown'; | ||
networkThrottling = 'Unknown'; | ||
} | ||
|
||
return { | ||
'deviceEmulation': 'Nexus 5X', | ||
'cpuThrottling': `${CPU_THROTTLE_METRICS.rate}x slowdown`, | ||
'networkThrottling': `${latency}ms RTT, ${byteToMbit(downloadThroughput)}Mbps down, ` + | ||
`${byteToMbit(uploadThroughput)}Mbps up`, | ||
deviceEmulation: settings.disableDeviceEmulation ? 'Disabled' : 'Nexus 5X', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 馃憤 way better than the old enabled/disabled and a decription thing |
||
cpuThrottling, | ||
networkThrottling, | ||
}; | ||
} | ||
|
||
module.exports = { | ||
enableNexus5X, | ||
enableNetworkThrottling, | ||
disableNetworkThrottling, | ||
clearAllNetworkEmulation, | ||
enableCPUThrottling, | ||
disableCPUThrottling, | ||
goOffline, | ||
getEmulationDesc, | ||
settings: { | ||
NEXUS5X_EMULATION_METRICS, | ||
NEXUS5X_USERAGENT, | ||
TYPICAL_MOBILE_THROTTLING_METRICS, | ||
MOBILE_3G_THROTTLING, | ||
OFFLINE_METRICS, | ||
NO_THROTTLING_METRICS, | ||
NO_CPU_THROTTLE_METRICS, | ||
|
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.
we use
setThrottling
forgoOnline
andgoOffline
, so does this break that for nondevtools
throttling? We may need to separate those from this anyways aspassConfig.useThrottling
probably shouldn't affect a call todriver.goOffline()
anyways.We can kick that to another PR if it gets involved, but is there another quick fix in the meantime?
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.
Ah yeah good catch, we can go offline but never go back online atm if devtools throttling is off or useThrottling is false :), I'll clean this up it's a pretty easy fix
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.
done and tests added!