Skip to content

Commit 1c62db3

Browse files
Michael Gerakispaulirish
authored andcommitted
Traces that are passed in through the config file are cleaned
Traces that are passed in through the config file are cleaned to handle duplicate/missing TracingStartedInPage events.
1 parent 1275762 commit 1c62db3

File tree

5 files changed

+189
-16
lines changed

5 files changed

+189
-16
lines changed

lighthouse-core/audits/first-meaningful-paint.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,12 @@ class FirstMeaningfulPaint extends Audit {
132132
score = Math.min(100, score);
133133
score = Math.max(0, score);
134134

135+
if (!data.navStart) {
136+
throw new Error(FAILURE_MESSAGE);
137+
}
138+
135139
timings.navStart = data.navStart.ts / 1000;
140+
136141
return {
137142
duration: `${firstMeaningfulPaint.toFixed(1)}`,
138143
score: Math.round(score),
@@ -243,7 +248,10 @@ class FirstMeaningfulPaint extends Audit {
243248
}
244249
});
245250

246-
const paintAfterMSLayout = evts.paints.find(e => e.ts > mostSignificantLayout.ts);
251+
let paintAfterMSLayout;
252+
if (mostSignificantLayout) {
253+
paintAfterMSLayout = evts.paints.find(e => e.ts > mostSignificantLayout.ts);
254+
}
247255
return paintAfterMSLayout;
248256
}
249257
}

