Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@patrickhulce
patrickhulce committed Sep 1, 2017
1 parent 1342bb1 commit a632442
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 67 deletions.
2 changes: 1 addition & 1 deletion lighthouse-core/audits/predictive-perf.js
View file
Expand Up @@ -102,7 +102,7 @@ class PredictivePerf extends Audit {
sum += values[key];
});

const rawValue = sum / 4;
const rawValue = sum / Object.keys(values).length;
const score = Audit.computeLogNormalScore(
rawValue,
SCORING_POINT_OF_DIMINISHING_RETURNS,
Expand Down
83 changes: 45 additions & 38 deletions lighthouse-core/gather/computed/dependency-graph/estimator/estimator.js
View file
Expand Up @@ -7,12 +7,15 @@

const Node = require('../node');
const TcpConnection = require('./tcp-connection');
const emulation = require('../../../../lib/emulation').settings;

// see https://cs.chromium.org/search/?q=kDefaultMaxNumDelayableRequestsPerClient&sq=package:chromium&type=cs
const DEFAULT_MAXIMUM_CONCURRENT_REQUESTS = 10;
const DEFAULT_RESPONSE_TIME = 30;
const DEFAULT_RTT = 150;
const DEFAULT_THROUGHPUT = 1600 * 1024; // 1.6 Mbps
const DEFAULT_FALLBACK_TTFB = 30;
const DEFAULT_RTT = emulation.TYPICAL_MOBILE_THROTTLING_METRICS.targetLatency;
const DEFAULT_THROUGHPUT = emulation.TYPICAL_MOBILE_THROTTLING_METRICS.targetDownloadThroughput * 8;

const TLS_SCHEMES = ['https', 'wss'];

function groupBy(items, keyFunc) {
const grouped = new Map();
Expand All @@ -29,7 +32,7 @@ function groupBy(items, keyFunc) {
class Estimator {
/**
* @param {!Node} graph
* @param {{rtt: number, throughput: number, defaultResponseTime: number,
* @param {{rtt: number, throughput: number, fallbackTTFB: number,
* maximumConcurrentRequests: number}=} options
*/
constructor(graph, options) {
Expand All @@ -38,15 +41,15 @@ class Estimator {
{
rtt: DEFAULT_RTT,
throughput: DEFAULT_THROUGHPUT,
defaultResponseTime: DEFAULT_RESPONSE_TIME,
fallbackTTFB: DEFAULT_FALLBACK_TTFB,
maximumConcurrentRequests: DEFAULT_MAXIMUM_CONCURRENT_REQUESTS,
},
options
);

this._rtt = this._options.rtt;
this._throughput = this._options.throughput;
this._defaultResponseTime = this._options.defaultResponseTime;
this._fallbackTTFB = this._options.fallbackTTFB;
this._maximumConcurrentRequests = Math.min(
TcpConnection.maximumSaturatedConnections(this._rtt, this._throughput),
this._options.maximumConcurrentRequests
Expand All @@ -57,22 +60,19 @@ class Estimator {
* @param {!WebInspector.NetworkRequest} record
* @return {number}
*/
static getResponseTime(record) {
static getTTFB(record) {
const timing = record._timing;
return (timing && timing.receiveHeadersEnd - timing.sendEnd) || Infinity;
}

_initializeNetworkRecords() {
const records = [];
this._networkRecords = [];

this._graph.getRootNode().traverse(node => {
if (node.type === Node.TYPES.NETWORK) {
records.push(node.record);
this._networkRecords.push(node.record);
}
});

this._networkRecords = records;
return records;
}

_initializeNetworkConnections() {
Expand All @@ -83,21 +83,29 @@ class Estimator {
);

for (const [connectionId, records] of recordsByConnection.entries()) {
const isSsl = records[0].parsedURL.scheme === 'https';
let responseTime = records.reduce(
(min, record) => Math.min(min, Estimator.getResponseTime(record)),
const isTLS = TLS_SCHEMES.includes(records[0].parsedURL.scheme);

// We'll approximate how much time the server for a connection took to respond after receiving
// the request by computing the minimum TTFB time for requests on that connection.
// TTFB = one way latency + server response time + one way latency
// Even though TTFB is greater than server response time, the RTT is underaccounted for by
// not varying per-server and so the difference roughly evens out.
// TODO(patrickhulce): investigate a way to identify per-server RTT
let estimatedResponseTime = records.reduce(
(min, record) => Math.min(min, Estimator.getTTFB(record)),
Infinity
);

if (!Number.isFinite(responseTime)) {
responseTime = this._defaultResponseTime;
// If we couldn't find a TTFB for the requests, use the fallback TTFB instead.
if (!Number.isFinite(estimatedResponseTime)) {
estimatedResponseTime = this._fallbackTTFB;
}

const connection = new TcpConnection(
this._rtt,
this._throughput,
responseTime,
isSsl
estimatedResponseTime,
isTLS
);

connections.set(connectionId, connection);
Expand All @@ -108,7 +116,7 @@ class Estimator {
}

_initializeAuxiliaryData() {
this._nodeAuxiliaryData = new Map();
this._nodeTiming = new Map();
this._nodesCompleted = new Set();
this._nodesInProcess = new Set();
this._nodesInQueue = new Set(); // TODO: replace this with priority queue
Expand Down Expand Up @@ -146,7 +154,7 @@ class Estimator {

this._nodesInQueue.delete(node);
this._nodesInProcess.add(node);
this._nodeAuxiliaryData.set(node, {
this._nodeTiming.set(node, {
startTime: totalElapsedTime,
timeElapsed: 0,
timeElapsedOvershoot: 0,
Expand All @@ -169,15 +177,15 @@ class Estimator {
_estimateTimeRemaining(node) {
if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported');

const auxiliaryData = this._nodeAuxiliaryData.get(node);
const timingData = this._nodeTiming.get(node);
const connection = this._connections.get(node.record.connectionId);
const calculation = connection.calculateTimeToDownload(
node.record.transferSize - auxiliaryData.bytesDownloaded,
auxiliaryData.timeElapsed
const calculation = connection.simulateDownloadUntil(
node.record.transferSize - timingData.bytesDownloaded,
timingData.timeElapsed
);

const estimate = calculation.timeElapsed + auxiliaryData.timeElapsedOvershoot;
auxiliaryData.estimatedTimeElapsed = estimate;
const estimate = calculation.timeElapsed + timingData.timeElapsedOvershoot;
timingData.estimatedTimeElapsed = estimate;
return estimate;
}

Expand All @@ -201,18 +209,18 @@ class Estimator {
_updateProgressMadeInTimePeriod(node, timePeriodLength, totalElapsedTime) {
if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported');

const auxiliaryData = this._nodeAuxiliaryData.get(node);
const timingData = this._nodeTiming.get(node);
const connection = this._connections.get(node.record.connectionId);
const calculation = connection.calculateTimeToDownload(
node.record.transferSize - auxiliaryData.bytesDownloaded,
auxiliaryData.timeElapsed,
timePeriodLength - auxiliaryData.timeElapsedOvershoot
const calculation = connection.simulateDownloadUntil(
node.record.transferSize - timingData.bytesDownloaded,
timingData.timeElapsed,
timePeriodLength - timingData.timeElapsedOvershoot
);

connection.setCongestionWindow(calculation.congestionWindow);

if (auxiliaryData.estimatedTimeElapsed === timePeriodLength) {
auxiliaryData.endTime = totalElapsedTime;
if (timingData.estimatedTimeElapsed === timePeriodLength) {
timingData.endTime = totalElapsedTime;

connection.setWarmed(true);
this._connectionsInUse.delete(connection);
Expand All @@ -224,10 +232,9 @@ class Estimator {
this._enqueueNodeIfPossible(dependent);
}
} else {
auxiliaryData.timeElapsed += calculation.timeElapsed;
auxiliaryData.timeElapsedOvershoot +=
calculation.timeElapsed - timePeriodLength;
auxiliaryData.bytesDownloaded += calculation.bytesDownloaded;
timingData.timeElapsed += calculation.timeElapsed;
timingData.timeElapsedOvershoot += calculation.timeElapsed - timePeriodLength;
timingData.bytesDownloaded += calculation.bytesDownloaded;
}
}

Expand Down
10 changes: 5 additions & 5 deletions lighthouse-core/gather/computed/dependency-graph/estimator/tcp-connection.js
View file
Expand Up @@ -19,7 +19,7 @@ class TcpConnection {
this._warmed = false;
this._ssl = ssl;
this._rtt = rtt;
this._availableThroughput = throughput;
this._throughput = throughput;
this._responseTime = responseTime;
this._congestionWindow = INITIAL_CONGESTION_WINDOW;
}
Expand All @@ -41,7 +41,7 @@ class TcpConnection {
* @return {number}
*/
_computeMaximumCongestionWindowInSegments() {
const bytesPerSecond = this._availableThroughput / 8;
const bytesPerSecond = this._throughput / 8;
const secondsPerRoundTrip = this._rtt / 1000;
const bytesPerRoundTrip = bytesPerSecond * secondsPerRoundTrip;
return Math.floor(bytesPerRoundTrip / TCP_SEGMENT_SIZE);
Expand All @@ -51,7 +51,7 @@ class TcpConnection {
* @param {number} throughput
*/
setThroughput(throughput) {
this._availableThroughput = throughput;
this._throughput = throughput;
}

/**
Expand Down Expand Up @@ -80,7 +80,7 @@ class TcpConnection {
* @param {number=} maximumTimeToElapse
* @return {{timeElapsed: number, roundTrips: number, bytesDownloaded: number, congestionWindow: number}}
*/
calculateTimeToDownload(bytesToDownload, timeAlreadyElapsed = 0, maximumTimeToElapse = Infinity) {
simulateDownloadUntil(bytesToDownload, timeAlreadyElapsed = 0, maximumTimeToElapse = Infinity) {
const twoWayLatency = this._rtt;
const oneWayLatency = twoWayLatency / 2;
const maximumCongestionWindow = this._computeMaximumCongestionWindowInSegments();
Expand All @@ -94,7 +94,7 @@ class TcpConnection {
oneWayLatency +
// ACK + Application Data
oneWayLatency +
// ClientHello/ServerHello with TLS False Start
// ClientHello/ServerHello assuming TLS Flase Start is enabled (https://istlsfastyet.com/#server-performance).
(this._ssl ? twoWayLatency : 0);
}

Expand Down
10 changes: 5 additions & 5 deletions lighthouse-core/test/gather/computed/dependency-graph/estimator/estimator-test.js
View file
Expand Up @@ -31,7 +31,7 @@ describe('DependencyGraph/Estimator', () => {
describe('.estimate', () => {
it('should estimate basic graphs', () => {
const rootNode = new Node(request({}));
const estimator = new Estimator(rootNode, {defaultResponseTime: 500});
const estimator = new Estimator(rootNode, {fallbackTTFB: 500});
const result = estimator.estimate();
// should be 2 RTTs and 500ms for the server response time
assert.equal(result, 300 + 500);
Expand All @@ -47,7 +47,7 @@ describe('DependencyGraph/Estimator', () => {
nodeB.addDependent(nodeC);
nodeC.addDependent(nodeD);

const estimator = new Estimator(nodeA, {defaultResponseTime: 500});
const estimator = new Estimator(nodeA, {fallbackTTFB: 500});
const result = estimator.estimate();
// should be 800ms each for A, B, C, D
assert.equal(result, 3200);
Expand All @@ -63,7 +63,7 @@ describe('DependencyGraph/Estimator', () => {
nodeA.addDependent(nodeC);
nodeA.addDependent(nodeD);

const estimator = new Estimator(nodeA, {defaultResponseTime: 500});
const estimator = new Estimator(nodeA, {fallbackTTFB: 500});
const result = estimator.estimate();
// should be 800ms for A and 950ms for C (2 round trips of downloading)
assert.equal(result, 800 + 950);
Expand All @@ -79,7 +79,7 @@ describe('DependencyGraph/Estimator', () => {
nodeA.addDependent(nodeC);
nodeA.addDependent(nodeD);

const estimator = new Estimator(nodeA, {defaultResponseTime: 500});
const estimator = new Estimator(nodeA, {fallbackTTFB: 500});
const result = estimator.estimate();
// should be 800ms for A and 650ms for the next 3
assert.equal(result, 800 + 650 * 3);
Expand All @@ -95,7 +95,7 @@ describe('DependencyGraph/Estimator', () => {
nodeA.addDependent(nodeC);
nodeA.addDependent(nodeD);

const estimator = new Estimator(nodeA, {defaultResponseTime: 500});
const estimator = new Estimator(nodeA, {fallbackTTFB: 500});
const result = estimator.estimate();
// should be 800ms for A and 950ms for C (2 round trips of downloading)
assert.equal(result, 800 + 950);
Expand Down

0 comments on commit a632442

Please sign in to comment.