From 32aeb1052db97c89d1b56821b13af162183efdb3 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Thu, 15 Sep 2016 09:25:45 -0700 Subject: [PATCH] refactor(benchpress): normalize phase `b` into `B` and `e` into `E` This simplifies the perflog metrics and prevents future errors. --- .../benchpress/src/firefox_extension/lib/main.ts | 4 ++-- .../benchpress/src/metric/perflog_metric.ts | 12 ++++++------ .../benchpress/src/web_driver_extension.ts | 14 +++++++++----- .../src/webdriver/chrome_driver_extension.ts | 16 ++++++---------- .../src/webdriver/ios_driver_extension.ts | 6 +++--- .../benchpress/test/trace_event_factory.ts | 4 ++-- 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/modules/@angular/benchpress/src/firefox_extension/lib/main.ts b/modules/@angular/benchpress/src/firefox_extension/lib/main.ts index 870bfd781a..4786914b07 100644 --- a/modules/@angular/benchpress/src/firefox_extension/lib/main.ts +++ b/modules/@angular/benchpress/src/firefox_extension/lib/main.ts @@ -42,11 +42,11 @@ class Profiler { } addStartEvent(name: string, timeStarted: number) { - this._markerEvents.push({ph: 'b', ts: timeStarted - this._profilerStartTime, name: name}); + this._markerEvents.push({ph: 'B', ts: timeStarted - this._profilerStartTime, name: name}); } addEndEvent(name: string, timeEnded: number) { - this._markerEvents.push({ph: 'e', ts: timeEnded - this._profilerStartTime, name: name}); + this._markerEvents.push({ph: 'E', ts: timeEnded - this._profilerStartTime, name: name}); } } diff --git a/modules/@angular/benchpress/src/metric/perflog_metric.ts b/modules/@angular/benchpress/src/metric/perflog_metric.ts index 70d609deab..0e141543cf 100644 --- a/modules/@angular/benchpress/src/metric/perflog_metric.ts +++ b/modules/@angular/benchpress/src/metric/perflog_metric.ts @@ -222,13 +222,13 @@ export class PerflogMetric extends Metric { events.forEach((event) => { var ph = event['ph']; var name = event['name']; - if (ph === 'b' && name === markName) { + if (ph === 'B' && name === markName) { markStartEvent = event; } else if (ph === 'I' && name === 'navigationStart') { // if a benchmark measures reload of a page, use the last // navigationStart as begin event markStartEvent = event; - } else if (ph === 'e' && name === markName) { + } else if (ph === 'E' && name === markName) { markEndEvent = event; } }); @@ -272,7 +272,7 @@ export class PerflogMetric extends Metric { } else if (this._receivedData && name === 'receivedData' && ph === 'I') { result['receivedData'] += event['args']['encodedDataLength']; } - if (ph === 'b' && name === _MARK_NAME_FRAME_CAPUTRE) { + if (ph === 'B' && name === _MARK_NAME_FRAME_CAPUTRE) { if (frameCaptureStartEvent) { throw new Error('can capture frames only once per benchmark run'); } @@ -281,7 +281,7 @@ export class PerflogMetric extends Metric { 'found start event for frame capture, but frame capture was not requested in benchpress'); } frameCaptureStartEvent = event; - } else if (ph === 'e' && name === _MARK_NAME_FRAME_CAPUTRE) { + } else if (ph === 'E' && name === _MARK_NAME_FRAME_CAPUTRE) { if (!frameCaptureStartEvent) { throw new Error('missing start event for frame capture'); } @@ -297,14 +297,14 @@ export class PerflogMetric extends Metric { } } - if (ph === 'B' || ph === 'b') { + if (ph === 'B') { if (!intervalStarts[name]) { intervalStartCount[name] = 1; intervalStarts[name] = event; } else { intervalStartCount[name]++; } - } else if ((ph === 'E' || ph === 'e') && intervalStarts[name]) { + } else if ((ph === 'E') && intervalStarts[name]) { intervalStartCount[name]--; if (intervalStartCount[name] === 0) { var startEvent = intervalStarts[name]; diff --git a/modules/@angular/benchpress/src/web_driver_extension.ts b/modules/@angular/benchpress/src/web_driver_extension.ts index 7b78043bae..afd680f372 100644 --- a/modules/@angular/benchpress/src/web_driver_extension.ts +++ b/modules/@angular/benchpress/src/web_driver_extension.ts @@ -14,13 +14,18 @@ import {isBlank, isPresent} from './facade/lang'; export type PerfLogEvent = { [key: string]: any } & { - cat?: string, - ph?: 'X' | 'B' | 'E' | 'b' | 'e' | 'I', + ph?: 'X' | 'B' | 'E' | 'I', ts?: number, dur?: number, name?: string, pid?: string, - args?: {encodedDataLength?: number, usedHeapSize?: number, majorGc?: number} + args?: { + encodedDataLength?: number, + usedHeapSize?: number, + majorGc?: boolean, + url?: string, + method?: string + } }; /** @@ -66,8 +71,7 @@ export abstract class WebDriverExtension { * Format: * - cat: category of the event * - name: event name: 'script', 'gc', 'render', ... - * - ph: phase: 'B' (begin), 'E' (end), 'b' (nestable start), 'e' (nestable end), 'X' (Complete - *event) + * - ph: phase: 'B' (begin), 'E' (end), 'X' (Complete event), 'I' (Instant event) * - ts: timestamp in ms, e.g. 12345 * - pid: process id * - args: arguments, e.g. {heapSize: 1234} diff --git a/modules/@angular/benchpress/src/webdriver/chrome_driver_extension.ts b/modules/@angular/benchpress/src/webdriver/chrome_driver_extension.ts index c457cabe96..384ae4a0fd 100644 --- a/modules/@angular/benchpress/src/webdriver/chrome_driver_extension.ts +++ b/modules/@angular/benchpress/src/webdriver/chrome_driver_extension.ts @@ -158,7 +158,7 @@ export class ChromeDriverExtension extends WebDriverExtension { let normArgs = {'url': data['url'], 'method': data['requestMethod']}; return normalizeEvent(event, {'name': 'sendRequest', 'args': normArgs}); } else if (this._isEvent(categories, name, ['blink.user_timing'], 'navigationStart')) { - return normalizeEvent(event, {'name': name}); + return normalizeEvent(event, {'name': 'navigationStart'}); } return null; // nothing useful in this event } @@ -182,23 +182,19 @@ export class ChromeDriverExtension extends WebDriverExtension { } } -function normalizeEvent( - chromeEvent: {[key: string]: any}, data: {[key: string]: any}): PerfLogEvent { - var ph = chromeEvent['ph']; +function normalizeEvent(chromeEvent: {[key: string]: any}, data: PerfLogEvent): PerfLogEvent { + var ph = chromeEvent['ph'].toUpperCase(); if (ph === 'S') { - ph = 'b'; + ph = 'B'; } else if (ph === 'F') { - ph = 'e'; + ph = 'E'; } else if (ph === 'R') { // mark events from navigation timing ph = 'I'; - } else if (ph === 'i') { - // legacy support - ph = 'I'; } var result: {[key: string]: any} = {'pid': chromeEvent['pid'], 'ph': ph, 'cat': 'timeline', 'ts': chromeEvent['ts'] / 1000}; - if (chromeEvent['ph'] === 'X') { + if (ph === 'X') { var dur = chromeEvent['dur']; if (dur === undefined) { dur = chromeEvent['tdur']; diff --git a/modules/@angular/benchpress/src/webdriver/ios_driver_extension.ts b/modules/@angular/benchpress/src/webdriver/ios_driver_extension.ts index b34c6fef00..0259485bcb 100644 --- a/modules/@angular/benchpress/src/webdriver/ios_driver_extension.ts +++ b/modules/@angular/benchpress/src/webdriver/ios_driver_extension.ts @@ -97,7 +97,7 @@ export class IOsDriverExtension extends WebDriverExtension { } function createEvent( - ph: 'X' | 'B' | 'E' | 'b' | 'e', name: string, time: number, args: any = null) { + ph: 'X' | 'B' | 'E' | 'B' | 'E', name: string, time: number, args: any = null) { var result: PerfLogEvent = { 'cat': 'timeline', 'name': name, @@ -122,9 +122,9 @@ function createEndEvent(name: string, time: number, args: any = null) { } function createMarkStartEvent(name: string, time: number) { - return createEvent('b', name, time); + return createEvent('B', name, time); } function createMarkEndEvent(name: string, time: number) { - return createEvent('e', name, time); + return createEvent('E', name, time); } diff --git a/modules/@angular/benchpress/test/trace_event_factory.ts b/modules/@angular/benchpress/test/trace_event_factory.ts index ed113b51d6..55da9f8c15 100644 --- a/modules/@angular/benchpress/test/trace_event_factory.ts +++ b/modules/@angular/benchpress/test/trace_event_factory.ts @@ -21,9 +21,9 @@ export class TraceEventFactory { return res; } - markStart(name: string, time: number) { return this.create('b', name, time); } + markStart(name: string, time: number) { return this.create('B', name, time); } - markEnd(name: string, time: number) { return this.create('e', name, time); } + markEnd(name: string, time: number) { return this.create('E', name, time); } start(name: string, time: number, args: any = null) { return this.create('B', name, time, args); }