Skip to content

Commit ce2d733

Browse files
committed
fix: Fix warming and configuration change issues with detectDeadCodeElimination=true
Avoid configuration changes mid-run, and move warnings to the top of the output. As previously implemented, the worker mode warnings would be issued once per benchmark, interlaced with them, and with DCE enabled wouldn't issue a warning at all. Putting the warnings in the middle of the test output is more likely to be missed by a user, lost in the noise. This change also settles the issue of which plugins we are running to the constructor, also avoiding 1) re-writing the benchmark string after creation 2) rewriting the benchmark string after warming has already supposedly completed
1 parent d678f9d commit ce2d733

File tree

1 file changed

+32
-27
lines changed

1 file changed

+32
-27
lines changed

lib/index.js

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -141,31 +141,49 @@ class Suite {
141141
}
142142

143143
this.#useWorkers = options.useWorkers || false;
144+
this.#benchmarkMode = options.benchmarkMode || "ops";
145+
validateBenchmarkMode(this.#benchmarkMode, "options.benchmarkMode");
144146

145-
// DCE detection is opt-in to avoid breaking changes
146-
const dceEnabled = options.detectDeadCodeElimination === true;
147-
if (dceEnabled) {
148-
this.#dceDetector = new DeadCodeEliminationDetectionPlugin(
149-
options.dceThreshold ? { threshold: options.dceThreshold } : {},
147+
if (this.#useWorkers && this.#benchmarkMode === "time") {
148+
console.warn(
149+
"Warning: Worker mode currently doesn't fully support 'time' benchmarkMode.",
150150
);
151151
}
152152

153-
// Plugin setup: If DCE detection is enabled, default to no plugins (allow optimization)
154-
// Otherwise, use V8NeverOptimizePlugin as the default
153+
// DCE detection is opt-in to avoid breaking changes
154+
let dceEnabled = false;
155+
156+
if (options.detectDeadCodeElimination === true) {
157+
if (this.#useWorkers) {
158+
console.warn(
159+
"Warning: Worker mode currently doesn't support detectDeadCodeElimination=true.",
160+
);
161+
} else {
162+
dceEnabled = true;
163+
}
164+
}
165+
166+
let plugins = [];
167+
155168
if (options?.plugins) {
156169
validateArray(options.plugins, "plugin");
157170
validatePlugins(options.plugins);
158-
this.#plugins = options.plugins;
159-
} else if (dceEnabled) {
171+
plugins = [...options.plugins];
172+
} else if (!dceEnabled) {
160173
// DCE detection requires optimization to be enabled, so no default plugins
161-
this.#plugins = [];
162-
} else {
163174
// Default behavior - use V8NeverOptimizePlugin
164-
this.#plugins = [new V8NeverOptimizePlugin()];
175+
plugins = [new V8NeverOptimizePlugin()];
165176
}
166177

167-
this.#benchmarkMode = options.benchmarkMode || "ops";
168-
validateBenchmarkMode(this.#benchmarkMode, "options.benchmarkMode");
178+
if (dceEnabled) {
179+
this.#dceDetector = new DeadCodeEliminationDetectionPlugin(
180+
options.dceThreshold ? { threshold: options.dceThreshold } : {},
181+
);
182+
183+
plugins.push(this.#dceDetector);
184+
}
185+
186+
this.#plugins = plugins;
169187

170188
this.#reporterOptions = options.reporterOptions || {
171189
printHeader: true,
@@ -278,14 +296,6 @@ class Suite {
278296
for (let i = 0; i < this.#benchmarks.length; ++i) {
279297
const benchmark = this.#benchmarks[i];
280298

281-
// Add DCE detector to benchmark plugins if enabled
282-
if (this.#dceDetector && this.#benchmarkMode === "ops") {
283-
const originalPlugins = benchmark.plugins;
284-
benchmark.plugins = [...benchmark.plugins, this.#dceDetector];
285-
// Regenerate function string with new plugins
286-
benchmark.fnStr = createFnString(benchmark);
287-
}
288-
289299
// Warmup is calculated to reduce noise/bias on the results
290300
const initialIterations = await getInitialIterations(benchmark);
291301
debugBench(
@@ -294,11 +304,6 @@ class Suite {
294304

295305
let result;
296306
if (this.#useWorkers) {
297-
if (this.#benchmarkMode === "time") {
298-
console.warn(
299-
"Warning: Worker mode currently doesn't fully support 'time' benchmarkMode.",
300-
);
301-
}
302307
result = await this.runWorkerBenchmark(benchmark, initialIterations);
303308
} else {
304309
result = await runBenchmark(

0 commit comments

Comments
 (0)