Skip to content

Commit fa03ae1

Browse files
alxhubkara
authored andcommitted
fix(benchpress): work around missing events from Chrome 63 (#21396)
Chrome 63 can cause the navigationStart event for the first run to arrive with a different pid than the start of the benchpress run. This makes the first collected result invalid. This workaround causes the sampler to ignore runs that have this condition. PR Close #21396
1 parent 27196b6 commit fa03ae1

File tree

3 files changed

+27
-1
lines changed

3 files changed

+27
-1
lines changed

packages/benchpress/src/metric/perflog_metric.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,9 @@ export class PerflogMetric extends Metric {
249249
// not all events have been received, no further processing for now
250250
return null;
251251
}
252+
if (markStartEvent.pid !== markEndEvent.pid) {
253+
result['invalid'] = 1;
254+
}
252255

253256
let gcTimeInScript = 0;
254257
let renderTimeInScript = 0;

packages/benchpress/src/sampler.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,12 @@ export class Sampler {
6363
}
6464
return resultPromise.then((_) => this._driver.waitFor(this._execute))
6565
.then((_) => this._metric.endMeasure(this._prepare === Options.NO_PREPARE))
66-
.then((measureValues) => this._report(lastState, measureValues));
66+
.then((measureValues) => {
67+
if (!!measureValues['invalid']) {
68+
return lastState;
69+
}
70+
return this._report(lastState, measureValues);
71+
});
6772
}
6873

6974
private _report(state: SampleState, metricValues: {[key: string]: any}): Promise<SampleState> {

packages/benchpress/test/metric/perflog_metric_spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,24 @@ import {TraceEventFactory} from '../trace_event_factory';
537537
});
538538
}));
539539

540+
it('should mark a run as invalid if the start and end marks are different',
541+
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
542+
const otherProcessEventFactory = new TraceEventFactory('timeline', 'pid1');
543+
const metric = createMetric(
544+
[[
545+
eventFactory.markStart('benchpress0', 0), eventFactory.start('script', 0, null),
546+
eventFactory.end('script', 5, null),
547+
otherProcessEventFactory.start('script', 10, null),
548+
otherProcessEventFactory.end('script', 17, null),
549+
otherProcessEventFactory.markEnd('benchpress0', 20)
550+
]],
551+
null !);
552+
metric.beginMeasure().then((_) => metric.endMeasure(false)).then((data) => {
553+
expect(data['invalid']).toBe(1);
554+
async.done();
555+
});
556+
}));
557+
540558
it('should support scriptTime metric',
541559
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
542560
aggregate([

0 commit comments

Comments
 (0)