From 5e798c632bb5fa65ed0c67db73cc7bd1dbdd2629 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Tue, 17 Feb 2015 14:30:24 -0800 Subject: [PATCH] refactor(bench press): wrap measure values into an object with time and iteration number. Closes #689 --- modules/benchpress/benchpress.js | 1 + modules/benchpress/src/measure_values.js | 13 ++++++++ modules/benchpress/src/reporter.js | 6 ++-- .../src/reporter/console_reporter.js | 9 +++--- modules/benchpress/src/sampler.js | 30 +++++++++++------ modules/benchpress/src/validator.js | 4 ++- .../validator/regression_slope_validator.js | 5 +-- .../test/reporter/console_reporter_spec.js | 17 ++++++---- modules/benchpress/test/sampler_spec.js | 32 +++++++++++-------- .../regression_slope_validator_spec.js | 24 +++++++++----- .../test/validator/size_validator_spec.js | 17 +++++++--- 11 files changed, 106 insertions(+), 52 deletions(-) create mode 100644 modules/benchpress/src/measure_values.js diff --git a/modules/benchpress/benchpress.js b/modules/benchpress/benchpress.js index bbdf9cce3f..d2c372d666 100644 --- a/modules/benchpress/benchpress.js +++ b/modules/benchpress/benchpress.js @@ -12,5 +12,6 @@ export { PerflogMetric } from './src/metric/perflog_metric'; export { ChromeDriverExtension } from './src/webdriver/chrome_driver_extension'; export { Runner } from './src/runner'; export { Options } from './src/sample_options'; +export { MeasureValues } from './src/measure_values'; export { bind, Injector, OpaqueToken } from 'angular2/di'; diff --git a/modules/benchpress/src/measure_values.js b/modules/benchpress/src/measure_values.js new file mode 100644 index 0000000000..6e4c5a4ee1 --- /dev/null +++ b/modules/benchpress/src/measure_values.js @@ -0,0 +1,13 @@ +import { Date } from 'angular2/src/facade/lang'; + +export class MeasureValues { + timeStamp:Date; + runIndex:number; + values:any; + + constructor(runIndex:number, timeStamp:Date, values:any) { + this.timeStamp = timeStamp; + this.runIndex = runIndex; + this.values = values; + } +} diff --git a/modules/benchpress/src/reporter.js b/modules/benchpress/src/reporter.js index 72f5bba4c5..abff062ea2 100644 --- a/modules/benchpress/src/reporter.js +++ b/modules/benchpress/src/reporter.js @@ -5,16 +5,18 @@ import { ABSTRACT, BaseException } from 'angular2/src/facade/lang'; +import { MeasureValues } from './measure_values'; + /** * A reporter reports measure values and the valid sample. */ @ABSTRACT() export class Reporter { - reportMeasureValues(index:number, values:any):Promise { + reportMeasureValues(values:MeasureValues):Promise { throw new BaseException('NYI'); } - reportSample(completeSample:List, validSample:List):Promise { + reportSample(completeSample:List, validSample:List):Promise { throw new BaseException('NYI'); } } diff --git a/modules/benchpress/src/reporter/console_reporter.js b/modules/benchpress/src/reporter/console_reporter.js index d500ed1033..94cd88180b 100644 --- a/modules/benchpress/src/reporter/console_reporter.js +++ b/modules/benchpress/src/reporter/console_reporter.js @@ -7,6 +7,7 @@ import { bind, OpaqueToken } from 'angular2/di'; import { Statistic } from '../statistic'; import { Reporter } from '../reporter'; import { SampleDescription } from '../sample_description'; +import { MeasureValues } from '../measure_values'; /** * A reporter for the console @@ -72,20 +73,20 @@ export class ConsoleReporter extends Reporter { this._printStringRow(this._metricNames.map( (_) => '' ), '-'); } - reportMeasureValues(index:number, measuredValues:any):Promise { + reportMeasureValues(measureValues:MeasureValues):Promise { var formattedValues = ListWrapper.map(this._metricNames, (metricName) => { - var value = measuredValues[metricName]; + var value = measureValues.values[metricName]; return ConsoleReporter._formatNum(value); }); this._printStringRow(formattedValues); return PromiseWrapper.resolve(null); } - reportSample(completeSample:List, validSample:List):Promise { + reportSample(completeSample:List, validSample:List):Promise { this._printStringRow(this._metricNames.map( (_) => '' ), '='); this._printStringRow( ListWrapper.map(this._metricNames, (metricName) => { - var sample = ListWrapper.map(validSample, (measuredValues) => measuredValues[metricName]); + var sample = ListWrapper.map(validSample, (measureValues) => measureValues.values[metricName]); var mean = Statistic.calculateMean(sample); var cv = Statistic.calculateCoefficientOfVariation(sample, mean); return `${ConsoleReporter._formatNum(mean)}\u00B1${Math.floor(cv)}%`; diff --git a/modules/benchpress/src/sampler.js b/modules/benchpress/src/sampler.js index 286c4bed00..8454e249bd 100644 --- a/modules/benchpress/src/sampler.js +++ b/modules/benchpress/src/sampler.js @@ -1,4 +1,4 @@ -import { isPresent, isBlank } from 'angular2/src/facade/lang'; +import { isPresent, isBlank, Date, DateWrapper } from 'angular2/src/facade/lang'; import { Promise, PromiseWrapper } from 'angular2/src/facade/async'; import { StringMapWrapper, List, ListWrapper } from 'angular2/src/facade/collection'; import { bind, OpaqueToken } from 'angular2/di'; @@ -10,6 +10,7 @@ import { WebDriverExtension } from './web_driver_extension'; import { WebDriverAdapter } from './web_driver_adapter'; import { Options } from './sample_options'; +import { MeasureValues} from './measure_values'; /** * The Sampler owns the sample loop: @@ -22,6 +23,8 @@ import { Options } from './sample_options'; export class Sampler { // TODO(tbosch): use static values when our transpiler supports them static get BINDINGS() { return _BINDINGS; } + // TODO(tbosch): use static values when our transpiler supports them + static get TIME() { return _TIME; } _driver:WebDriverAdapter; _driverExtension:WebDriverExtension; @@ -31,13 +34,14 @@ export class Sampler { _forceGc:boolean; _prepare:Function; _execute:Function; + _time:Function; constructor({ - driver, driverExtension, metric, reporter, validator, forceGc, prepare, execute + driver, driverExtension, metric, reporter, validator, forceGc, prepare, execute, time }:{ driver: WebDriverAdapter, driverExtension: WebDriverExtension, metric: Metric, reporter: Reporter, - validator: Validator, prepare: Function, execute: Function + validator: Validator, prepare: Function, execute: Function, time: Function }={}) { this._driver = driver; this._driverExtension = driverExtension; @@ -47,6 +51,7 @@ export class Sampler { this._forceGc = forceGc; this._prepare = prepare; this._execute = execute; + this._time = time; } sample():Promise { @@ -90,10 +95,11 @@ export class Sampler { .then( (measureValues) => this._report(lastState, measureValues) ); } - _report(state:SampleState, measuredValues:any):Promise { - var completeSample = ListWrapper.concat(state.completeSample, [measuredValues]); + _report(state:SampleState, metricValues:any):Promise { + var measureValues = new MeasureValues(state.completeSample.length, this._time(), metricValues); + var completeSample = ListWrapper.concat(state.completeSample, [measureValues]); var validSample = this._validator.validate(completeSample); - var resultPromise = this._reporter.reportMeasureValues(completeSample.length - 1, measuredValues); + var resultPromise = this._reporter.reportMeasureValues(measureValues); if (isPresent(validSample)) { resultPromise = resultPromise.then( (_) => this._reporter.reportSample(completeSample, validSample) ) } @@ -112,9 +118,11 @@ export class SampleState { } } +var _TIME = new OpaqueToken('Sampler.time'); + var _BINDINGS = [ bind(Sampler).toFactory( - (driver, driverExtension, metric, reporter, validator, forceGc, prepare, execute) => new Sampler({ + (driver, driverExtension, metric, reporter, validator, forceGc, prepare, execute, time) => new Sampler({ driver: driver, driverExtension: driverExtension, reporter: reporter, @@ -125,10 +133,12 @@ var _BINDINGS = [ // Mostly because the cache would have to be initialized with a // special null object, which is expensive. prepare: prepare !== false ? prepare : null, - execute: execute + execute: execute, + time: time }), - [WebDriverAdapter, WebDriverExtension, Metric, Reporter, Validator, Options.FORCE_GC, Options.PREPARE, Options.EXECUTE] + [WebDriverAdapter, WebDriverExtension, Metric, Reporter, Validator, Options.FORCE_GC, Options.PREPARE, Options.EXECUTE, _TIME] ), bind(Options.FORCE_GC).toValue(false), - bind(Options.PREPARE).toValue(false) + bind(Options.PREPARE).toValue(false), + bind(_TIME).toValue( () => DateWrapper.now() ) ]; diff --git a/modules/benchpress/src/validator.js b/modules/benchpress/src/validator.js index 752552ddb8..1d0c2a313d 100644 --- a/modules/benchpress/src/validator.js +++ b/modules/benchpress/src/validator.js @@ -3,6 +3,8 @@ import { ABSTRACT, BaseException } from 'angular2/src/facade/lang'; +import { MeasureValues } from './measure_values'; + /** * A Validator calculates a valid sample out of the complete sample. * A valid sample is a sample that represents the population that should be observed @@ -13,7 +15,7 @@ export class Validator { /** * Calculates a valid sample out of the complete sample */ - validate(completeSample:List):List { + validate(completeSample:List):List { throw new BaseException('NYI'); } diff --git a/modules/benchpress/src/validator/regression_slope_validator.js b/modules/benchpress/src/validator/regression_slope_validator.js index 3a3d171aff..23bf7eca4c 100644 --- a/modules/benchpress/src/validator/regression_slope_validator.js +++ b/modules/benchpress/src/validator/regression_slope_validator.js @@ -3,6 +3,7 @@ import { bind, OpaqueToken } from 'angular2/di'; import { Validator } from '../validator'; import { Statistic } from '../statistic'; +import { MeasureValues } from '../measure_values'; /** * A validator that checks the regression slope of a specific metric. @@ -32,7 +33,7 @@ export class RegressionSlopeValidator extends Validator { }; } - validate(completeSample:List):List { + validate(completeSample:List):List { if (completeSample.length >= this._sampleSize) { var latestSample = ListWrapper.slice(completeSample, completeSample.length - this._sampleSize, completeSample.length); @@ -42,7 +43,7 @@ export class RegressionSlopeValidator extends Validator { // For now, we only use the array index as x value. // TODO(tbosch): think about whether we should use time here instead ListWrapper.push(xValues, i); - ListWrapper.push(yValues, latestSample[i][this._metric]); + ListWrapper.push(yValues, latestSample[i].values[this._metric]); } var regressionSlope = Statistic.calculateRegressionSlope( xValues, Statistic.calculateMean(xValues), diff --git a/modules/benchpress/test/reporter/console_reporter_spec.js b/modules/benchpress/test/reporter/console_reporter_spec.js index 23abbd5eac..aff49414e4 100644 --- a/modules/benchpress/test/reporter/console_reporter_spec.js +++ b/modules/benchpress/test/reporter/console_reporter_spec.js @@ -1,11 +1,11 @@ import {describe, ddescribe, it, iit, xit, expect, beforeEach, afterEach} from 'angular2/test_lib'; -import { isBlank, isPresent } from 'angular2/src/facade/lang'; +import { isBlank, isPresent, Date, DateWrapper } from 'angular2/src/facade/lang'; import { List, ListWrapper } from 'angular2/src/facade/collection'; import { SampleState, Reporter, bind, Injector, - ConsoleReporter, SampleDescription + ConsoleReporter, SampleDescription, MeasureValues } from 'benchpress/benchpress'; export function main() { @@ -68,9 +68,9 @@ export function main() { } }); log = []; - reporter.reportMeasureValues(0, { + reporter.reportMeasureValues(mv(0, 0, { 'a': 1.23, 'b': 2 - }); + })); expect(log).toEqual([ ' 1.23 | 2.00' ]); @@ -85,11 +85,11 @@ export function main() { } }); log = []; - reporter.reportSample([], [{ + reporter.reportSample([], [mv(0,0,{ 'a': 3, 'b': 6 - },{ + }), mv(1,1,{ 'a': 5, 'b': 9 - }]); + })]); expect(log).toEqual([ '======== | ========', '4.00±25% | 7.50±20%' @@ -99,3 +99,6 @@ export function main() { }); } +function mv(runIndex, time, values) { + return new MeasureValues(runIndex, DateWrapper.fromMillis(time), values); +} diff --git a/modules/benchpress/test/sampler_spec.js b/modules/benchpress/test/sampler_spec.js index cb95eac7d1..8459b691e9 100644 --- a/modules/benchpress/test/sampler_spec.js +++ b/modules/benchpress/test/sampler_spec.js @@ -1,13 +1,13 @@ import {describe, it, iit, xit, expect, beforeEach, afterEach} from 'angular2/test_lib'; -import { isBlank, isPresent, BaseException, stringify } from 'angular2/src/facade/lang'; +import { isBlank, isPresent, BaseException, stringify, Date, DateWrapper } from 'angular2/src/facade/lang'; import { ListWrapper, List } from 'angular2/src/facade/collection'; import { PromiseWrapper, Promise } from 'angular2/src/facade/async'; import { Sampler, WebDriverAdapter, WebDriverExtension, Validator, Metric, Reporter, Browser, - bind, Injector, Options + bind, Injector, Options, MeasureValues } from 'benchpress/benchpress'; export function main() { @@ -26,6 +26,7 @@ export function main() { prepare, execute } = {}) { + var time = 1000; if (isBlank(metric)) { metric = new MockMetric([]); } @@ -44,7 +45,8 @@ export function main() { bind(WebDriverAdapter).toValue(driver), bind(WebDriverExtension).toValue(driverExtension), bind(Options.EXECUTE).toValue(execute), - bind(Validator).toValue(validator) + bind(Validator).toValue(validator), + bind(Sampler.TIME).toValue( () => DateWrapper.fromMillis(time++) ) ]); if (isPresent(prepare)) { ListWrapper.push(bindings, bind(Options.PREPARE).toValue(prepare)); @@ -181,8 +183,8 @@ export function main() { }); sampler.sample().then( (state) => { expect(state.completeSample.length).toBe(2); - expect(state.completeSample[0]).toEqual({'script': 10}); - expect(state.completeSample[1]).toEqual({'script': 20}); + expect(state.completeSample[0]).toEqual(mv(0, 1000, {'script': 10})); + expect(state.completeSample[1]).toEqual(mv(1, 1001, {'script': 20})); done(); }); }); @@ -206,10 +208,10 @@ export function main() { expect(log.length).toBe(2); expect(log[0]).toEqual( - ['validate', [{'script': 0}], null] + ['validate', [mv(0, 1000, {'script': 0})], null] ); expect(log[1]).toEqual( - ['validate', [{'script': 0}, {'script': 1}], validSample] + ['validate', [mv(0, 1000, {'script': 0}), mv(1, 1001, {'script': 1})], validSample] ); done(); @@ -234,13 +236,13 @@ export function main() { // ]); expect(log.length).toBe(3); expect(log[0]).toEqual( - ['reportMeasureValues', 0, {'script': 0}] + ['reportMeasureValues', mv(0, 1000, {'script': 0})] ); expect(log[1]).toEqual( - ['reportMeasureValues', 1, {'script': 1}] + ['reportMeasureValues', mv(1, 1001, {'script': 1})] ); expect(log[2]).toEqual( - ['reportSample', [{'script': 0}, {'script': 1}], validSample] + ['reportSample', [mv(0, 1000, {'script': 0}), mv(1, 1001, {'script': 1})], validSample] ); done(); @@ -250,6 +252,10 @@ export function main() { }); } +function mv(runIndex, time, values) { + return new MeasureValues(runIndex, DateWrapper.fromMillis(time), values); +} + function createCountingValidator(count, validSample = null, log = null) { return new MockValidator(log, (completeSample) => { count--; @@ -315,7 +321,7 @@ class MockValidator extends Validator { } this._log = log; } - validate(completeSample:List):List { + validate(completeSample:List):List { var stableSample = isPresent(this._validate) ? this._validate(completeSample) : completeSample; ListWrapper.push(this._log, ['validate', completeSample, stableSample]); return stableSample; @@ -353,8 +359,8 @@ class MockReporter extends Reporter { } this._log = log; } - reportMeasureValues(index, values):Promise { - ListWrapper.push(this._log, ['reportMeasureValues', index, values]); + reportMeasureValues(values):Promise { + ListWrapper.push(this._log, ['reportMeasureValues', values]); return PromiseWrapper.resolve(null); } reportSample(completeSample, validSample):Promise { diff --git a/modules/benchpress/test/validator/regression_slope_validator_spec.js b/modules/benchpress/test/validator/regression_slope_validator_spec.js index d65db4da20..5a78f72ab3 100644 --- a/modules/benchpress/test/validator/regression_slope_validator_spec.js +++ b/modules/benchpress/test/validator/regression_slope_validator_spec.js @@ -1,7 +1,9 @@ import {describe, ddescribe, it, iit, xit, expect, beforeEach, afterEach} from 'angular2/test_lib'; +import { Date, DateWrapper } from 'angular2/src/facade/lang'; +import { ListWrapper } from 'angular2/src/facade/collection'; import { - Validator, RegressionSlopeValidator, Injector, bind + Validator, RegressionSlopeValidator, Injector, bind, MeasureValues } from 'benchpress/benchpress'; export function main() { @@ -27,25 +29,31 @@ export function main() { it('should return null while the completeSample is smaller than the given size', () => { createValidator({size: 2, metric: 'script'}); expect(validator.validate([])).toBe(null); - expect(validator.validate([{}])).toBe(null); + expect(validator.validate([mv(0,0,{})])).toBe(null); }); it('should return null while the regression slope is < 0', () => { createValidator({size: 2, metric: 'script'}); - expect(validator.validate([{'script':2}, {'script':1}])).toBe(null); + expect(validator.validate([mv(0,0,{'script':2}), mv(1,1,{'script':1})])).toBe(null); }); it('should return the last sampleSize runs when the regression slope is ==0', () => { createValidator({size: 2, metric: 'script'}); - expect(validator.validate([{'script':1}, {'script':1}])).toEqual([{'script':1}, {'script':1}]); - expect(validator.validate([{'script':1}, {'script':1}, {'script':1}])).toEqual([{'script':1}, {'script':1}]); + var sample = [mv(0,0,{'script':1}), mv(1,1,{'script':1}), mv(2,2,{'script':1})]; + expect(validator.validate(ListWrapper.slice(sample,0,2))).toEqual(ListWrapper.slice(sample,0,2)); + expect(validator.validate(sample)).toEqual(ListWrapper.slice(sample,1,3)); }); it('should return the last sampleSize runs when the regression slope is >0', () => { createValidator({size: 2, metric: 'script'}); - expect(validator.validate([{'script':1}, {'script':2}])).toEqual([{'script':1}, {'script':2}]); - expect(validator.validate([{'script':1}, {'script':2}, {'script':3}])).toEqual([{'script':2}, {'script':3}]); + var sample = [mv(0,0,{'script':1}), mv(1,1,{'script':2}), mv(2,2,{'script':3})]; + expect(validator.validate(ListWrapper.slice(sample,0,2))).toEqual(ListWrapper.slice(sample,0,2)); + expect(validator.validate(sample)).toEqual(ListWrapper.slice(sample,1,3)); }); }); -} \ No newline at end of file +} + +function mv(runIndex, time, values) { + return new MeasureValues(runIndex, DateWrapper.fromMillis(time), values); +} diff --git a/modules/benchpress/test/validator/size_validator_spec.js b/modules/benchpress/test/validator/size_validator_spec.js index 794cefde19..147addc908 100644 --- a/modules/benchpress/test/validator/size_validator_spec.js +++ b/modules/benchpress/test/validator/size_validator_spec.js @@ -1,7 +1,9 @@ import {describe, ddescribe, it, iit, xit, expect, beforeEach, afterEach} from 'angular2/test_lib'; +import { Date, DateWrapper } from 'angular2/src/facade/lang'; +import { ListWrapper } from 'angular2/src/facade/collection'; import { - Validator, SizeValidator, Injector, bind + Validator, SizeValidator, Injector, bind, MeasureValues } from 'benchpress/benchpress'; export function main() { @@ -25,14 +27,19 @@ export function main() { it('should return null while the completeSample is smaller than the given size', () => { createValidator(2); expect(validator.validate([])).toBe(null); - expect(validator.validate([{}])).toBe(null); + expect(validator.validate([mv(0,0,{})])).toBe(null); }); it('should return the last sampleSize runs when it has at least the given size', () => { createValidator(2); - expect(validator.validate([{'a':1}, {'b':2}])).toEqual([{'a':1}, {'b':2}]); - expect(validator.validate([{'a':1}, {'b':2}, {'c':3}])).toEqual([{'b':2}, {'c':3}]); + var sample = [mv(0,0,{'a':1}), mv(1,1,{'b':2}), mv(2,2,{'c':3})]; + expect(validator.validate(ListWrapper.slice(sample, 0, 2))).toEqual(ListWrapper.slice(sample, 0, 2)); + expect(validator.validate(sample)).toEqual(ListWrapper.slice(sample, 1, 3)); }); }); -} \ No newline at end of file +} + +function mv(runIndex, time, values) { + return new MeasureValues(runIndex, DateWrapper.fromMillis(time), values); +}