Skip to content

Commit 32c2814

Browse files
rubennortefacebook-github-bot
authored andcommitted
[skip ci] Run benchmarks in test mode when not specifying verification functions in CI (#48451)
Summary: Changelog: [internal] Modifies the execution of benchmarks in CI to run benchmarks in test mode when they don't define a `verify` method. If a benchmark uses `verify`, the test is meant to make sure that the benchmark doesn't regress in CI. If it doesn't, then running the benchmark on CI doesn't provide much value. In that case, we run a single iteration of each test case just to make sure things don't break over time. Reviewed By: rshest Differential Revision: D67637754
1 parent 97eb97a commit 32c2814

5 files changed

Lines changed: 78 additions & 13 deletions

File tree

packages/react-native-fantom/runner/entrypoint-template.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ module.exports = function entrypointTemplate({
1818
featureFlagsModulePath,
1919
featureFlags,
2020
snapshotConfig,
21+
isRunningFromCI,
2122
}: {
2223
testPath: string,
2324
setupModulePath: string,
2425
featureFlagsModulePath: string,
2526
featureFlags: FantomTestConfigJsOnlyFeatureFlags,
2627
snapshotConfig: SnapshotConfig,
28+
isRunningFromCI: boolean,
2729
}): string {
2830
return `/**
2931
* Copyright (c) Meta Platforms, Inc. and affiliates.
@@ -38,6 +40,7 @@ module.exports = function entrypointTemplate({
3840
*/
3941
4042
import {registerTest} from '${setupModulePath}';
43+
import {setConstants} from '@react-native/fantom';
4144
${
4245
Object.keys(featureFlags).length > 0
4346
? `import * as ReactNativeFeatureFlags from '${featureFlagsModulePath}';
@@ -50,6 +53,10 @@ ${Object.entries(featureFlags)
5053
: ''
5154
}
5255
56+
setConstants({
57+
isRunningFromCI: ${String(isRunningFromCI)},
58+
});
59+
5360
registerTest(() => require('${testPath}'), ${JSON.stringify(snapshotConfig)});
5461
`;
5562
};

packages/react-native-fantom/runner/runner.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
getBuckModesForPlatform,
2525
getDebugInfoFromCommandResult,
2626
getShortHash,
27+
isRunningFromCI,
2728
printConsoleLog,
2829
runBuck2,
2930
runBuck2Sync,
@@ -198,6 +199,7 @@ module.exports = async function runTest(
198199
updateSnapshot: snapshotState._updateSnapshot,
199200
data: getInitialSnapshotData(snapshotState),
200201
},
202+
isRunningFromCI: isRunningFromCI(),
201203
});
202204

203205
const entrypointPath = path.join(

packages/react-native-fantom/runner/utils.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,16 @@ export type SyncCommandResult = {
6767
stderr: string,
6868
};
6969

70+
function isEmpty(value: ?string): boolean {
71+
return value == null || value === '';
72+
}
73+
74+
export function isRunningFromCI(): boolean {
75+
return (
76+
!isEmpty(process.env.SANDCASTLE) || !isEmpty(process.env.GITHUB_ACTIONS)
77+
);
78+
}
79+
7080
function maybeLogCommand(command: string, args: Array<string>): void {
7181
if (EnvironmentOptions.logCommands) {
7282
console.log(`RUNNING \`${command} ${args.join(' ')}\``);

packages/react-native-fantom/src/Benchmark.js

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* @format
99
*/
1010

11+
import {getConstants} from './index';
1112
import nullthrows from 'nullthrows';
1213
import NativeCPUTime from 'react-native/src/private/specs/modules/NativeCPUTime';
1314
import {
@@ -36,31 +37,60 @@ export function suite(
3637
suiteName: string,
3738
suiteOptions?: ?SuiteOptions,
3839
): SuiteAPI {
39-
const {disableOptimizedBuildCheck, ...benchOptions} = suiteOptions ?? {};
40-
41-
const bench = new Bench({
42-
...benchOptions,
43-
name: suiteName,
44-
throws: true,
45-
now: () => NativeCPUTime.getCPUTimeNanos() / 1000000,
46-
});
47-
40+
const tasks: Array<{
41+
name: string,
42+
fn: () => void,
43+
options: FnOptions | void,
44+
}> = [];
4845
const verifyFns = [];
4946

5047
global.it(suiteName, () => {
51-
if (bench.tasks.length === 0) {
48+
if (tasks.length === 0) {
5249
throw new Error('No benchmark tests defined');
5350
}
5451

52+
const {isRunningFromCI} = getConstants();
53+
54+
// If we're running from CI and there's no verification function, there's
55+
// no point in running the benchmark.
56+
// We still run a single iteration of each test just to make sure that the
57+
// logic in the benchmark doesn't break.
58+
const isTestOnly = isRunningFromCI && verifyFns.length === 0;
59+
60+
const overriddenOptions: BenchOptions = isTestOnly
61+
? {
62+
warmupIterations: 1,
63+
warmupTime: 0,
64+
iterations: 1,
65+
time: 0,
66+
}
67+
: {};
68+
69+
const {disableOptimizedBuildCheck, ...benchOptions} = suiteOptions ?? {};
70+
71+
const bench = new Bench({
72+
...benchOptions,
73+
...overriddenOptions,
74+
name: suiteName,
75+
throws: true,
76+
now: () => NativeCPUTime.getCPUTimeNanos() / 1000000,
77+
});
78+
79+
for (const task of tasks) {
80+
bench.add(task.name, task.fn, task.options);
81+
}
82+
5583
bench.runSync();
5684

57-
printBenchmarkResults(bench);
85+
if (!isTestOnly) {
86+
printBenchmarkResults(bench);
87+
}
5888

5989
for (const verify of verifyFns) {
6090
verify(bench.results);
6191
}
6292

63-
if (!NativeCPUTime.hasAccurateCPUTimeNanosForBenchmarks()) {
93+
if (!isTestOnly && !NativeCPUTime.hasAccurateCPUTimeNanosForBenchmarks()) {
6494
throw new Error(
6595
'`NativeCPUTime` module does not provide accurate CPU time information in this environment. Please run the benchmarks in an environment where it does.',
6696
);
@@ -73,7 +103,7 @@ export function suite(
73103

74104
const suiteAPI = {
75105
add(name: string, fn: () => void, options?: FnOptions): SuiteAPI {
76-
bench.add(name, fn, options);
106+
tasks.push({name, fn, options});
77107
return suiteAPI;
78108
},
79109
verify(fn: (results: SuiteResults) => void): SuiteAPI {

packages/react-native-fantom/src/index.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,22 @@ export function createRoot(rootConfig?: RootConfig): Root {
141141

142142
export const benchmark = Benchmark;
143143

144+
type FantomConstants = $ReadOnly<{
145+
isRunningFromCI: boolean,
146+
}>;
147+
148+
let constants: FantomConstants = {
149+
isRunningFromCI: false,
150+
};
151+
152+
export function getConstants(): FantomConstants {
153+
return constants;
154+
}
155+
156+
export function setConstants(newConstants: FantomConstants): void {
157+
constants = newConstants;
158+
}
159+
144160
/**
145161
* Quick and dirty polyfills required by tinybench.
146162
*/

0 commit comments

Comments
 (0)