Skip to content

Commit 21a293b

Browse files
committed
refactor(bench press): rename metrics and adapt them to the features of the browser
* Rename metrics, add `Time` suffix to all so that they are more consistent * iOS does not give us `gc` metrics, so they should not be reported * Rename `scriptMicroAvg` into `microScriptTimeAvg` * Rename previous `script` metric into `pureScriptTime` metric, and keep `scriptTime` metric as the overall time, so that we still have a shared metric across devices independent of the supported browser features * `microScriptTimeAvg` is now based on overall `scriptTime`, including gc and render time. * Move more shared DI tokens into `common_options` (previously `sample_options`). Closes angular#930
1 parent 75ecaf0 commit 21a293b

16 files changed

+214
-147
lines changed

modules/benchpress/benchpress.es6

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import { bind } from 'angular2/di';
2-
import { JsonFileReporter } from './common';
2+
import { Options } from './common';
33

44
export * from './common';
55
export { SeleniumWebDriverAdapter } from './src/webdriver/selenium_webdriver_adapter';
66

77
var fs = require('fs');
88

9-
// Note: Can't do the `require` call in a facade as it can't be loaded into the browser!
10-
JsonFileReporter.BINDINGS.push(
11-
bind(JsonFileReporter.WRITE_FILE).toValue(writeFile)
9+
// TODO(tbosch): right now we bind the `writeFile` method
10+
// in benchpres/benchpress.es6. This does not work for Dart,
11+
// find another way...
12+
// Note: Can't do the `require` call in a facade as it can't be loaded into the browser
13+
// for our unit tests via karma.
14+
Options.DEFAULT_BINDINGS.push(
15+
bind(Options.WRITE_FILE).toValue(writeFile)
1216
);
1317

1418
function writeFile(filename, content) {

modules/benchpress/common.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ export { Sampler, SampleState } from './src/sampler';
22
export { Metric } from './src/metric';
33
export { Validator } from './src/validator';
44
export { Reporter } from './src/reporter';
5-
export { WebDriverExtension } from './src/web_driver_extension';
5+
export { WebDriverExtension, PerfLogFeatures } from './src/web_driver_extension';
66
export { WebDriverAdapter } from './src/web_driver_adapter';
77
export { SizeValidator } from './src/validator/size_validator';
88
export { RegressionSlopeValidator } from './src/validator/regression_slope_validator';
@@ -13,7 +13,7 @@ export { PerflogMetric } from './src/metric/perflog_metric';
1313
export { ChromeDriverExtension } from './src/webdriver/chrome_driver_extension';
1414
export { IOsDriverExtension } from './src/webdriver/ios_driver_extension';
1515
export { Runner } from './src/runner';
16-
export { Options } from './src/sample_options';
16+
export { Options } from './src/common_options';
1717
export { MeasureValues } from './src/measure_values';
1818
export { MultiMetric } from './src/metric/multi_metric';
1919
export { MultiReporter } from './src/reporter/multi_reporter';

modules/benchpress/src/sample_options.js renamed to modules/benchpress/src/common_options.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { bind, OpaqueToken } from 'angular2/di';
2+
import { DateWrapper } from 'angular2/src/facade/lang';
23

34
export class Options {
5+
static get DEFAULT_BINDINGS() { return _DEFAULT_BINDINGS; }
46
// TODO(tbosch): use static initializer when our transpiler supports it
57
static get SAMPLE_ID() { return _SAMPLE_ID; }
68
// TODO(tbosch): use static initializer when our transpiler supports it
@@ -23,6 +25,10 @@ export class Options {
2325
* Used for micro benchmarks.
2426
**/
2527
static get MICRO_ITERATIONS() { return _MICRO_ITERATIONS; }
28+
// TODO(tbosch): use static initializer when our transpiler supports it
29+
static get NOW() { return _NOW; }
30+
// TODO(tbosch): use static values when our transpiler supports them
31+
static get WRITE_FILE() { return _WRITE_FILE; }
2632
}
2733

2834
var _SAMPLE_ID = new OpaqueToken('Options.sampleId');
@@ -34,3 +40,13 @@ var _EXECUTE = new OpaqueToken('Options.execute');
3440
var _CAPABILITIES = new OpaqueToken('Options.capabilities');
3541
var _USER_AGENT = new OpaqueToken('Options.userAgent');
3642
var _MICRO_ITERATIONS = new OpaqueToken('Options.microIterations');
43+
var _NOW = new OpaqueToken('Options.now');
44+
var _WRITE_FILE = new OpaqueToken('Options.writeFile');
45+
46+
var _DEFAULT_BINDINGS = [
47+
bind(_DEFAULT_DESCRIPTION).toValue({}),
48+
bind(_SAMPLE_DESCRIPTION).toValue({}),
49+
bind(_FORCE_GC).toValue(false),
50+
bind(_PREPARE).toValue(false),
51+
bind(_NOW).toValue( () => DateWrapper.now() )
52+
];

modules/benchpress/src/metric/perflog_metric.js

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ import { isPresent, isBlank, int, BaseException, StringWrapper, Math } from 'ang
33
import { ListWrapper, StringMap, StringMapWrapper } from 'angular2/src/facade/collection';
44
import { bind, OpaqueToken } from 'angular2/di';
55

6-
import { WebDriverExtension } from '../web_driver_extension';
6+
import { WebDriverExtension, PerfLogFeatures } from '../web_driver_extension';
77
import { Metric } from '../metric';
8-
import { Options } from '../sample_options';
8+
import { Options } from '../common_options';
99

1010
/**
1111
* A metric that reads out the performance log
@@ -21,6 +21,7 @@ export class PerflogMetric extends Metric {
2121
_measureCount:int;
2222
_setTimeout:Function;
2323
_microIterations:int;
24+
_perfLogFeatures:PerfLogFeatures;
2425

2526
/**
2627
* @param driverExtension
@@ -35,19 +36,24 @@ export class PerflogMetric extends Metric {
3536
this._measureCount = 0;
3637
this._setTimeout = setTimeout;
3738
this._microIterations = microIterations;
39+
this._perfLogFeatures = driverExtension.perfLogFeatures();
3840
}
3941

4042
describe():StringMap {
4143
var res = {
42-
'script': 'script execution time in ms',
43-
'render': 'render time in ms',
44-
'gcTime': 'gc time in ms',
45-
'gcAmount': 'gc amount in kbytes',
46-
'majorGcTime': 'time of major gcs in ms',
47-
'majorGcAmount': 'amount of major gcs in kbytes'
44+
'scriptTime': 'script execution time in ms, including gc and render',
45+
'pureScriptTime': 'script execution time in ms, without gc nor render'
4846
};
47+
if (this._perfLogFeatures.render) {
48+
res['renderTime'] = 'render time in and ouside of script in ms';
49+
}
50+
if (this._perfLogFeatures.gc) {
51+
res['gcTime'] = 'gc time in and ouside of script in ms';
52+
res['gcAmount'] = 'gc amount in kbytes';
53+
res['majorGcTime'] = 'time of major gcs in ms';
54+
}
4955
if (this._microIterations > 0) {
50-
res['scriptMicroAvg'] = 'average script time for a micro iteration';
56+
res['microScriptTimeAvg'] = 'average script time for a micro iteration';
5157
}
5258
return res;
5359
}
@@ -120,17 +126,22 @@ export class PerflogMetric extends Metric {
120126

121127
_aggregateEvents(events, markName) {
122128
var result = {
123-
'script': 0,
124-
'render': 0,
125-
'gcTime': 0,
126-
'gcAmount': 0,
127-
'majorGcTime': 0,
128-
'majorGcAmount': 0
129+
'scriptTime': 0,
130+
'pureScriptTime': 0
129131
};
132+
if (this._perfLogFeatures.gc) {
133+
result['gcTime'] = 0;
134+
result['majorGcTime'] = 0;
135+
result['gcAmount'] = 0;
136+
}
137+
if (this._perfLogFeatures.render) {
138+
result['renderTime'] = 0;
139+
}
130140

131141
var markStartEvent = null;
132142
var markEndEvent = null;
133143
var gcTimeInScript = 0;
144+
var renderTimeInScript = 0;
134145

135146
var intervalStarts = {};
136147
events.forEach( (event) => {
@@ -149,26 +160,30 @@ export class PerflogMetric extends Metric {
149160
var duration = event['ts'] - startEvent['ts'];
150161
intervalStarts[name] = null;
151162
if (StringWrapper.equals(name, 'gc')) {
152-
var amount = (startEvent['args']['usedHeapSize'] - event['args']['usedHeapSize']) / 1000;
153163
result['gcTime'] += duration;
164+
var amount = (startEvent['args']['usedHeapSize'] - event['args']['usedHeapSize']) / 1000;
154165
result['gcAmount'] += amount;
155166
var majorGc = event['args']['majorGc'];
156167
if (isPresent(majorGc) && majorGc) {
157168
result['majorGcTime'] += duration;
158-
result['majorGcAmount'] += amount;
159169
}
160170
if (isPresent(intervalStarts['script'])) {
161171
gcTimeInScript += duration;
162172
}
163-
} else if (StringWrapper.equals(name, 'script') || StringWrapper.equals(name, 'render')) {
164-
result[name] += duration;
173+
} else if (StringWrapper.equals(name, 'render')) {
174+
result['renderTime'] += duration;
175+
if (isPresent(intervalStarts['script'])) {
176+
renderTimeInScript += duration;
177+
}
178+
} else if (StringWrapper.equals(name, 'script')) {
179+
result['scriptTime'] += duration;
165180
}
166181
}
167182
}
168183
});
169-
result['script'] -= gcTimeInScript;
184+
result['pureScriptTime'] = result['scriptTime'] - gcTimeInScript - renderTimeInScript;
170185
if (this._microIterations > 0) {
171-
result['scriptMicroAvg'] = result['script'] / this._microIterations;
186+
result['microScriptTimeAvg'] = result['scriptTime'] / this._microIterations;
172187
}
173188
return isPresent(markStartEvent) && isPresent(markEndEvent) ? result : null;
174189
}
@@ -183,7 +198,8 @@ var _MARK_NAME_PREFIX = 'benchpress';
183198
var _SET_TIMEOUT = new OpaqueToken('PerflogMetric.setTimeout');
184199
var _BINDINGS = [
185200
bind(PerflogMetric).toFactory(
186-
(driverExtension, setTimeout, microIterations) => new PerflogMetric(driverExtension, setTimeout, microIterations),
201+
(driverExtension, setTimeout, microIterations) =>
202+
new PerflogMetric(driverExtension, setTimeout, microIterations),
187203
[WebDriverExtension, _SET_TIMEOUT, Options.MICRO_ITERATIONS]
188204
),
189205
bind(_SET_TIMEOUT).toValue( (fn, millis) => PromiseWrapper.setTimeout(fn, millis) ),

modules/benchpress/src/reporter/json_file_reporter.js

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,27 @@ import { bind, OpaqueToken } from 'angular2/di';
77
import { Reporter } from '../reporter';
88
import { SampleDescription } from '../sample_description';
99
import { MeasureValues } from '../measure_values';
10+
import { Options } from '../common_options';
1011

1112
/**
1213
* A reporter that writes results into a json file.
13-
* TODO(tbosch): right now we bind the `writeFile` method
14-
* in benchpres/benchpress.es6. This does not work for Dart,
15-
* find another way...
1614
*/
1715
export class JsonFileReporter extends Reporter {
18-
// TODO(tbosch): use static values when our transpiler supports them
19-
static get WRITE_FILE() { return _WRITE_FILE; }
2016
// TODO(tbosch): use static values when our transpiler supports them
2117
static get PATH() { return _PATH; }
2218
static get BINDINGS() { return _BINDINGS; }
2319

2420
_writeFile:Function;
2521
_path:string;
2622
_description:SampleDescription;
23+
_now:Function;
2724

28-
constructor(sampleDescription, path, writeFile) {
25+
constructor(sampleDescription, path, writeFile, now) {
2926
super();
3027
this._description = sampleDescription;
3128
this._path = path;
3229
this._writeFile = writeFile;
30+
this._now = now;
3331
}
3432

3533
reportMeasureValues(measureValues:MeasureValues):Promise {
@@ -42,17 +40,16 @@ export class JsonFileReporter extends Reporter {
4240
'completeSample': completeSample,
4341
'validSample': validSample
4442
});
45-
var filePath = `${this._path}/${this._description.id}_${DateWrapper.toMillis(DateWrapper.now())}.json`;
43+
var filePath = `${this._path}/${this._description.id}_${DateWrapper.toMillis(this._now())}.json`;
4644
return this._writeFile(filePath, content);
4745
}
4846
}
4947

50-
var _WRITE_FILE = new OpaqueToken('JsonFileReporter.writeFile');
5148
var _PATH = new OpaqueToken('JsonFileReporter.path');
5249
var _BINDINGS = [
5350
bind(JsonFileReporter).toFactory(
54-
(sampleDescription, path, writeFile) => new JsonFileReporter(sampleDescription, path, writeFile),
55-
[SampleDescription, _PATH, _WRITE_FILE]
51+
(sampleDescription, path, writeFile, now) => new JsonFileReporter(sampleDescription, path, writeFile, now),
52+
[SampleDescription, _PATH, Options.WRITE_FILE, Options.NOW]
5653
),
5754
bind(_PATH).toValue('.')
5855
];

modules/benchpress/src/runner.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { SampleDescription } from './sample_description';
1818
import { WebDriverAdapter } from './web_driver_adapter';
1919
import { Reporter } from './reporter';
2020
import { Metric } from './metric';
21-
import { Options } from './sample_options';
21+
import { Options } from './common_options';
2222

2323
/**
2424
* The Runner is the main entry point for executing a sample run.
@@ -56,6 +56,7 @@ export class Runner {
5656
}
5757

5858
var _DEFAULT_BINDINGS = [
59+
Options.DEFAULT_BINDINGS,
5960
Sampler.BINDINGS,
6061
ConsoleReporter.BINDINGS,
6162
RegressionSlopeValidator.BINDINGS,

modules/benchpress/src/sample_description.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { StringMapWrapper, ListWrapper, StringMap } from 'angular2/src/facade/co
22
import { bind, OpaqueToken } from 'angular2/di';
33
import { Validator } from './validator';
44
import { Metric } from './metric';
5-
import { Options } from './sample_options';
5+
import { Options } from './common_options';
66

77
/**
88
* SampleDescription merges all available descriptions about a sample
@@ -47,7 +47,5 @@ var _BINDINGS = [
4747
Metric, Options.SAMPLE_ID, Options.FORCE_GC, Options.USER_AGENT,
4848
Validator, Options.DEFAULT_DESCRIPTION, Options.SAMPLE_DESCRIPTION
4949
]
50-
),
51-
bind(Options.DEFAULT_DESCRIPTION).toValue({}),
52-
bind(Options.SAMPLE_DESCRIPTION).toValue({})
50+
)
5351
];

modules/benchpress/src/sampler.js

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { Reporter } from './reporter';
99
import { WebDriverExtension } from './web_driver_extension';
1010
import { WebDriverAdapter } from './web_driver_adapter';
1111

12-
import { Options } from './sample_options';
12+
import { Options } from './common_options';
1313
import { MeasureValues} from './measure_values';
1414

1515
/**
@@ -23,8 +23,6 @@ import { MeasureValues} from './measure_values';
2323
export class Sampler {
2424
// TODO(tbosch): use static values when our transpiler supports them
2525
static get BINDINGS() { return _BINDINGS; }
26-
// TODO(tbosch): use static values when our transpiler supports them
27-
static get TIME() { return _TIME; }
2826

2927
_driver:WebDriverAdapter;
3028
_driverExtension:WebDriverExtension;
@@ -34,14 +32,14 @@ export class Sampler {
3432
_forceGc:boolean;
3533
_prepare:Function;
3634
_execute:Function;
37-
_time:Function;
35+
_now:Function;
3836

3937
constructor({
40-
driver, driverExtension, metric, reporter, validator, forceGc, prepare, execute, time
38+
driver, driverExtension, metric, reporter, validator, forceGc, prepare, execute, now
4139
}:{
4240
driver: WebDriverAdapter,
4341
driverExtension: WebDriverExtension, metric: Metric, reporter: Reporter,
44-
validator: Validator, prepare: Function, execute: Function, time: Function
42+
validator: Validator, prepare: Function, execute: Function, now: Function
4543
}={}) {
4644
this._driver = driver;
4745
this._driverExtension = driverExtension;
@@ -51,7 +49,7 @@ export class Sampler {
5149
this._forceGc = forceGc;
5250
this._prepare = prepare;
5351
this._execute = execute;
54-
this._time = time;
52+
this._now = now;
5553
}
5654

5755
sample():Promise<SampleState> {
@@ -96,7 +94,7 @@ export class Sampler {
9694
}
9795

9896
_report(state:SampleState, metricValues:StringMap):Promise<SampleState> {
99-
var measureValues = new MeasureValues(state.completeSample.length, this._time(), metricValues);
97+
var measureValues = new MeasureValues(state.completeSample.length, this._now(), metricValues);
10098
var completeSample = ListWrapper.concat(state.completeSample, [measureValues]);
10199
var validSample = this._validator.validate(completeSample);
102100
var resultPromise = this._reporter.reportMeasureValues(measureValues);
@@ -118,11 +116,9 @@ export class SampleState {
118116
}
119117
}
120118

121-
var _TIME = new OpaqueToken('Sampler.time');
122-
123119
var _BINDINGS = [
124120
bind(Sampler).toFactory(
125-
(driver, driverExtension, metric, reporter, validator, forceGc, prepare, execute, time) => new Sampler({
121+
(driver, driverExtension, metric, reporter, validator, forceGc, prepare, execute, now) => new Sampler({
126122
driver: driver,
127123
driverExtension: driverExtension,
128124
reporter: reporter,
@@ -134,14 +130,11 @@ var _BINDINGS = [
134130
// special null object, which is expensive.
135131
prepare: prepare !== false ? prepare : null,
136132
execute: execute,
137-
time: time
133+
now: now
138134
}),
139135
[
140136
WebDriverAdapter, WebDriverExtension, Metric, Reporter, Validator,
141-
Options.FORCE_GC, Options.PREPARE, Options.EXECUTE, _TIME
137+
Options.FORCE_GC, Options.PREPARE, Options.EXECUTE, Options.NOW
142138
]
143-
),
144-
bind(Options.FORCE_GC).toValue(false),
145-
bind(Options.PREPARE).toValue(false),
146-
bind(_TIME).toValue( () => DateWrapper.now() )
139+
)
147140
];

modules/benchpress/src/validator/regression_slope_validator.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,5 @@ var _BINDINGS = [
6565
[_SAMPLE_SIZE, _METRIC]
6666
),
6767
bind(_SAMPLE_SIZE).toValue(10),
68-
bind(_METRIC).toValue('script')
68+
bind(_METRIC).toValue('scriptTime')
6969
];

0 commit comments

Comments
 (0)