Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename horizons to auto-sample-conditions #225

Merged
merged 1 commit into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

- `--horizons` flag and `horizon` config setting has been replaced with the
`--auto-sample-conditions` and `autoSampleConditions`. `--horizon` will
continue to work for backwards compatibility, but please do update to the new
name.

- Copyright notice owner changed from "The Polymer Project Authors" to "Google
LLC". Trivial reformatting for `LICENSE` file to match spdx.org version.
Source license headers replaced with concise SPDX-style.
Expand Down
67 changes: 35 additions & 32 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ By default, a **minimum of 50 samples** are taken from **each** benchmark. You
can change the minimum sample size with the `--sample-size` flag or the
`sampleSize` JSON config option.

### Auto sampling
### Auto sample

After the initial 50 samples, tachometer will continue taking samples until
there is a clear statistically significant difference between all benchmarks,
Expand All @@ -130,33 +130,36 @@ You can change this duration with the `--timeout` flag or the `timeout` JSON
config option, measured in minutes. Set `--timeout=0` to disable auto sampling
entirely. Set `--timeout=60` to sample for up to an hour.

### Horizons
### Auto sample conditions
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe both? "Auto sample stopping conditions" - unless conditions can do more than just stop? Also no action required.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd like to be 100% consistent with the phrase "auto sample conditions".


You can also configure which statistical conditions tachometer should check for
when deciding when to stop auto sampling by configuring _horizons_.
when deciding when to stop auto sampling by configuring _auto sample
conditions_.

To set horizons from the command-line, use the `--horizon` flag with a
comma-delimited list:
To set auto sample conditions from the command-line, use the
`--auto-sample-conditions` flag with a comma-delimited list:

```sh
--horizon=0%,10%
--auto-sample-conditions=0%,10%
```

To set horizons from a JSON config file, use the `horizons` property with an
array of strings (including if there is only one condition):
To set auto sample conditions from a JSON config file, use the
`autoSampleConditions` property with an array of strings (including if there is
only one condition):

```json
{
"horizons": ["0%", "10%"]
"autoSampleConditions": ["0%", "10%"]
}
```

A horizon can be thought of as a point of interest on the number-line of either
absolute milliseconds, or relative percent change. By setting a horizon, you are
asking tachometer to try to shrink the confidence interval until it is
unambiguously placed on one side or the other of that horizon.
An auto sample condition can be thought of as a point of interest on the
number-line of either absolute milliseconds, or relative percent change. By
setting a condition, you are asking tachometer to try to shrink the confidence
interval until it is unambiguously placed on one side or the other of that
condition.

| Example horizon | Question |
| Example condition | Question |
| ------------------- | ---------------------------------------------------------- |
| `0%` | Is A faster or slower than B _at all_? (The **default**) |
| `10%` | Is A faster or slower than B by at least 10%? |
Expand All @@ -166,24 +169,24 @@ unambiguously placed on one side or the other of that horizon.
| `0%`, `10%`, `100%` | Is A at all, a little, or a lot slower or faster than B? |
| `0.5ms` | Is A faster or slower than B by at least 0.5 milliseconds? |

In the following example, we have set `--horizon=10%`, meaning we are interested
in knowing whether A differs from B by at least 10% in either direction. The
sample size automatically increases until the confidence interval is narrow
enough to place the estimated difference squarely on one side or the other of
both horizons.
In the following example, we have set `--auto-sample-conditions=10%`, meaning we
are interested in knowing whether A differs from B by at least 10% in either
direction. The sample size automatically increases until the confidence interval
is narrow enough to place the estimated difference squarely on one side or the
other of both conditions.

```
<-------------------------------> n=50 -10% +10%
<------------------> n=100 ✔️ -10% +10%
<-------------------------------> n=50 X -10% X +10%
<------------------> n=100 ✔️ -10% X +10%
<-----> n=200 ✔️ -10% ✔️ +10%

|---------|---------|---------|---------| difference in runtime
-20% -10% 0 +10% +20%

n = sample size
<---> = confidence interval for percent difference of mean runtimes
✔️ = resolved horizon
= unresolved horizon
n = sample size
<--> = confidence interval for percent difference of mean runtimes
✔️ = resolved condition
X = unresolved condition
```

