Skip to content
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

core(lantern): rename Simulation.Result.nodeTiming to be plural #5038

Merged
merged 3 commits into from
Apr 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ class UnusedBytes extends Audit {
});

const savingsOnTTI = Math.max(
ConsistentlyInteractive.getLastLongTaskEndTime(simulationBeforeChanges.nodeTiming) -
ConsistentlyInteractive.getLastLongTaskEndTime(simulationAfterChanges.nodeTiming),
ConsistentlyInteractive.getLastLongTaskEndTime(simulationBeforeChanges.nodeTimings) -
ConsistentlyInteractive.getLastLongTaskEndTime(simulationAfterChanges.nodeTimings),
0
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ const WebInspector = require('../../lib/web-inspector');
const MINIMUM_WASTED_MS = 50;

/**
* Given a simulation's nodeTiming, return an object with the nodes/timing keyed by network URL
* @param {LH.Gatherer.Simulation.Result['nodeTiming']} nodeTimingMap
* Given a simulation's nodeTimings, return an object with the nodes/timing keyed by network URL
* @param {LH.Gatherer.Simulation.Result['nodeTimings']} nodeTimings
* @return {Object<string, {node: Node, nodeTiming: LH.Gatherer.Simulation.NodeTiming}>}
*/
const getNodesAndTimingByUrl = nodeTimingMap => {
const nodes = Array.from(nodeTimingMap.keys());
const getNodesAndTimingByUrl = nodeTimings => {
const nodes = Array.from(nodeTimings.keys());
return nodes.reduce((map, node) => {
map[node.record && node.record.url] = {node, nodeTiming: nodeTimingMap.get(node)};
map[node.record && node.record.url] = {node, nodeTiming: nodeTimings.get(node)};
return map;
}, {});
};
Expand Down Expand Up @@ -70,7 +70,7 @@ class RenderBlockingResources extends Audit {
const fcpSimulation = await artifacts.requestFirstContentfulPaint(metricComputationData);
const fcpTsInMs = traceOfTab.timestamps.firstContentfulPaint / 1000;

const nodesByUrl = getNodesAndTimingByUrl(fcpSimulation.optimisticEstimate.nodeTiming);
const nodesByUrl = getNodesAndTimingByUrl(fcpSimulation.optimisticEstimate.nodeTimings);

const results = [];
const deferredNodeIds = new Set();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ class ConsistentlyInteractive extends MetricArtifact {
* @return {LH.Gatherer.Simulation.Result}
*/
getEstimateFromSimulation(simulationResult, extras) {
const lastTaskAt = ConsistentlyInteractive.getLastLongTaskEndTime(simulationResult.nodeTiming);
const lastTaskAt = ConsistentlyInteractive.getLastLongTaskEndTime(simulationResult.nodeTimings);
const minimumTime = extras.optimistic
? extras.fmpResult.optimisticEstimate.timeInMs
: extras.fmpResult.pessimisticEstimate.timeInMs;
return {
timeInMs: Math.max(minimumTime, lastTaskAt),
nodeTiming: simulationResult.nodeTiming,
nodeTimings: simulationResult.nodeTimings,
};
}

Expand All @@ -94,12 +94,12 @@ class ConsistentlyInteractive extends MetricArtifact {
}

/**
* @param {Map<Node, {startTime?: number, endTime?: number}>} nodeTiming
* @param {Map<Node, LH.Gatherer.Simulation.NodeTiming>} nodeTimings
* @return {number}
*/
static getLastLongTaskEndTime(nodeTiming, duration = 50) {
static getLastLongTaskEndTime(nodeTimings, duration = 50) {
// @ts-ignore TS can't infer how the object invariants change
return Array.from(nodeTiming.entries())
return Array.from(nodeTimings.entries())
.filter(([node, timing]) => {
if (node.type !== Node.TYPES.CPU) return false;
if (!timing.endTime || !timing.startTime) return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,20 @@ class LanternFirstCPUIdle extends LanternConsistentlyInteractive {
: extras.fmpResult.pessimisticEstimate.timeInMs;

return {
timeInMs: LanternFirstCPUIdle.getFirstCPUIdleWindowStart(simulation.nodeTiming, fmpTimeInMs),
nodeTiming: simulation.nodeTiming,
timeInMs: LanternFirstCPUIdle.getFirstCPUIdleWindowStart(simulation.nodeTimings, fmpTimeInMs),
nodeTimings: simulation.nodeTimings,
};
}

/**
*
* @param {Map<Node, LH.Gatherer.Simulation.NodeTiming>} nodeTiming
* @param {Map<Node, LH.Gatherer.Simulation.NodeTiming>} nodeTimings
* @param {number} fmpTimeInMs
*/
static getFirstCPUIdleWindowStart(nodeTiming, fmpTimeInMs, longTaskLength = 50) {
static getFirstCPUIdleWindowStart(nodeTimings, fmpTimeInMs, longTaskLength = 50) {
/** @type {Array<{start: number, end: number}>} */
const longTasks = [];
for (const [node, timing] of nodeTiming.entries()) {
for (const [node, timing] of nodeTimings.entries()) {
if (node.type !== Node.TYPES.CPU) continue;
if (!timing.endTime || !timing.startTime) continue;
if (timing.endTime - timing.startTime < longTaskLength) continue;
Expand Down
10 changes: 5 additions & 5 deletions lighthouse-core/gather/computed/metrics/lantern-speed-index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ class SpeedIndex extends MetricArtifact {
const fcpTimeInMs = extras.fcpResult.timing;
const estimate = extras.optimistic
? extras.speedline.speedIndex
: SpeedIndex.computeLayoutBasedSpeedIndex(simulationResult.nodeTiming, fcpTimeInMs);
: SpeedIndex.computeLayoutBasedSpeedIndex(simulationResult.nodeTimings, fcpTimeInMs);
return {
timeInMs: estimate,
nodeTiming: simulationResult.nodeTiming,
nodeTimings: simulationResult.nodeTimings,
};
}

Expand Down Expand Up @@ -84,14 +84,14 @@ class SpeedIndex extends MetricArtifact {
* different methods. Read more in the evaluation doc.
*
* @see https://docs.google.com/document/d/1qJWXwxoyVLVadezIp_Tgdk867G3tDNkkVRvUJSH3K1E/edit#
* @param {Map<Node, LH.Gatherer.Simulation.NodeTiming>} nodeTiming
* @param {Map<Node, LH.Gatherer.Simulation.NodeTiming>} nodeTimings
* @param {number} fcpTimeInMs
* @return {number}
*/
static computeLayoutBasedSpeedIndex(nodeTiming, fcpTimeInMs) {
static computeLayoutBasedSpeedIndex(nodeTimings, fcpTimeInMs) {
/** @type {Array<{time: number, weight: number}>} */
const layoutWeights = [];
for (const [node, timing] of nodeTiming.entries()) {
for (const [node, timing] of nodeTimings.entries()) {
if (node.type !== Node.TYPES.CPU) continue;
if (!timing.startTime || !timing.endTime) continue;

Expand Down
16 changes: 8 additions & 8 deletions lighthouse-core/lib/dependency-graph/simulator/simulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class Simulator {
this._layoutTaskMultiplier = this._cpuSlowdownMultiplier * this._options.layoutTaskMultiplier;

// Properties reset on every `.simulate` call but duplicated here for type checking
this._nodeTiming = new Map();
this._nodeTimings = new Map();
this._numberInProgressByType = new Map();
this._nodes = {};
// @ts-ignore
Expand All @@ -78,7 +78,7 @@ class Simulator {
* Initializes the various state data structures such as _nodesReadyToStart and _nodesCompleted.
*/
_initializeAuxiliaryData() {
this._nodeTiming = new Map();
this._nodeTimings = new Map();
this._numberInProgressByType = new Map();

this._nodes = {};
Expand All @@ -100,9 +100,9 @@ class Simulator {
* @param {LH.Gatherer.Simulation.NodeTiming} values
*/
_setTimingData(node, values) {
const timingData = this._nodeTiming.get(node) || {};
const timingData = this._nodeTimings.get(node) || {};
Object.assign(timingData, values);
this._nodeTiming.set(node, timingData);
this._nodeTimings.set(node, timingData);
}

/**
Expand Down Expand Up @@ -195,7 +195,7 @@ class Simulator {
*/
_estimateTimeRemaining(node) {
if (node.type === Node.TYPES.CPU) {
const timingData = this._nodeTiming.get(node);
const timingData = this._nodeTimings.get(node);
const multiplier = /** @type {CpuNode} */ (node).didPerformLayout()
? this._layoutTaskMultiplier
: this._cpuSlowdownMultiplier;
Expand All @@ -211,7 +211,7 @@ class Simulator {
if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported');

const record = /** @type {NetworkNode} */ (node).record;
const timingData = this._nodeTiming.get(node);
const timingData = this._nodeTimings.get(node);
const connection = /** @type {TcpConnection} */ (this._connectionPool.acquire(record));
const calculation = connection.simulateDownloadUntil(
record.transferSize - timingData.bytesDownloaded,
Expand Down Expand Up @@ -243,7 +243,7 @@ class Simulator {
* @param {number} totalElapsedTime
*/
_updateProgressMadeInTimePeriod(node, timePeriodLength, totalElapsedTime) {
const timingData = this._nodeTiming.get(node);
const timingData = this._nodeTimings.get(node);
const isFinished = timingData.estimatedTimeElapsed === timePeriodLength;

if (node.type === Node.TYPES.CPU) {
Expand Down Expand Up @@ -329,7 +329,7 @@ class Simulator {

return {
timeInMs: totalElapsedTime,
nodeTiming: this._nodeTiming,
nodeTimings: this._nodeTimings,
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ describe('Metrics: TTCI', () => {
assert.equal(Math.round(result.timing), 5308);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 2451);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 2752);
assert.equal(result.optimisticEstimate.nodeTiming.size, 19);
assert.equal(result.pessimisticEstimate.nodeTiming.size, 79);
assert.equal(result.optimisticEstimate.nodeTimings.size, 19);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 79);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ describe('Metrics: FCP', () => {
assert.equal(Math.round(result.timing), 2038);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 611);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 611);
assert.equal(result.optimisticEstimate.nodeTiming.size, 2);
assert.equal(result.pessimisticEstimate.nodeTiming.size, 2);
assert.equal(result.optimisticEstimate.nodeTimings.size, 2);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 2);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ describe('FirstInteractive computed artifact:', () => {
assert.equal(Math.round(result.timing), 5308);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 2451);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 2752);
assert.equal(result.optimisticEstimate.nodeTiming.size, 19);
assert.equal(result.pessimisticEstimate.nodeTiming.size, 79);
assert.equal(result.optimisticEstimate.nodeTimings.size, 19);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 79);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ describe('Metrics: FMP', () => {
assert.equal(Math.round(result.timing), 2851);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 911);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1198);
assert.equal(result.optimisticEstimate.nodeTiming.size, 4);
assert.equal(result.pessimisticEstimate.nodeTiming.size, 7);
assert.equal(result.optimisticEstimate.nodeTimings.size, 4);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 7);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ describe('Metrics: Lantern TTCI', () => {
assert.equal(Math.round(result.timing), 5308);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 2451);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 2752);
assert.equal(result.optimisticEstimate.nodeTiming.size, 19);
assert.equal(result.pessimisticEstimate.nodeTiming.size, 79);
assert.equal(result.optimisticEstimate.nodeTimings.size, 19);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 79);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ describe('Metrics: Lantern FCP', () => {
assert.equal(Math.round(result.timing), 2038);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 611);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 611);
assert.equal(result.optimisticEstimate.nodeTiming.size, 2);
assert.equal(result.pessimisticEstimate.nodeTiming.size, 2);
assert.equal(result.optimisticEstimate.nodeTimings.size, 2);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 2);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ describe('Metrics: Lantern TTFCPUI', () => {
assert.equal(Math.round(result.timing), 5308);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 2451);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 2752);
assert.equal(result.optimisticEstimate.nodeTiming.size, 19);
assert.equal(result.pessimisticEstimate.nodeTiming.size, 79);
assert.equal(result.optimisticEstimate.nodeTimings.size, 19);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 79);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ describe('Metrics: Lantern FMP', () => {
assert.equal(Math.round(result.timing), 2851);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 911);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1198);
assert.equal(result.optimisticEstimate.nodeTiming.size, 4);
assert.equal(result.pessimisticEstimate.nodeTiming.size, 7);
assert.equal(result.optimisticEstimate.nodeTimings.size, 4);
assert.equal(result.pessimisticEstimate.nodeTimings.size, 7);
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('DependencyGraph/Simulator', () => {
const serverResponseTimeByOrigin = new Map([['http://example.com', 500]]);

function assertNodeTiming(result, node, assertions) {
const timing = result.nodeTiming.get(node);
const timing = result.nodeTimings.get(node);
assert.ok(timing, 'missing node timing information');
Object.keys(assertions).forEach(key => {
assert.equal(timing[key], assertions[key]);
Expand Down
2 changes: 1 addition & 1 deletion typings/gatherer.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ declare global {

export interface Result {
timeInMs: number;
nodeTiming: Map<GraphNode, NodeTiming>;
nodeTimings: Map<GraphNode, NodeTiming>;
}
}
}
Expand Down