Skip to content

Commit

Permalink
core(simulator): start nodes in observed order (#5362)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce committed Oct 23, 2018
1 parent 5aebc82 commit 253053f
Show file tree
Hide file tree
Showing 15 changed files with 173 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class LanternEstimatedInputLatency extends LanternMetric {
});
}

return events;
return events.sort((a, b) => a.start - b.start);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class LanternFirstCPUIdle extends LanternInteractive {
longTasks.push({start: timing.startTime, end: timing.endTime});
}

longTasks.sort((a, b) => a.start - b.start);
// Require here to resolve circular dependency.
const FirstCPUIdle = require('./first-cpu-idle');
return FirstCPUIdle.findQuietWindow(fmpTimeInMs, Infinity, longTasks);
Expand Down
28 changes: 22 additions & 6 deletions lighthouse-core/lib/dependency-graph/simulator/simulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,17 @@ class Simulator {
});
}

/**
* @param {Set<Node>} nodes
* @return {Node[]}
*/
_getNodesSortedByStartTime(nodes) {
return Array.from(nodes).sort((nodeA, nodeB) => {
// Sort nodes by startTime to match original execution order
return nodeA.startTime - nodeB.startTime;
});
}

/**
* @param {Node} node
* @param {number} totalElapsedTime
Expand Down Expand Up @@ -354,18 +365,23 @@ class Simulator {
}
}

/**
* @return {Map<Node, LH.Gatherer.Simulation.NodeTiming>}
*/
_computeFinalNodeTimings() {
/** @type {Map<Node, LH.Gatherer.Simulation.NodeTiming>} */
const nodeTimings = new Map();
/** @type {Array<[Node, LH.Gatherer.Simulation.NodeTiming]>} */
const nodeTimingEntries = [];
for (const [node, timing] of this._nodeTimings) {
nodeTimings.set(node, {
nodeTimingEntries.push([node, {
startTime: timing.startTime,
endTime: timing.endTime,
duration: timing.endTime - timing.startTime,
});
}]);
}

return nodeTimings;
// Most consumers will want the entries sorted by startTime, so insert them in that order
nodeTimingEntries.sort((a, b) => a[1].startTime - b[1].startTime);
return new Map(nodeTimingEntries);
}

