Skip to content

Commit

Permalink
Run benchmarks in test mode when not specifying verification function…
Browse files Browse the repository at this point in the history
…s in CI (facebook#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
  • Loading branch information
rubennorte authored and facebook-github-bot committed Jan 4, 2025
1 parent 2280ae9 commit 526e0e0
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 13 deletions.
7 changes: 7 additions & 0 deletions packages/react-native-fantom/runner/entrypoint-template.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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}';
Expand All @@ -50,6 +53,10 @@ ${Object.entries(featureFlags)
: ''
}
setConstants({
isRunningFromCI: ${String(isRunningFromCI)},
});
registerTest(() => require('${testPath}'), ${JSON.stringify(snapshotConfig)});
`;
};
2 changes: 2 additions & 0 deletions packages/react-native-fantom/runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
getBuckModesForPlatform,
getDebugInfoFromCommandResult,
getShortHash,
isRunningFromCI,
printConsoleLog,
runBuck2,
runBuck2Sync,
Expand Down Expand Up @@ -198,6 +199,7 @@ module.exports = async function runTest(
updateSnapshot: snapshotState._updateSnapshot,
data: getInitialSnapshotData(snapshotState),
},
isRunningFromCI: isRunningFromCI(),
});

const entrypointPath = path.join(
Expand Down
10 changes: 10 additions & 0 deletions packages/react-native-fantom/runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>): void {
if (EnvironmentOptions.logCommands) {
console.log(`RUNNING \`${command} ${args.join(' ')}\``);
Expand Down
56 changes: 43 additions & 13 deletions packages/react-native-fantom/src/Benchmark.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* @format
*/

import {getConstants} from './index';
import nullthrows from 'nullthrows';
import NativeCPUTime from 'react-native/src/private/specs/modules/NativeCPUTime';
import {
Expand Down Expand Up @@ -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.',
);
Expand All @@ -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 {
Expand Down
16 changes: 16 additions & 0 deletions packages/react-native-fantom/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down

0 comments on commit 526e0e0

Please sign in to comment.