From db22a74e27941726a909b5533197ef89fba4918b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Mon, 6 Jan 2025 02:54:04 -0800 Subject: [PATCH] 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 --- .../runner/entrypoint-template.js | 7 +++ packages/react-native-fantom/runner/runner.js | 2 + packages/react-native-fantom/runner/utils.js | 10 ++++ packages/react-native-fantom/src/Benchmark.js | 56 ++++++++++++++----- packages/react-native-fantom/src/index.js | 16 ++++++ 5 files changed, 78 insertions(+), 13 deletions(-) diff --git a/packages/react-native-fantom/runner/entrypoint-template.js b/packages/react-native-fantom/runner/entrypoint-template.js index c52c66209e0e8a..dc99d9f0551d84 100644 --- a/packages/react-native-fantom/runner/entrypoint-template.js +++ b/packages/react-native-fantom/runner/entrypoint-template.js @@ -18,12 +18,14 @@ module.exports = function entrypointTemplate({ featureFlagsModulePath, featureFlags, snapshotConfig, + isRunningFromCI, }: { testPath: string, setupModulePath: string, featureFlagsModulePath: string, featureFlags: FantomTestConfigJsOnlyFeatureFlags, snapshotConfig: SnapshotConfig, + isRunningFromCI: boolean, }): string { return `/** * Copyright (c) Meta Platforms, Inc. and affiliates. @@ -38,6 +40,7 @@ module.exports = function entrypointTemplate({ */ import {registerTest} from '${setupModulePath}'; +import {setConstants} from '@react-native/fantom'; ${ Object.keys(featureFlags).length > 0 ? `import * as ReactNativeFeatureFlags from '${featureFlagsModulePath}'; @@ -50,6 +53,10 @@ ${Object.entries(featureFlags) : '' } +setConstants({ + isRunningFromCI: ${String(isRunningFromCI)}, +}); + registerTest(() => require('${testPath}'), ${JSON.stringify(snapshotConfig)}); `; }; diff --git a/packages/react-native-fantom/runner/runner.js b/packages/react-native-fantom/runner/runner.js index 3c270350443021..b6580745ea394a 100644 --- a/packages/react-native-fantom/runner/runner.js +++ b/packages/react-native-fantom/runner/runner.js @@ -24,6 +24,7 @@ import { getBuckModesForPlatform, getDebugInfoFromCommandResult, getShortHash, + isRunningFromCI, printConsoleLog, runBuck2, runBuck2Sync, @@ -198,6 +199,7 @@ module.exports = async function runTest( updateSnapshot: snapshotState._updateSnapshot, data: getInitialSnapshotData(snapshotState), }, + isRunningFromCI: isRunningFromCI(), }); const entrypointPath = path.join( diff --git a/packages/react-native-fantom/runner/utils.js b/packages/react-native-fantom/runner/utils.js index 93873737d3f0d6..4ea73e21c51ac7 100644 --- a/packages/react-native-fantom/runner/utils.js +++ b/packages/react-native-fantom/runner/utils.js @@ -67,6 +67,16 @@ export type SyncCommandResult = { stderr: string, }; +function isEmpty(value: ?string): boolean { + return value == null || value === ''; +} + +export function isRunningFromCI(): boolean { + return ( + !isEmpty(process.env.SANDCASTLE) || !isEmpty(process.env.GITHUB_ACTIONS) + ); +} + function maybeLogCommand(command: string, args: Array): void { if (EnvironmentOptions.logCommands) { console.log(`RUNNING \`${command} ${args.join(' ')}\``); diff --git a/packages/react-native-fantom/src/Benchmark.js b/packages/react-native-fantom/src/Benchmark.js index a54f4edc440950..d8ff1fafeb348b 100644 --- a/packages/react-native-fantom/src/Benchmark.js +++ b/packages/react-native-fantom/src/Benchmark.js @@ -8,6 +8,7 @@ * @format */ +import {getConstants} from './index'; import nullthrows from 'nullthrows'; import NativeCPUTime from 'react-native/src/private/specs/modules/NativeCPUTime'; import { @@ -36,31 +37,60 @@ export function suite( suiteName: string, suiteOptions?: ?SuiteOptions, ): SuiteAPI { - const {disableOptimizedBuildCheck, ...benchOptions} = suiteOptions ?? {}; - - const bench = new Bench({ - ...benchOptions, - name: suiteName, - throws: true, - now: () => NativeCPUTime.getCPUTimeNanos() / 1000000, - }); - + const tasks: Array<{ + name: string, + fn: () => void, + options: FnOptions | void, + }> = []; const verifyFns = []; global.it(suiteName, () => { - if (bench.tasks.length === 0) { + if (tasks.length === 0) { throw new Error('No benchmark tests defined'); } + const {isRunningFromCI} = getConstants(); + + // If we're running from CI and there's no verification function, there's + // no point in running the benchmark. + // We still run a single iteration of each test just to make sure that the + // logic in the benchmark doesn't break. + const isTestOnly = isRunningFromCI && verifyFns.length === 0; + + const overriddenOptions: BenchOptions = isTestOnly + ? { + warmupIterations: 1, + warmupTime: 0, + iterations: 1, + time: 0, + } + : {}; + + const {disableOptimizedBuildCheck, ...benchOptions} = suiteOptions ?? {}; + + const bench = new Bench({ + ...benchOptions, + ...overriddenOptions, + name: suiteName, + throws: true, + now: () => NativeCPUTime.getCPUTimeNanos() / 1000000, + }); + + for (const task of tasks) { + bench.add(task.name, task.fn, task.options); + } + bench.runSync(); - printBenchmarkResults(bench); + if (!isTestOnly) { + printBenchmarkResults(bench); + } for (const verify of verifyFns) { verify(bench.results); } - if (!NativeCPUTime.hasAccurateCPUTimeNanosForBenchmarks()) { + if (!isTestOnly && !NativeCPUTime.hasAccurateCPUTimeNanosForBenchmarks()) { throw new Error( '`NativeCPUTime` module does not provide accurate CPU time information in this environment. Please run the benchmarks in an environment where it does.', ); @@ -73,7 +103,7 @@ export function suite( const suiteAPI = { add(name: string, fn: () => void, options?: FnOptions): SuiteAPI { - bench.add(name, fn, options); + tasks.push({name, fn, options}); return suiteAPI; }, verify(fn: (results: SuiteResults) => void): SuiteAPI { diff --git a/packages/react-native-fantom/src/index.js b/packages/react-native-fantom/src/index.js index 0072ab4d72350d..3124225ff92634 100644 --- a/packages/react-native-fantom/src/index.js +++ b/packages/react-native-fantom/src/index.js @@ -141,6 +141,22 @@ export function createRoot(rootConfig?: RootConfig): Root { export const benchmark = Benchmark; +type FantomConstants = $ReadOnly<{ + isRunningFromCI: boolean, +}>; + +let constants: FantomConstants = { + isRunningFromCI: false, +}; + +export function getConstants(): FantomConstants { + return constants; +} + +export function setConstants(newConstants: FantomConstants): void { + constants = newConstants; +} + /** * Quick and dirty polyfills required by tinybench. */