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): handle disk cache simulation #5221

Merged
merged 7 commits into from
May 18, 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
5 changes: 4 additions & 1 deletion lighthouse-core/gather/computed/metrics/lantern-metric.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,11 @@ class LanternMetricArtifact extends ComputedArtifact {
Object.assign({}, extras, {optimistic: false})
);

// Estimates under 1s don't really follow the normal curve fit, minimize the impact of the intercept
const interceptMultiplier = this.COEFFICIENTS.intercept > 0 ?
Math.min(1, optimisticEstimate.timeInMs / 1000) : 1;
const timing =
this.COEFFICIENTS.intercept +
this.COEFFICIENTS.intercept * interceptMultiplier +
this.COEFFICIENTS.optimistic * optimisticEstimate.timeInMs +
this.COEFFICIENTS.pessimistic * pessimisticEstimate.timeInMs;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class SpeedIndex extends MetricArtifact {
* @return {LH.Gatherer.Simulation.Result}
*/
getEstimateFromSimulation(simulationResult, extras) {
const fcpTimeInMs = extras.fcpResult.timing;
const fcpTimeInMs = extras.fcpResult.pessimisticEstimate.timeInMs;
const estimate = extras.optimistic
? extras.speedline.speedIndex
: SpeedIndex.computeLayoutBasedSpeedIndex(simulationResult.nodeTimings, fcpTimeInMs);
Expand Down
7 changes: 7 additions & 0 deletions lighthouse-core/lib/dependency-graph/network-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ class NetworkNode extends Node {
return this._record._initiator && this._record._initiator.type;
}

/**
* @return {boolean}
*/
get fromDiskCache() {
return !!this._record._fromDiskCache;
}

/**
* @return {boolean}
*/
Expand Down
96 changes: 66 additions & 30 deletions lighthouse-core/lib/dependency-graph/simulator/simulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,15 @@ class Simulator {

if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported');

// Start a network request if we're not at max requests and a connection is available
const numberOfActiveRequests = this._numberInProgress(node.type);
if (numberOfActiveRequests >= this._maximumConcurrentRequests) return;
const connection = this._acquireConnection(/** @type {NetworkNode} */ (node).record);
if (!connection) return;
const networkNode = /** @type {NetworkNode} */ (node);
// If a network request is cached, we can always start it, so skip the connection checks
if (!networkNode.fromDiskCache) {
// Start a network request if we're not at max requests and a connection is available
const numberOfActiveRequests = this._numberInProgress(node.type);
if (numberOfActiveRequests >= this._maximumConcurrentRequests) return;
const connection = this._acquireConnection(networkNode.record);
if (!connection) return;
}

this._markNodeAsInProgress(node, totalElapsedTime);
this._setTimingData(node, {
Expand All @@ -209,32 +213,58 @@ class Simulator {
*/
_estimateTimeRemaining(node) {
if (node.type === Node.TYPES.CPU) {
const timingData = this._nodeTimings.get(node);
const multiplier = /** @type {CpuNode} */ (node).didPerformLayout()
? this._layoutTaskMultiplier
: this._cpuSlowdownMultiplier;
const totalDuration = Math.min(
Math.round(/** @type {CpuNode} */ (node).event.dur / 1000 * multiplier),
DEFAULT_MAXIMUM_CPU_TASK_DURATION
);
const estimatedTimeElapsed = totalDuration - timingData.timeElapsed;
this._setTimingData(node, {estimatedTimeElapsed});
return estimatedTimeElapsed;
return this._estimateCPUTimeRemaining(/** @type {CpuNode} */ (node));
} else if (node.type === Node.TYPES.NETWORK) {
return this._estimateNetworkTimeRemaining(/** @type {NetworkNode} */ (node));
} else {
throw new Error('Unsupported');
}
}

if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported');

const record = /** @type {NetworkNode} */ (node).record;
const timingData = this._nodeTimings.get(node);
// If we're estimating time remaining, we already acquired a connection for this record, definitely non-null
const connection = /** @type {TcpConnection} */ (this._acquireConnection(record));
const calculation = connection.simulateDownloadUntil(
record.transferSize - timingData.bytesDownloaded,
{timeAlreadyElapsed: timingData.timeElapsed, maximumTimeToElapse: Infinity}
/**
* @param {CpuNode} cpuNode
* @return {number}
*/
_estimateCPUTimeRemaining(cpuNode) {
const timingData = this._nodeTimings.get(cpuNode);
const multiplier = cpuNode.didPerformLayout()
? this._layoutTaskMultiplier
: this._cpuSlowdownMultiplier;
const totalDuration = Math.min(
Math.round(cpuNode.event.dur / 1000 * multiplier),
DEFAULT_MAXIMUM_CPU_TASK_DURATION
);
const estimatedTimeElapsed = totalDuration - timingData.timeElapsed;
this._setTimingData(cpuNode, {estimatedTimeElapsed});
return estimatedTimeElapsed;
}

/**
* @param {NetworkNode} networkNode
* @return {number}
*/
_estimateNetworkTimeRemaining(networkNode) {
const timingData = this._nodeTimings.get(networkNode);

let timeElapsed = 0;
if (networkNode.fromDiskCache) {
// Rough access time for seeking to location on disk and reading sequentially
// @see http://norvig.com/21-days.html#answers
const sizeInMb = (networkNode.record._resourceSize || 0) / 1024 / 1024;
timeElapsed = 8 + 20 * sizeInMb - timingData.timeElapsed;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is slower in Chrome. UMA must have some timings we could look at.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we're not really accounting for the full Chrome IPC RT time, just the OS/disk access, but the same could be said for network really

perhaps we add an IPC RT latency factor in the future where we account for more based on the chunk size of all IPC-based traffic?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay yah sg. DiskCache.0.AsyncIOTime is the best one but it doesn't give us much as its measured from the cache thread and no cost to browser IO thread or over the IPC to the renderer.

} else {
// If we're estimating time remaining, we already acquired a connection for this record, definitely non-null
const connection = /** @type {TcpConnection} */ (this._acquireConnection(networkNode.record));
const calculation = connection.simulateDownloadUntil(
networkNode.record.transferSize - timingData.bytesDownloaded,
{timeAlreadyElapsed: timingData.timeElapsed, maximumTimeToElapse: Infinity}
);

timeElapsed = calculation.timeElapsed;
}

const estimatedTimeElapsed = calculation.timeElapsed + timingData.timeElapsedOvershoot;
this._setTimingData(node, {estimatedTimeElapsed});
const estimatedTimeElapsed = timeElapsed + timingData.timeElapsedOvershoot;
this._setTimingData(networkNode, {estimatedTimeElapsed});
return estimatedTimeElapsed;
}

Expand All @@ -261,15 +291,17 @@ class Simulator {
const timingData = this._nodeTimings.get(node);
const isFinished = timingData.estimatedTimeElapsed === timePeriodLength;

if (node.type === Node.TYPES.CPU) {
const networkNode = /** @type {NetworkNode} */ (node);

if (node.type === Node.TYPES.CPU || networkNode.fromDiskCache) {
return isFinished
? this._markNodeAsComplete(node, totalElapsedTime)
: (timingData.timeElapsed += timePeriodLength);
}

if (node.type !== Node.TYPES.NETWORK) throw new Error('Unsupported');

const record = /** @type {NetworkNode} */ (node).record;
const record = networkNode.record;
// If we're updating the progress, we already acquired a connection for this record, definitely non-null
const connection = /** @type {TcpConnection} */ (this._acquireConnection(record));
const calculation = connection.simulateDownloadUntil(
Expand Down Expand Up @@ -345,7 +377,11 @@ class Simulator {
}

if (!nodesInProgress.size) {
throw new Error('Failed to start a node, potential mismatch in original execution');
// interplay between fromDiskCache and connectionReused can be incorrect
// proceed with flexibleOrdering if we can, otherwise give up
if (this._flexibleOrdering) throw new Error('Failed to start a node');
this._flexibleOrdering = true;
continue;
}

// set the available throughput for all connections based on # inflight
Expand Down
6 changes: 3 additions & 3 deletions lighthouse-core/test/audits/first-meaningful-paint-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ describe('Performance: first-meaningful-paint audit', () => {
const context = {options, settings: {throttlingMethod: 'simulate'}};
const fmpResult = await FMPAudit.audit(artifacts, context);

assert.equal(fmpResult.score, 0.95);
assert.equal(Util.formatDisplayValue(fmpResult.displayValue), '2,030\xa0ms');
assert.equal(Math.round(fmpResult.rawValue), 2029);
assert.equal(fmpResult.score, 0.96);
assert.equal(Util.formatDisplayValue(fmpResult.displayValue), '1,950\xa0ms');
assert.equal(Math.round(fmpResult.rawValue), 1949);
});
});
6 changes: 3 additions & 3 deletions lighthouse-core/test/audits/metrics-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ describe('Performance: metrics', () => {
const result = await Audit.audit(artifacts, {settings});

assert.deepStrictEqual(result.details.items[0], {
firstContentfulPaint: 1272,
firstContentfulPaint: 1038,
firstContentfulPaintTs: undefined,
firstMeaningfulPaint: 2029,
firstMeaningfulPaint: 1949,
firstMeaningfulPaintTs: undefined,
firstCPUIdle: 4309,
firstCPUIdleTs: undefined,
interactive: 4309,
interactiveTs: undefined,
speedIndex: 1501,
speedIndex: 1461,
speedIndexTs: undefined,
estimatedInputLatency: 104,
estimatedInputLatencyTs: undefined,
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/audits/predictive-perf-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ describe('Performance: predictive performance audit', () => {
assert.equal(output.displayValue, '4,310\xa0ms');

const valueOf = name => Math.round(output.extendedInfo.value[name]);
assert.equal(valueOf('roughEstimateOfFCP'), 1272);
assert.equal(valueOf('roughEstimateOfFCP'), 1038);
assert.equal(valueOf('optimisticFCP'), 611);
assert.equal(valueOf('pessimisticFCP'), 611);
assert.equal(valueOf('roughEstimateOfFMP'), 2029);
assert.equal(valueOf('roughEstimateOfFMP'), 1949);
assert.equal(valueOf('optimisticFMP'), 911);
assert.equal(valueOf('pessimisticFMP'), 1198);
assert.equal(valueOf('roughEstimateOfTTI'), 4309);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('Metrics: FCP', () => {
const settings = {throttlingMethod: 'simulate'};
const result = await artifacts.requestFirstContentfulPaint({trace, devtoolsLog, settings});

assert.equal(Math.round(result.timing), 1272);
assert.equal(Math.round(result.timing), 1038);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 611);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 611);
assert.equal(result.optimisticEstimate.nodeTimings.size, 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('Metrics: FMP', () => {

const result = await artifacts.requestFirstMeaningfulPaint({trace, devtoolsLog, settings});

assert.equal(Math.round(result.timing), 2029);
assert.equal(Math.round(result.timing), 1949);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 911);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1198);
assert.equal(result.optimisticEstimate.nodeTimings.size, 4);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('Metrics: Lantern FCP', () => {
const result = await artifacts.requestLanternFirstContentfulPaint({trace, devtoolsLog,
settings: {}});

assert.equal(Math.round(result.timing), 1272);
assert.equal(Math.round(result.timing), 1038);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 611);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 611);
assert.equal(result.optimisticEstimate.nodeTimings.size, 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('Metrics: Lantern FMP', () => {
const result = await artifacts.requestLanternFirstMeaningfulPaint({trace, devtoolsLog,
settings: {}});

assert.equal(Math.round(result.timing), 2029);
assert.equal(Math.round(result.timing), 1949);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 911);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1198);
assert.equal(result.optimisticEstimate.nodeTimings.size, 4);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ describe('Metrics: Lantern Speed Index', () => {
const artifacts = Runner.instantiateComputedArtifacts();
const result = await artifacts.requestLanternSpeedIndex({trace, devtoolsLog, settings: {}});

assert.equal(Math.round(result.timing), 1501);
assert.equal(Math.round(result.timing), 1461);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 605);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1392);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1330);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ describe('Metrics: Speed Index', () => {
const settings = {throttlingMethod: 'simulate'};
const result = await artifacts.requestSpeedIndex({trace, devtoolsLog, settings});

assert.equal(Math.round(result.timing), 1501);
assert.equal(Math.round(result.timing), 1461);
assert.equal(Math.round(result.optimisticEstimate.timeInMs), 605);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1392);
assert.equal(Math.round(result.pessimisticEstimate.timeInMs), 1330);
});

it('should compute an observed value', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,19 @@ describe('DependencyGraph/Simulator', () => {
assertNodeTiming(result, nodeD, {startTime: 2400, endTime: 3200});
});

it('should simulate cached network graphs', () => {
const nodeA = new NetworkNode(request({startTime: 0, endTime: 1, _fromDiskCache: true}));
const nodeB = new NetworkNode(request({startTime: 0, endTime: 3, _fromDiskCache: true}));
nodeA.addDependent(nodeB);

const simulator = new Simulator({serverResponseTimeByOrigin});
const result = simulator.simulate(nodeA);
// should be ~8ms each for A, B
assert.equal(result.timeInMs, 16);
assertNodeTiming(result, nodeA, {startTime: 0, endTime: 8});
assertNodeTiming(result, nodeB, {startTime: 8, endTime: 16});
});

it('should simulate basic CPU queue graphs', () => {
const nodeA = new NetworkNode(request({}));
const nodeB = new CpuNode(cpuTask({duration: 100}));
Expand Down
1 change: 1 addition & 0 deletions typings/web-inspector.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ declare global {
_transferSize?: number;
/** Should use a default of 0 if not defined */
_resourceSize?: number;
_fromDiskCache?: boolean;

finished: boolean;
requestMethod: string;
Expand Down