lighthouse-core/audits/time-to-interactive.js

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -79,17 +79,22 @@ class TTIMetric extends Audit {
7979
const fMPts = timings.fMPfull + timings.navStart;
8080

8181
// look at speedline results for 85% starting at FMP
82-
let eightyFivePctVC = artifacts.Speedline.frames.find(frame => {
83-
return frame.getTimeStamp() >= fMPts && frame.getProgress() >= 85;
84-
});
82+
let eightyFivePctVC;
83+
let visuallyReadyTiming = 0;
8584

86-
// Check to avoid closure compiler null dereferencing errors
87-
if (eightyFivePctVC === undefined) {
88-
eightyFivePctVC = 0;
89-
}
85+
if (artifacts.Speedline.frames) {
86+
eightyFivePctVC = artifacts.Speedline.frames.find(frame => {
87+
return frame.getTimeStamp() >= fMPts && frame.getProgress() >= 85;
88+
});
9089

91-
// TODO CHECK these units are the same
92-
const visuallyReadyTiming = (eightyFivePctVC.getTimeStamp() - timings.navStart) || 0;
90+
// Check to avoid closure compiler null dereferencing errors
91+
if (eightyFivePctVC === undefined) {
92+
eightyFivePctVC = 0;
93+
}
94+
95+
// TODO CHECK these units are the same
96+
visuallyReadyTiming = eightyFivePctVC.getTimeStamp() - timings.navStart || 0;
97+
}
9398

9499
// Find first 500ms window where Est Input Latency is <50ms at the 90% percentile.
95100
let startTime = Math.max(fmpTiming, visuallyReadyTiming) - 50;
@@ -150,6 +155,7 @@ class TTIMetric extends Audit {
150155
rawValue: timeToInteractive,
151156
displayValue: `${timeToInteractive}ms`,
152157
optimalValue: this.meta.optimalValue,
158+
debugString: artifacts.Speedline.debugString,
153159
extendedInfo: {
154160
value: extendedInfo,
155161
formatter: Formatter.SUPPORTED_FORMATS.NULL

lighthouse-core/config/index.js

Lines changed: 102 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,92 @@
1919
const defaultConfig = require('./default.json');
2020
const recordsFromLogs = require('../lib/network-recorder').recordsFromLogs;
2121
const CriticalRequestChainsGatherer = require('../driver/gatherers/critical-request-chains');
22+
const SpeedlineGatherer = require('../driver/gatherers/speedline');
2223
const Driver = require('../driver');
2324
const log = require('../lib/log');
2425

26+
// cleanTrace is run to remove duplicate TracingStartedInPage events,
27+
// and to change TracingStartedInBrowser events into TracingStartedInPage.
28+
// This is done by searching for most occuring threads and basing new events
29+
// off of those.
30+
function cleanTrace(traceContents) {
31+
// Keep track of most occuring threads
32+
const threads = [];
33+
const countsByThread = {};
34+
const traceStartEvents = [];
35+
const makeMockEvent = (evt, ts) => {
36+
return {
37+
pid: evt.pid,
38+
tid: evt.tid,
39+
ts: ts || 0, // default to 0 for now
40+
ph: 'I',
41+
cat: 'disabled-by-default-devtools.timeline',
42+
name: 'TracingStartedInPage',
43+
args: {
44+
data: {
45+
page: evt.frame
46+
}
47+
},
48+
s: 't'
49+
};
50+
};
51+
52+
let frame;
53+
let data;
54+
let name;
55+
let counter;
56+
57+
traceContents.forEach((evt, idx) => {
58+
if (evt.name.startsWith('TracingStartedIn')) {
59+
traceStartEvents.push(idx);
60+
}
61+
62+
// find the event's frame
63+
data = evt.args && (evt.args.data || evt.args.beginData || evt.args.counters);
64+
frame = (evt.args && evt.args.frame) || data && (data.frame || data.page);
65+
66+
if (!frame) {
67+
return;
68+
}
69+
70+
// Increase occurences count of the frame
71+
name = `pid${evt.pid}-tid${evt.tid}-frame${frame}`;
72+
counter = countsByThread[name];
73+
if (!counter) {
74+
counter = {
75+
pid: evt.pid,
76+
tid: evt.tid,
77+
frame: frame,
78+
count: 0
79+
};
80+
countsByThread[name] = counter;
81+
threads.push(counter);
82+
}
83+
counter.count++;
84+
});
85+
86+
// find most active thread (and frame)
87+
threads.sort((a, b) => b.count - a.count);
88+
const mostActiveFrame = threads[0];
89+
90+
// Remove all current TracingStartedIn* events, storing
91+
// the first events ts.
92+
const ts = traceContents[traceStartEvents[0]] && traceContents[traceStartEvents[0]].ts;
93+
94+
// account for offset after removing items
95+
let i = 0;
96+
for (let dup of traceStartEvents) {
97+
traceContents.splice(dup - i, 1);
98+
i++;
99+
}
100+
101+
// Add a new TracingStartedInPage event based on most active thread
102+
// and using TS of first found TracingStartedIn* event
103+
traceContents.unshift(makeMockEvent(mostActiveFrame, ts));
104+
105+
return traceContents;
106+
}
107+
25108
function filterPasses(passes, audits) {
26109
const requiredGatherers = getGatherersNeededByAudits(audits);
27110

@@ -96,18 +179,28 @@ function expandAudits(audits) {
96179
});
97180
}
98181

99-
function expandArtifacts(artifacts) {
182+
function expandArtifacts(artifacts, includeSpeedline) {
100183
const expandedArtifacts = Object.assign({}, artifacts);
101184

102185
// currently only trace logs and performance logs should be imported
103186
if (artifacts.traces) {
187+
let trace;
104188
Object.keys(artifacts.traces).forEach(key => {
105189
if (artifacts.traces[key].traceContents) {
106-
expandedArtifacts.traces[key].traceContents =
107-
require(artifacts.traces[key].traceContents);
190+
log.log('info', 'Normalizng trace contents into expected state...');
191+
trace = require(artifacts.traces[key].traceContents);
192+
193+
expandedArtifacts.traces[key].traceContents = cleanTrace(trace.traceEvents || trace);
108194
}
109195
});
110196
}
197+
198+
if (includeSpeedline) {
199+
const speedline = new SpeedlineGatherer();
200+
speedline.afterPass({}, {traceContents: expandedArtifacts.traces.defaultPass.traceContents});
201+
expandedArtifacts.Speedline = speedline.artifact;
202+
}
203+
111204
if (artifacts.performanceLog) {
112205
expandedArtifacts.CriticalRequestChains =
113206
parsePerformanceLog(require(artifacts.performanceLog));
@@ -146,7 +239,12 @@ class Config {
146239
// filterPasses expects audits to have been expanded
147240
this._passes = configJSON.passes ? filterPasses(configJSON.passes, this._audits) : null;
148241
this._auditResults = configJSON.auditResults ? Array.from(configJSON.auditResults) : null;
149-
this._artifacts = configJSON.artifacts ? expandArtifacts(configJSON.artifacts) : null;
242+
this._artifacts = null;
243+
if (configJSON.artifacts) {
244+
this._artifacts = expandArtifacts(configJSON.artifacts,
245+
// If time-to-interactive is present, add the speedline artifact
246+
configJSON.audits && configJSON.audits.find(a => a === 'time-to-interactive'));
247+
}
150248
this._aggregations = configJSON.aggregations ? Array.from(configJSON.aggregations) : null;
151249
}
152250

lighthouse-core/test/config/index.js

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,19 +131,76 @@ describe('Config', () => {
131131
const config = new Config({
132132
artifacts: {
133133
traces: {
134-
firstPass: {
134+
defaultPass: {
135135
traceContents: path.resolve(__dirname, '../fixtures/traces/trace-user-timings.json')
136136
}
137137
},
138138
performanceLog: path.resolve(__dirname, '../fixtures/perflog.json')
139139
}
140140
});
141141
const traceUserTimings = require('../fixtures/traces/trace-user-timings.json');
142-
assert.deepStrictEqual(config.artifacts.traces.firstPass.traceContents, traceUserTimings);
142+
assert.deepStrictEqual(config.artifacts.traces.defaultPass.traceContents, traceUserTimings);
143143
assert.ok(config.artifacts.CriticalRequestChains);
144144
assert.ok(config.artifacts.CriticalRequestChains['93149.1']);
145145
assert.ok(config.artifacts.CriticalRequestChains['93149.1'].request);
146146
assert.ok(config.artifacts.CriticalRequestChains['93149.1'].children);
147147
});
148+
149+
it('handles traces with no TracingStartedInPage events', () => {
150+
const config = new Config({
151+
artifacts: {
152+
traces: {
153+
defaultPass: {
154+
traceContents: path.resolve(__dirname,
155+
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json')
156+
}
157+
},
158+
performanceLog: path.resolve(__dirname, '../fixtures/perflog.json')
159+
}
160+
});
161+
162+
assert.ok(config.artifacts.traces.defaultPass.traceContents.find(
163+
e => e.name === 'TracingStartedInPage' && e.args.data.page === '0xhad00p'));
164+
});
165+
166+
it('doesnt add speedline artifact to tests without tti audit', () => {
167+
const config = new Config({
168+
artifacts: {
169+
traces: {
170+
defaultPass: {
171+
traceContents: path.resolve(__dirname,
172+
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json')
173+
}
174+
},
175+
performanceLog: path.resolve(__dirname, '../fixtures/perflog.json')
176+
},
177+
audits: [
178+
'first-meaningful-paint'
179+
180+
]
181+
});
182+
183+
assert.equal(config.artifacts.Speedline, undefined);
184+
});
185+
186+
it('does add speedline artifact to tests without tti audit', () => {
187+
const config = new Config({
188+
artifacts: {
189+
traces: {
190+
defaultPass: {
191+
traceContents: path.resolve(__dirname,
192+
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json')
193+
}
194+
},
195+
performanceLog: path.resolve(__dirname, '../fixtures/perflog.json')
196+
},
197+
audits: [
198+
'first-meaningful-paint',
199+
'time-to-interactive'
200+
]
201+
});
202+
203+
assert.notEqual(config.artifacts.Speedline, undefined);
204+
});
148205
});
149206

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
[{"pid":41904,"tid":1295,"ts":506085991145,"ph":"R","cat":"blink.user_timing","name":"navigationStart","args":{"frame": "0xhad00p"},"tts":314882},
2+
{"pid":41904,"tid":1295,"ts":506086992099,"ph":"R","cat":"blink.user_timing","name":"mark_test","args":{},"tts":331149},
3+
{"pid":41904,"tid":1295,"ts":506085991147,"ph":"b","cat":"blink.user_timing","name":"measure_test","args":{},"tts":331184,"id":"0x73b016"},
4+
{"pid":41904,"tid":1295,"ts":506086992112,"ph":"e","cat":"blink.user_timing","name":"measure_test","args":{},"tts":331186,"id":"0x73b016"}]

0 commit comments

Comments
 (0)