In this example, by `n=50` we are not sure whether A is faster or slower than B
Expand All @@ -192,10 +195,10 @@ than 10%, but we're still not sure if it's _slower_ by more than 10%. By `n=200`
we have also ruled out that B is slower than A by more than 10%, so we stop
sampling. Note that we still don't know which is _absolutely_ faster, we just
know that whatever the difference is, it is neither faster nor slower than 10%
(and if we did want to know, we could add `0` to our horizons).
(and if we did want to know, we could add `0` to our conditions).

Note that, if the _actual_ difference is very close to a horizon, then it is
likely that the horizon will never be met, and the timeout will expire.
Note that, if the _actual_ difference is very close to a condition, then it is
likely that the condition will never be met, and the timeout will expire.

## Measurement modes

Expand Down Expand Up @@ -705,7 +708,7 @@ Defaults are the same as the corresponding command-line flags.
"root": "./benchmarks",
"sampleSize": 50,
"timeout": 3,
"horizons": ["0%", "1%"],
"autoSampleConditions": ["0%", "1%"],
"benchmarks": [
{
"name": "foo",
Expand Down Expand Up @@ -806,9 +809,9 @@ tach http://example.com
| `--package-version` / `-p` | _(none)_ | Specify an NPM package version to swap in ([details](#swap-npm-dependencies)) |
| `--browser` / `-b` | `chrome` | Which browsers to launch in automatic mode, comma-delimited (chrome, firefox, safari, edge, ie) ([details](#browsers)) |
| `--window-size` | `1024,768` | "width,height" in pixels of the browser windows that will be created |
| `--sample-size` / `-n` | `50` | Minimum number of times to run each benchmark ([details](#sample-size)] |
| `--horizon` | `0%` | The degrees of difference to try and resolve when auto-sampling ("N%" or "Nms", comma-delimited) ([details](#auto-sampling)) |
| `--timeout` | `3` | The maximum number of minutes to spend auto-sampling ([details](#auto-sampling)) |
| `--sample-size` / `-n` | `50` | Minimum number of times to run each benchmark ([details](#sample-size)) |
| `--auto-sample-conditions` | `0%` | The degrees of difference to try and resolve when auto-sampling ("N%" or "Nms", comma-delimited) ([details](#auto-sample-conditions)) |
| `--timeout` | `3` | The maximum number of minutes to spend auto-sampling ([details](#auto-sample)) |
| `--measure` | `callback` | Which time interval to measure (`callback`, `global`, `fcp`) ([details](#measurement-modes)) |
| `--measurement-expression` | `window.tachometerResult` | JS expression to poll for on page to retrieve measurement result when `measure` setting is set to `global` |
| `--remote-accessible-host` | matches `--host` | When using a browser over a remote WebDriver connection, the URL that those browsers should use to access the local tachometer server ([details](#remote-control)) |
Expand Down
9 changes: 8 additions & 1 deletion config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,13 @@
"description": "An optional reference to the JSON Schema for this file.\n\nIf none is given, and the file is a valid tachometer config file,\ntachometer will write back to the config file to give this a value.",
"type": "string"
},
"autoSampleConditions": {
"description": "The degrees of difference to try and resolve when auto-sampling\n(e.g. 0ms, +1ms, -1ms, 0%, +1%, -1%, default 0%).",
"items": {
"type": "string"
},
"type": "array"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 Very helpful!

},
"benchmarks": {
"description": "Benchmarks to run.",
"items": {
Expand All @@ -462,7 +469,7 @@
"type": "array"
},
"horizons": {
"description": "The degrees of difference to try and resolve when auto-sampling\n(e.g. 0ms, +1ms, -1ms, 0%, +1%, -1%, default 0%).",
"description": "Deprecated alias for autoSampleConditions.",
"items": {
"type": "string"
},
Expand Down
38 changes: 22 additions & 16 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as defaults from './defaults';
import {Opts} from './flags';
import {CheckConfig, parseGithubCheckFlag} from './github';
import {specsFromOpts} from './specs';
import {Horizons} from './stats';
import {AutoSampleConditions} from './stats';
import {BenchmarkSpec} from './types';
import {fileKind} from './util';

Expand All @@ -25,7 +25,7 @@ export interface Config {
sampleSize: number;
timeout: number;
benchmarks: BenchmarkSpec[];
horizons: Horizons;
autoSampleConditions: AutoSampleConditions;
mode: 'automatic' | 'manual';
jsonFile: string;
// TODO(aomarks) Remove this in next major version.
Expand Down Expand Up @@ -69,8 +69,10 @@ export async function makeConfig(opts: Opts): Promise<Config> {
if (opts.timeout !== undefined) {
throw new Error('--timeout cannot be specified when using --config');
}
if (opts.horizon !== undefined) {
throw new Error('--horizon cannot be specified when using --config');
if (opts['auto-sample-conditions'] !== undefined) {
throw new Error(
'--auto-sample-conditions cannot be specified when using --config'
);
}
if (opts.measure !== undefined) {
throw new Error('--measure cannot be specified when using --config');
Expand Down Expand Up @@ -98,9 +100,9 @@ export async function makeConfig(opts: Opts): Promise<Config> {
root: opts.root,
sampleSize: opts['sample-size'],
timeout: opts.timeout,
horizons:
opts.horizon !== undefined
? parseHorizons(opts.horizon.split(','))
autoSampleConditions:
opts['auto-sample-conditions'] !== undefined
? parseAutoSampleConditions(opts['auto-sample-conditions'].split(','))
: undefined,
benchmarks: await specsFromOpts(opts),
resolveBareModules: opts['resolve-bare-modules'],
Expand Down Expand Up @@ -148,10 +150,10 @@ export function applyDefaults(partial: Partial<Config>): Config {
? partial.forceCleanNpmInstall
: defaults.forceCleanNpmInstall,
githubCheck: partial.githubCheck,
horizons:
partial.horizons !== undefined
? partial.horizons
: parseHorizons([...defaults.horizons]),
autoSampleConditions:
partial.autoSampleConditions !== undefined
? partial.autoSampleConditions
: parseAutoSampleConditions([...defaults.autoSampleConditions]),
jsonFile: partial.jsonFile !== undefined ? partial.jsonFile : '',
legacyJsonFile:
partial.legacyJsonFile !== undefined ? partial.legacyJsonFile : '',
Expand Down Expand Up @@ -209,13 +211,17 @@ export async function urlFromLocalPath(
return urlPath;
}

/** Parse horizon flags into signed horizon values. */
export function parseHorizons(strs: string[]): Horizons {
/**
* Parse auto sample condition strings.
*/
export function parseAutoSampleConditions(
strs: string[]
): AutoSampleConditions {
const absolute = new Set<number>();
const relative = new Set<number>();
for (const str of strs) {
if (!str.match(/^[-+]?(\d*\.)?\d+(ms|%)$/)) {
throw new Error(`Invalid horizon ${str}`);
throw new Error(`Invalid auto sample condition ${str}`);
}

let num;
Expand All @@ -232,10 +238,10 @@ export function parseHorizons(strs: string[]): Horizons {

if (str.startsWith('+') || str.startsWith('-') || num === 0) {
// If the sign was explicit (e.g. "+0.1", "-0.1") then we're only
// interested in that signed horizon.
// interested in that signed condition.
absOrRel.add(num);
} else {
// Otherwise (e.g. "0.1") we're interested in the horizon as a
// Otherwise (e.g. "0.1") we're interested in the condition as a
// difference in either direction.
absOrRel.add(-num);
absOrRel.add(num);
Expand Down
26 changes: 22 additions & 4 deletions src/configfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
parseBrowserConfigString,
validateBrowserConfig,
} from './browser';
import {Config, parseHorizons, urlFromLocalPath} from './config';
import {Config, parseAutoSampleConditions, urlFromLocalPath} from './config';
import * as defaults from './defaults';
import {makeUniqueSpecLabelFn} from './format';
import {
Expand Down Expand Up @@ -53,6 +53,11 @@ export interface ConfigFile {
* The degrees of difference to try and resolve when auto-sampling
* (e.g. 0ms, +1ms, -1ms, 0%, +1%, -1%, default 0%).
*/
autoSampleConditions?: string[];

/**
* Deprecated alias for autoSampleConditions.
*/
horizons?: string[];

/**
Expand Down Expand Up @@ -342,13 +347,26 @@ export async function parseConfigFile(
}
}

if (validated.horizons !== undefined) {
if (validated.autoSampleConditions !== undefined) {
throw new Error(
'Please use only "autoSampleConditions" and not "horizons".'
);
}
console.warn(
'\nNOTE: The "horizons" setting has been renamed to "autoSampleConditions".\n' +
'Please rename it.\n'
);
validated.autoSampleConditions = validated.horizons;
}

return {
root,
sampleSize: validated.sampleSize,
timeout: validated.timeout,
horizons:
validated.horizons !== undefined
? parseHorizons(validated.horizons)
autoSampleConditions:
validated.autoSampleConditions !== undefined
? parseAutoSampleConditions(validated.autoSampleConditions)
: undefined,
benchmarks,
resolveBareModules: validated.resolveBareModules,
Expand Down
2 changes: 1 addition & 1 deletion src/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const browserName: BrowserName = 'chrome';
export const headless = false;
export const sampleSize = 50;
export const timeout = 3;
export const horizons = ['0%'] as const;
export const autoSampleConditions = ['0%'] as const;
export const mode = 'automatic';
export const resolveBareModules = true;
export const forceCleanNpmInstall = false;
Expand Down
35 changes: 30 additions & 5 deletions src/flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,16 @@ export const optDefs: commandLineUsage.OptionDefinition[] = [
defaultValue: defaults.measurementExpression,
},
{
name: 'horizon',
name: 'auto-sample-conditions',
description:
'The degrees of difference to try and resolve when auto-sampling ' +
'(milliseconds, comma-delimited, optionally signed, ' +
`default ${defaults.horizons.join(',')})`,
`default ${defaults.autoSampleConditions.join(',')})`,
type: String,
},
{
name: 'horizon',
description: 'Deprecated alias for --auto-sample-conditions',
type: String,
},
{
Expand Down Expand Up @@ -241,7 +246,7 @@ export interface Opts {
save: string;
measure: CommandLineMeasurements | undefined;
'measurement-expression': string | undefined;
horizon: string | undefined;
'auto-sample-conditions': string | undefined;
timeout: number | undefined;
'github-check': string;
'resolve-bare-modules': boolean | undefined;
Expand All @@ -265,6 +270,10 @@ export interface Opts {
_unknown?: string[];
}

interface OptsWithDeprecated extends Opts {
horizon: string | undefined;
}

/**
* Boolean flags that default to true are not supported
* (https://github.com/75lb/command-line-args/issues/71).
Expand All @@ -286,7 +295,10 @@ function booleanString(flagName: string): (str: string) => boolean {
* Parse the given CLI argument list.
*/
export function parseFlags(argv: string[]): Opts {
const opts = commandLineArgs(optDefs, {partial: true, argv}) as Opts;
const opts = commandLineArgs(optDefs, {
partial: true,
argv,
}) as OptsWithDeprecated;
// Note that when a flag is used but not set to a value (i.e. "tachometer
// --resolve-bare-modules ..."), then the type function is not invoked, and
// the value will be null. Since in default-false cases (which aren't
Expand All @@ -295,5 +307,18 @@ export function parseFlags(argv: string[]): Opts {
if (opts['resolve-bare-modules'] === null) {
opts['resolve-bare-modules'] = true;
}
return opts;
if (opts['horizon']) {
if (opts['auto-sample-conditions']) {
throw new Error(
'Please use only --auto-sample-conditions and not --horizons.'
);
}
console.warn(
'\nNOTE: The --horizon flag has been renamed to --auto-sample-conditions.\n' +
'Please use --auto-sample-conditions going forward.\n'
);
opts['auto-sample-conditions'] = opts['horizon'];
delete opts['horizon'];
}
return opts as Opts;
}
Loading