Skip to content

Commit 1297a9b

Browse files
jerome-benoitjerome-benoit
authored
refactor!: remove *Concurrently methods (#135)
Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
1 parent 581c223 commit 1297a9b

File tree

5 files changed

+99
-149
lines changed

5 files changed

+99
-149
lines changed

‎README.md‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,7 @@ export type Hook = (task: Task, mode: 'warmup' | 'run') => void | Promise<void>;
131131
```
132132

133133
- `async run()`: run the added tasks that were registered using the `add` method
134-
- `async runConcurrently(threshold: number = Infinity, mode: "bench" | "task" = "bench")`: similar to the `run` method but runs concurrently rather than sequentially. See the [Concurrency](#Concurrency) section.
135134
- `async warmup()`: warmup the benchmark tasks
136-
- `async warmupConcurrently(threshold: number = Infinity, mode: "bench" | "task" = "bench")`: warmup the benchmark tasks concurrently
137135
- `reset()`: reset each task and remove its result
138136
- `add(name: string, fn: Fn, opts?: FnOpts)`: add a benchmark task to the task map
139137
- `Fn`: `() => any | Promise<any>`
@@ -367,15 +365,10 @@ It may make your benchmarks slower, check #42.
367365
- When `mode` is set to 'bench', different tasks within the bench run concurrently. Concurrent cycles.
368366

369367
```ts
370-
// options way (recommended)
371-
bench.threshold = 10; // The maximum number of concurrent tasks to run. Defaults to Infinity.
368+
bench.threshold = 10; // The maximum number of concurrent tasks to run. Defaults to Number.POSITIVE_INFINITY.
372369
bench.concurrency = 'task'; // The concurrency mode to determine how tasks are run.
373-
// await bench.warmup();
370+
await bench.warmup();
374371
await bench.run();
375-
376-
// standalone method way
377-
// await bench.warmupConcurrently(10, 'task');
378-
await bench.runConcurrently(10, 'task'); // with runConcurrently, mode is set to 'bench' by default
379372
```
380373

381374
## Prior art

‎src/bench.ts‎

Lines changed: 28 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ export default class Bench extends EventTarget {
3030
*
3131
* - When `mode` is set to `null` (default), concurrency is disabled.
3232
* - When `mode` is set to 'task', each task's iterations (calls of a task function) run concurrently.
33-
* - When `mode` is set to 'bench', different tasks within the bench run concurrently.
33+
* - When `mode` is set to 'bench', different tasks within the bench run concurrently. Concurrent cycles.
3434
*/
3535
concurrency: 'task' | 'bench' | null = null;
3636

3737
/**
38-
* The maximum number of concurrent tasks to run. Defaults to Infinity.
38+
* The maximum number of concurrent tasks to run @default Number.POSITIVE_INFINITY
3939
*/
4040
threshold = Number.POSITIVE_INFINITY;
4141

@@ -88,105 +88,56 @@ export default class Bench extends EventTarget {
8888
}
8989

9090
/**
91-
* run the added tasks that were registered using the
92-
* {@link add} method.
91+
* run the added tasks that were registered using the {@link add} method.
9392
* Note: This method does not do any warmup. Call {@link warmup} for that.
9493
*/
9594
async run(): Promise<Task[]> {
96-
if (this.concurrency === 'bench') {
97-
// TODO: in the next major, we should remove runConcurrently
98-
return this.runConcurrently(this.threshold, this.concurrency);
99-
}
95+
let values: Task[] = [];
10096
this.dispatchEvent(createBenchEvent('start'));
101-
const values: Task[] = [];
102-
for (const task of [...this._tasks.values()]) {
103-
values.push(await this.runTask(task));
97+
if (this.concurrency === 'bench') {
98+
const limit = pLimit(this.threshold);
99+
const promises: Promise<Task>[] = [];
100+
for (const task of this._tasks.values()) {
101+
promises.push(limit(() => this.runTask(task)));
102+
}
103+
values = await Promise.all(promises);
104+
} else {
105+
for (const task of this._tasks.values()) {
106+
values.push(await this.runTask(task));
107+
}
104108
}
105109
this.dispatchEvent(createBenchEvent('complete'));
106110
return values;
107111
}
108112

109-
/**
110-
* See Bench.{@link concurrency}
111-
*/
112-
async runConcurrently(
113-
threshold = Number.POSITIVE_INFINITY,
114-
mode: NonNullable<Bench['concurrency']> = 'bench',
115-
): Promise<Task[]> {
116-
this.threshold = threshold;
117-
this.concurrency = mode;
118-
119-
if (mode === 'task') {
120-
return this.run();
121-
}
122-
123-
this.dispatchEvent(createBenchEvent('start'));
124-
125-
const limit = pLimit(threshold);
126-
127-
const promises: Promise<Task>[] = [];
128-
for (const task of [...this._tasks.values()]) {
129-
promises.push(limit(() => this.runTask(task)));
130-
}
131-
132-
const values = await Promise.all(promises);
133-
134-
this.dispatchEvent(createBenchEvent('complete'));
135-
136-
return values;
137-
}
138-
139113
/**
140114
* warmup the benchmark tasks.
141115
* This is not run by default by the {@link run} method.
142116
*/
143117
async warmup(): Promise<void> {
144-
if (this.concurrency === 'bench') {
145-
// TODO: in the next major, we should remove *Concurrently methods
146-
await this.warmupConcurrently(this.threshold, this.concurrency);
147-
return;
148-
}
149118
this.dispatchEvent(createBenchEvent('warmup'));
150-
for (const [, task] of this._tasks) {
151-
await task.warmup();
152-
}
153-
}
154-
155-
/**
156-
* warmup the benchmark tasks concurrently.
157-
* This is not run by default by the {@link runConcurrently} method.
158-
*/
159-
async warmupConcurrently(
160-
threshold = Number.POSITIVE_INFINITY,
161-
mode: NonNullable<Bench['concurrency']> = 'bench',
162-
): Promise<void> {
163-
this.threshold = threshold;
164-
this.concurrency = mode;
165-
166-
if (mode === 'task') {
167-
await this.warmup();
168-
return;
169-
}
170-
171-
this.dispatchEvent(createBenchEvent('warmup'));
172-
const limit = pLimit(threshold);
173-
const promises: Promise<void>[] = [];
174-
175-
for (const [, task] of this._tasks) {
176-
promises.push(limit(() => task.warmup()));
119+
if (this.concurrency === 'bench') {
120+
const limit = pLimit(this.threshold);
121+
const promises: Promise<void>[] = [];
122+
for (const task of this._tasks.values()) {
123+
promises.push(limit(() => task.warmup()));
124+
}
125+
await Promise.all(promises);
126+
} else {
127+
for (const task of this._tasks.values()) {
128+
await task.warmup();
129+
}
177130
}
178-
179-
await Promise.all(promises);
180131
}
181132

182133
/**
183134
* reset each task and remove its result
184135
*/
185136
reset() {
186137
this.dispatchEvent(createBenchEvent('reset'));
187-
this._tasks.forEach((task) => {
138+
for (const task of this._tasks.values()) {
188139
task.reset();
189-
});
140+
}
190141
}
191142

192143
/**

‎src/task.ts‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ export default class Task extends EventTarget {
3939
fn: Fn;
4040

4141
/*
42-
* The number of times the task
43-
* function has been executed
42+
* The number of times the task function has been executed
4443
*/
4544
runs = 0;
4645

@@ -60,15 +59,13 @@ export default class Task extends EventTarget {
6059
this.name = name;
6160
this.fn = fn;
6261
this.opts = opts;
63-
// TODO: support signals in Tasks
62+
// TODO: support signal in Tasks
6463
}
6564

6665
private async loop(
6766
time: number,
6867
iterations: number,
6968
): Promise<{ error?: unknown; samples?: number[] }> {
70-
const concurrent = this.bench.concurrency === 'task';
71-
const { threshold } = this.bench;
7269
let totalTime = 0; // ms
7370
const samples: number[] = [];
7471
if (this.opts.beforeAll != null) {
@@ -105,22 +102,21 @@ export default class Task extends EventTarget {
105102
await this.opts.afterEach.call(this);
106103
}
107104
};
108-
109-
const limit = pLimit(threshold);
110105
try {
106+
const limit = pLimit(this.bench.threshold); // only for task level concurrency
111107
const promises: Promise<void>[] = []; // only for task level concurrency
112108
while (
113109
(totalTime < time
114110
|| samples.length + limit.activeCount + limit.pendingCount < iterations)
115111
&& !this.bench.signal?.aborted
116112
) {
117-
if (concurrent) {
113+
if (this.bench.concurrency === 'task') {
118114
promises.push(limit(executeTask));
119115
} else {
120116
await executeTask();
121117
}
122118
}
123-
if (promises.length) {
119+
if (promises.length > 0) {
124120
await Promise.all(promises);
125121
}
126122
} catch (error) {
@@ -138,7 +134,7 @@ export default class Task extends EventTarget {
138134
}
139135

140136
/**
141-
* run the current task and write the results in `Task.result` object
137+
* run the current task and write the results in `Task.result` object property
142138
*/
143139
async run() {
144140
if (this.result?.error) {
@@ -272,8 +268,7 @@ export default class Task extends EventTarget {
272268
}
273269

274270
/**
275-
* reset the task to make the `Task.runs` a zero-value and remove the `Task.result`
276-
* object
271+
* reset the task to make the `Task.runs` a zero-value and remove the `Task.result` object property
277272
*/
278273
reset() {
279274
this.dispatchEvent(createBenchEvent('reset', this));

‎test/index.test.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ test('events order', async () => {
154154
expect(abortTask.result).toBeUndefined();
155155
}, 10000);
156156

157-
test('events order 2', async () => {
157+
test('events order at task completion', async () => {
158158
const bench = new Bench({
159159
warmupIterations: 0,
160160
warmupTime: 0,
@@ -174,17 +174,19 @@ test('events order 2', async () => {
174174
const barTask = bench.getTask('bar')!;
175175
fooTask.addEventListener('complete', () => {
176176
events.push('foo-complete');
177-
expect(events).not.toContain('bar-complete');
177+
expect(events).toStrictEqual(['foo-complete']);
178178
});
179179

180180
barTask.addEventListener('complete', () => {
181181
events.push('bar-complete');
182-
expect(events).toContain('foo-complete');
182+
expect(events).toStrictEqual(['foo-complete', 'bar-complete']);
183183
});
184184

185-
await bench.run();
185+
const tasks = await bench.run();
186186

187-
await new Promise((resolve) => setTimeout(resolve, 150));
187+
expect(tasks.length).toBe(2);
188+
expect(tasks[0]?.name).toBe('foo');
189+
expect(tasks[1]?.name).toBe('bar');
188190
});
189191

190192
test('error event', async () => {

0 commit comments

Comments
 (0)