/**
Expand Down Expand Up @@ -419,7 +435,7 @@ class Simulator {
// loop as long as we have nodes in the queue or currently in progress
while (nodesReadyToStart.size || nodesInProgress.size) {
// move all possible queued nodes to in progress
for (const node of nodesReadyToStart) {
for (const node of this._getNodesSortedByStartTime(nodesReadyToStart)) {
this._startNodeIfPossible(node, totalElapsedTime);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`Performance: metrics evaluates valid input correctly 1`] = `
Object {
"estimatedInputLatency": 104,
"estimatedInputLatency": 100,
"estimatedInputLatencyTs": undefined,
"firstCPUIdle": 3351,
"firstCPUIdleTs": undefined,
Expand Down Expand Up @@ -32,7 +32,7 @@ Object {
"observedSpeedIndexTs": 225414776724,
"observedTraceEnd": 12540,
"observedTraceEndTs": 225426711887,
"speedIndex": 1656,
"speedIndex": 1657,
"speedIndexTs": undefined,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`Performance: predictive performance audit should compute the predicted values 1`] = `
Object {
"optimisticEIL": 101,
"optimisticEIL": 93,
"optimisticFCP": 911,
"optimisticFMP": 1211,
"optimisticSI": 605,
Expand All @@ -11,13 +11,13 @@ Object {
"pessimisticEIL": 158,
"pessimisticFCP": 911,
"pessimisticFMP": 1498,
"pessimisticSI": 1630,
"pessimisticSI": 1631,
"pessimisticTTFCPUI": 3502,
"pessimisticTTI": 3502,
"roughEstimateOfEIL": 104,
"roughEstimateOfEIL": 100,
"roughEstimateOfFCP": 911,
"roughEstimateOfFMP": 1355,
"roughEstimateOfSI": 1656,
"roughEstimateOfSI": 1657,
"roughEstimateOfTTFCPUI": 3351,
"roughEstimateOfTTI": 3427,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
exports[`Byte efficiency base audit should allow overriding of computeWasteWithTTIGraph 1`] = `
Object {
"default": 1330,
"justTTI": 950,
"justTTI": 800,
}
`;

exports[`Byte efficiency base audit should create load simulator with the specified settings 1`] = `1330`;

exports[`Byte efficiency base audit should create load simulator with the specified settings 2`] = `22950`;
exports[`Byte efficiency base audit should create load simulator with the specified settings 2`] = `22730`;
156 changes: 78 additions & 78 deletions lighthouse-core/test/fixtures/lantern-master-computed-values.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

exports[`Metrics: EIL should compute a simulated value 1`] = `
Object {
"optimistic": 101,
"optimistic": 93,
"pessimistic": 158,
"timing": 104,
"timing": 100,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

exports[`Metrics: Lantern EIL should compute a simulated value 1`] = `
Object {
"optimistic": 101,
"optimistic": 93,
"pessimistic": 158,
"timing": 104,
"timing": 100,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`Metrics: Lantern Speed Index should compute predicted value 1`] = `
Object {
"optimistic": 605,
"pessimistic": 1630,
"timing": 1656,
"pessimistic": 1631,
"timing": 1657,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`Metrics: Speed Index should compute a simulated value 1`] = `
Object {
"optimistic": 605,
"pessimistic": 1630,
"timing": 1656,
"pessimistic": 1631,
"timing": 1657,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,20 @@ describe('Metrics: Lantern TTFCPUI', () => {
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});

describe('#getFirstCPUIdleWindowStart', () => {
it('should sort tasks', () => {
const tasks = new Map([
[{type: 'cpu'}, {startTime: 600, endTime: 700, duration: 100}],
[{type: 'cpu'}, {startTime: 300, endTime: 400, duration: 100}],
[{type: 'cpu'}, {startTime: 0, endTime: 100, duration: 100}],
[{type: 'cpu'}, {startTime: 100, endTime: 200, duration: 100}],
[{type: 'cpu'}, {startTime: 500, endTime: 600, duration: 100}],
[{type: 'cpu'}, {startTime: 200, endTime: 300, duration: 100}],
[{type: 'cpu'}, {startTime: 400, endTime: 500, duration: 100}],
]);

assert.equal(LanternFirstCPUIdle.getFirstCPUIdleWindowStart(tasks, 0), 700);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const NetworkNode = require('../../../../lib/dependency-graph/network-node');
const CpuNode = require('../../../../lib/dependency-graph/cpu-node');
const Simulator = require('../../../../lib/dependency-graph/simulator/simulator');
const DNSCache = require('../../../../lib/dependency-graph/simulator/dns-cache');
const PageDependencyGraph = require('../../../../gather/computed/page-dependency-graph');

const assert = require('assert');
let nextRequestId = 1;
Expand All @@ -30,7 +31,7 @@ function request(opts) {
function cpuTask({tid, ts, duration}) {
tid = tid || nextTid++;
ts = ts || 0;
const dur = (duration || 0) * 1000 / 5;
const dur = ((duration || 0) * 1000) / 5;
return {tid, ts, dur};
}

Expand Down Expand Up @@ -255,5 +256,32 @@ describe('DependencyGraph/Simulator', () => {
const simulator = new Simulator({serverResponseTimeByOrigin});
assert.throws(() => simulator.simulate(rootNode), /cycle/);
});

describe('on a real trace', () => {
const trace = require('../../../fixtures/traces/progressive-app-m60.json');
const devtoolsLog = require('../../../fixtures/traces/progressive-app-m60.devtools.log.json');
let result;

beforeAll(async () => {
const computedCache = new Map();
const graph = await PageDependencyGraph.request({trace, devtoolsLog}, {computedCache});
const simulator = new Simulator({serverResponseTimeByOrigin});
result = simulator.simulate(graph);
});

it('should compute a timeInMs', () => {
expect(result.timeInMs).toBeGreaterThan(100);
});

it('should sort the task event times', () => {
const nodeTimings = Array.from(result.nodeTimings.entries());

for (let i = 1; i < nodeTimings.length; i++) {
const startTime = nodeTimings[i][1].startTime;
const previousStartTime = nodeTimings[i - 1][1].startTime;
expect(startTime).toBeGreaterThanOrEqual(previousStartTime);
}
});
});
});
});
12 changes: 6 additions & 6 deletions lighthouse-core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -1951,9 +1951,9 @@
"id": "unminified-javascript",
"title": "Minify JavaScript",
"description": "Minifying JavaScript files can reduce payload sizes and script parse time. [Learn more](https://developers.google.com/speed/docs/insights/MinifyResources).",
"score": 1,
"score": 0.88,
"scoreDisplayMode": "numeric",
"rawValue": 0,
"rawValue": 150,
"displayValue": "Potential savings of 30 KB",
"warnings": [],
"details": {
Expand Down Expand Up @@ -1983,7 +1983,7 @@
"wastedPercent": 42.52388078488413
}
],
"overallSavingsMs": 0,
"overallSavingsMs": 150,
"overallSavingsBytes": 30470
}
},
Expand Down Expand Up @@ -2070,9 +2070,9 @@
"id": "uses-text-compression",
"title": "Enable text compression",
"description": "Text-based resources should be served with compression (gzip, deflate or brotli) to minimize total network bytes. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/text-compression).",
"score": 0.88,
"score": 0.75,
"scoreDisplayMode": "numeric",
"rawValue": 150,
"rawValue": 300,
"displayValue": "Potential savings of 63 KB",
"details": {
"type": "opportunity",
Expand Down Expand Up @@ -2105,7 +2105,7 @@
"wastedBytes": 8442
}
],
"overallSavingsMs": 150,
"overallSavingsMs": 300,
"overallSavingsBytes": 64646
}
},
Expand Down
8 changes: 4 additions & 4 deletions proto/sample_v2_round_trip.json
Original file line number Diff line number Diff line change
Expand Up @@ -2075,12 +2075,12 @@
}
],
"overallSavingsBytes": 30470.0,
"overallSavingsMs": 0.0,
"overallSavingsMs": 150.0,
"type": "opportunity"
},
"displayValue": "Potential savings of 30\u00a0KB",
"id": "unminified-javascript",
"score": 1.0,
"score": 0.88,
"scoreDisplayMode": "numeric",
"title": "Minify JavaScript",
"warnings": []
Expand Down Expand Up @@ -2452,12 +2452,12 @@
}
],
"overallSavingsBytes": 64646.0,
"overallSavingsMs": 150.0,
"overallSavingsMs": 300.0,
"type": "opportunity"
},
"displayValue": "Potential savings of 63\u00a0KB",
"id": "uses-text-compression",
"score": 0.88,
"score": 0.75,
"scoreDisplayMode": "numeric",
"title": "Enable text compression"
},
Expand Down

0 comments on commit 253053f

Please sign in to comment.