Skip to content

Commit

Permalink
fix: normalize all times to navStart, remove traceviewer model (#2347)
Browse files Browse the repository at this point in the history
* Remove traceviewer's tracingModel

* revert user timings changes

* fix remaining tests

* add trace with redirects

* use first navstart, not last

* Revert "use first navstart, not last"

* reduce -> for..of (#2399)

*  feedback

* bug fix and test

* add second test

* mainthread events

* test updates

* add back colon traceg

* fixed user timing test

* use navstart as default

* tasks -> events
  • Loading branch information
patrickhulce authored Jun 6, 2017
1 parent 63b4ac1 commit d5f5c52
Show file tree
Hide file tree
Showing 18 changed files with 148,491 additions and 219 deletions.
5 changes: 2 additions & 3 deletions lighthouse-core/audits/consistently-interactive.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,11 @@ class ConsistentlyInteractiveMetric extends Audit {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const computedArtifacts = [
artifacts.requestNetworkRecords(devtoolsLog),
artifacts.requestTracingModel(trace),
artifacts.requestTraceOfTab(trace),
];

return Promise.all(computedArtifacts)
.then(([networkRecords, traceModel, traceOfTab]) => {
.then(([networkRecords, traceOfTab]) => {
if (!traceOfTab.timestamps.firstMeaningfulPaint) {
throw new Error('No firstMeaningfulPaint found in trace.');
}
Expand All @@ -221,7 +220,7 @@ class ConsistentlyInteractiveMetric extends Audit {
throw new Error('No domContentLoaded found in trace.');
}

const longTasks = TracingProcessor.getMainThreadTopLevelEvents(traceModel, trace)
const longTasks = TracingProcessor.getMainThreadTopLevelEvents(traceOfTab)
.filter(event => event.duration >= 50);
const quietPeriodInfo = this.findOverlappingQuietPeriods(longTasks, networkRecords,
traceOfTab);
Expand Down
15 changes: 5 additions & 10 deletions lighthouse-core/audits/estimated-input-latency.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ class EstimatedInputLatency extends Audit {
};
}

static calculate(tabTrace, model, trace) {
static calculate(tabTrace) {
const startTime = tabTrace.timings.firstMeaningfulPaint;
if (!startTime) {
throw new Error('No firstMeaningfulPaint event found in trace');
}

const latencyPercentiles = TracingProcessor.getRiskToResponsiveness(model, trace, startTime);
const latencyPercentiles = TracingProcessor.getRiskToResponsiveness(tabTrace, startTime);
const ninetieth = latencyPercentiles.find(result => result.percentile === 0.9);
const rawValue = parseFloat(ninetieth.time.toFixed(1));

Expand All @@ -57,7 +57,7 @@ class EstimatedInputLatency extends Audit {

return {
score: Math.round(score),
optimalValue: this.meta.optimalValue,
optimalValue: EstimatedInputLatency.meta.optimalValue,
rawValue,
displayValue: Util.formatMilliseconds(rawValue, 1),
extendedInfo: {
Expand All @@ -76,13 +76,8 @@ class EstimatedInputLatency extends Audit {
static audit(artifacts) {
const trace = artifacts.traces[this.DEFAULT_PASS];

const pending = [
artifacts.requestTraceOfTab(trace),
artifacts.requestTracingModel(trace)
];
return Promise.all(pending).then(([tabTrace, model]) => {
return EstimatedInputLatency.calculate(tabTrace, model, trace);
});
return artifacts.requestTraceOfTab(trace)
.then(EstimatedInputLatency.calculate);
}
}

Expand Down
9 changes: 4 additions & 5 deletions lighthouse-core/audits/time-to-interactive.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class TTIMetric extends Audit {

// Get our expected latency for the time window
const latencies = TracingProcessor.getRiskToResponsiveness(
data.model, data.trace, startTime, endTime, percentiles);
data.tabTrace, startTime, endTime, percentiles);
const estLatency = latencies[0].time;
foundLatencies.push({
estLatency: estLatency,
Expand Down Expand Up @@ -159,10 +159,9 @@ class TTIMetric extends Audit {
debugString = `Trace error: ${err.message}`;
return null;
}),
artifacts.requestTraceOfTab(trace),
artifacts.requestTracingModel(trace)
artifacts.requestTraceOfTab(trace)
];
return Promise.all(pending).then(([speedline, tabTrace, model]) => {
return Promise.all(pending).then(([speedline, tabTrace]) => {
// frame monotonic timestamps from speedline are in ms (ts / 1000), so we'll match
// https://github.com/pmdartus/speedline/blob/123f512632a/src/frame.js#L86
const fMPtsInMS = tabTrace.timestamps.firstMeaningfulPaint;
Expand All @@ -188,7 +187,7 @@ class TTIMetric extends Audit {
}

const times = {fmpTiming, visuallyReadyTiming, traceEndTiming};
const data = {tabTrace, model, trace};
const data = {tabTrace, trace};
const timeToInteractive = TTIMetric.findTTIAlpha(times, data);
const timeToInteractiveB = TTIMetric.findTTIAlphaFMPOnly(times, data);
const timeToInteractiveC = TTIMetric.findTTIAlphaFMPOnly5s(times, data);
Expand Down
19 changes: 18 additions & 1 deletion lighthouse-core/closure/typedefs/ComputedArtifacts.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,31 @@
* @externs
*/

/** @typedef
* {{
* navigationStart: number,
* firstPaint: number,
* firstContentfulPaint: number,
* firstMeaningfulPaint: number,
* traceEnd: number,
* onLoad: number,
* domContentLoaded: number,
* }}
*/
let TraceTimes;

/** @typedef
{{
timings: !TraceTimes,
timestamps: !TraceTimes,
processEvents: !Array<!TraceEvent>,
mainThreadEvents: !Array<!TraceEvent>,
startedInPageEvt: !TraceEvent,
navigationStartEvt: !TraceEvent,
firstPaintEvt: TraceEvent,
firstContentfulPaintEvt: TraceEvent,
firstMeaningfulPaintEvt: TraceEvent
firstMeaningfulPaintEvt: TraceEvent,
onLoadEvt: TraceEvent,
}} */
let TraceOfTabArtifact;

Expand Down
13 changes: 4 additions & 9 deletions lighthouse-core/gather/computed/first-interactive.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,10 @@ class FirstInteractive extends ComputedArtifact {
}

/**
* @param {!Trace} trace
* @param {!tr.Model} traceModel
* @param {!TraceOfTabArtifact} traceOfTab
* @return {{timeInMs: number, timestamp: number}}
*/
computeWithArtifacts(trace, traceModel, traceOfTab) {
computeWithArtifacts(traceOfTab) {
const navStart = traceOfTab.timestamps.navigationStart;
const FMP = traceOfTab.timings.firstMeaningfulPaint;
const DCL = traceOfTab.timings.domContentLoaded;
Expand All @@ -183,7 +181,7 @@ class FirstInteractive extends ComputedArtifact {
throw new Error(`No ${FMP ? 'domContentLoaded' : 'firstMeaningfulPaint'} event in trace`);
}

const longTasksAfterFMP = TracingProcessor.getMainThreadTopLevelEvents(traceModel, trace, FMP)
const longTasksAfterFMP = TracingProcessor.getMainThreadTopLevelEvents(traceOfTab, FMP)
.filter(evt => evt.duration >= LONG_TASK_THRESHOLD);
const firstInteractive = FirstInteractive.findQuietWindow(FMP, traceEnd, longTasksAfterFMP);

Expand All @@ -200,11 +198,8 @@ class FirstInteractive extends ComputedArtifact {
* @return {{timeInMs: number, timestamp: number}}
*/
compute_(trace, artifacts) {
return Promise.all([
artifacts.requestTracingModel(trace),
artifacts.requestTraceOfTab(trace),
]).then(([traceModel, traceOfTab]) => {
return this.computeWithArtifacts(trace, traceModel, traceOfTab);
return artifacts.requestTraceOfTab(trace).then(traceOfTab => {
return this.computeWithArtifacts(traceOfTab);
});
}
}
Expand Down
10 changes: 7 additions & 3 deletions lighthouse-core/gather/computed/trace-of-tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class TraceOfTab extends ComputedArtifact {

/**
* @param {{traceEvents: !Array}} trace
* @return {!{processEvents: !Array<TraceEvent>, startedInPageEvt: TraceEvent, navigationStartEvt: TraceEvent, firstContentfulPaintEvt: TraceEvent, firstMeaningfulPaintEvt: TraceEvent}}
* @return {!TraceOfTabArtifact}
*/
compute_(trace) {
// Parse the trace for our key events and sort them by timestamp.
Expand Down Expand Up @@ -86,6 +86,9 @@ class TraceOfTab extends ComputedArtifact {
.filter(e => e.pid === startedInPageEvt.pid)
.sort((event0, event1) => event0.ts - event1.ts);

const mainThreadEvents = processEvents
.filter(e => e.tid === startedInPageEvt.tid);

const traceEnd = trace.traceEvents.reduce((max, evt) => {
return max.ts > evt.ts ? max : evt;
});
Expand All @@ -95,7 +98,7 @@ class TraceOfTab extends ComputedArtifact {
firstPaint,
firstContentfulPaint,
firstMeaningfulPaint,
traceEnd,
traceEnd: {ts: traceEnd.ts + (traceEnd.dur || 0)},
onLoad,
domContentLoaded,
};
Expand All @@ -112,7 +115,8 @@ class TraceOfTab extends ComputedArtifact {
timings,
timestamps,
processEvents,
startedInPageEvt: startedInPageEvt,
mainThreadEvents,
startedInPageEvt,
navigationStartEvt: navigationStart,
firstPaintEvt: firstPaint,
firstContentfulPaintEvt: firstContentfulPaint,
Expand Down
29 changes: 0 additions & 29 deletions lighthouse-core/gather/computed/tracing-model.js

This file was deleted.

Loading

0 comments on commit d5f5c52

Please sign in to comment.