Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NoQA] e2e: new common metrics (FPS, CPU, RAM) #43482

Merged
merged 9 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 122 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@
"@octokit/core": "4.0.4",
"@octokit/plugin-paginate-rest": "3.1.0",
"@octokit/plugin-throttling": "4.1.0",
"@perf-profiler/profiler": "^0.10.9",
"@perf-profiler/reporter": "^0.8.1",
"@perf-profiler/types": "^0.8.0",
"@react-native-community/eslint-config": "3.2.0",
"@react-native/babel-preset": "^0.73.21",
"@react-native/metro-config": "^0.73.5",
Expand Down
54 changes: 54 additions & 0 deletions patches/@perf-profiler+android+0.12.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
diff --git a/node_modules/@perf-profiler/android/dist/src/commands.js b/node_modules/@perf-profiler/android/dist/src/commands.js
old mode 100755
new mode 100644
diff --git a/node_modules/@perf-profiler/android/dist/src/commands/platforms/UnixProfiler.js b/node_modules/@perf-profiler/android/dist/src/commands/platforms/UnixProfiler.js
index 77b9ee0..59aeed9 100644
--- a/node_modules/@perf-profiler/android/dist/src/commands/platforms/UnixProfiler.js
+++ b/node_modules/@perf-profiler/android/dist/src/commands/platforms/UnixProfiler.js
@@ -134,7 +134,20 @@ class UnixProfiler {
}
const subProcessesStats = (0, getCpuStatsByProcess_1.processOutput)(cpu, pid);
const ram = (0, pollRamUsage_1.processOutput)(ramStr, this.getRAMPageSize());
- const { frameTimes, interval: atraceInterval } = frameTimeParser.getFrameTimes(atrace, pid);
+
+ let output;
+ try {
+ output = frameTimeParser.getFrameTimes(atrace, pid);
+ } catch (e) {
+ console.error(e);
+ }
+
+ if (!output) {
+ return;
+ }
+
+ const { frameTimes, interval: atraceInterval } = output;
+
if (!initialTime) {
initialTime = timestamp;
}
diff --git a/node_modules/@perf-profiler/android/src/commands/platforms/UnixProfiler.ts b/node_modules/@perf-profiler/android/src/commands/platforms/UnixProfiler.ts
index d6983c1..ccacf09 100644
--- a/node_modules/@perf-profiler/android/src/commands/platforms/UnixProfiler.ts
+++ b/node_modules/@perf-profiler/android/src/commands/platforms/UnixProfiler.ts
@@ -136,7 +136,19 @@ export abstract class UnixProfiler implements Profiler {
const subProcessesStats = processOutput(cpu, pid);

const ram = processRamOutput(ramStr, this.getRAMPageSize());
- const { frameTimes, interval: atraceInterval } = frameTimeParser.getFrameTimes(atrace, pid);
+
+ let output;
+ try {
+ output = frameTimeParser.getFrameTimes(atrace, pid);
+ } catch (e) {
+ console.error(e);
+ }
+
+ if (!output) {
+ return;
+ }
+
+ const { frameTimes, interval: atraceInterval } = output;
Comment on lines +37 to +51
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this patch, or was that just for your debugging?

If we need it, we need an upstream issue + PR please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hannojg I already created it bamlab/flashlight#291 but haven't received any comments yet 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah awesome - I think we should add this to the PR description !

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hannojg done 😊


if (!initialTime) {
initialTime = timestamp;
25 changes: 25 additions & 0 deletions patches/@perf-profiler+reporter+0.8.1.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
diff --git a/node_modules/@perf-profiler/reporter/dist/src/index.d.ts b/node_modules/@perf-profiler/reporter/dist/src/index.d.ts
index 2f84d84..14ae688 100644
--- a/node_modules/@perf-profiler/reporter/dist/src/index.d.ts
+++ b/node_modules/@perf-profiler/reporter/dist/src/index.d.ts
@@ -4,4 +4,6 @@ export * from "./reporting/Report";
export * from "./utils/sanitizeProcessName";
export * from "./utils/round";
export * from "./reporting/cpu";
+export * from "./reporting/ram";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that I created PR to upstream: bamlab/flashlight#294

+export * from "./reporting/fps";
export { canComputeHighCpuUsage } from "./reporting/highCpu";
diff --git a/node_modules/@perf-profiler/reporter/dist/src/index.js b/node_modules/@perf-profiler/reporter/dist/src/index.js
index 4b50e3a..780963a 100644
--- a/node_modules/@perf-profiler/reporter/dist/src/index.js
+++ b/node_modules/@perf-profiler/reporter/dist/src/index.js
@@ -21,6 +21,8 @@ __exportStar(require("./reporting/Report"), exports);
__exportStar(require("./utils/sanitizeProcessName"), exports);
__exportStar(require("./utils/round"), exports);
__exportStar(require("./reporting/cpu"), exports);
+__exportStar(require("./reporting/fps"), exports);
+__exportStar(require("./reporting/ram"), exports);
var highCpu_1 = require("./reporting/highCpu");
Object.defineProperty(exports, "canComputeHighCpuUsage", { enumerable: true, get: function () { return highCpu_1.canComputeHighCpuUsage; } });
//# sourceMappingURL=index.js.map
\ No newline at end of file
2 changes: 2 additions & 0 deletions tests/e2e/config.dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const appPath = './android/app/build/outputs/apk/development/debug/app-developme
const config: Config = {
MAIN_APP_PACKAGE: packageName,
DELTA_APP_PACKAGE: packageName,
BRANCH_MAIN: 'main',
BRANCH_DELTA: 'main',
MAIN_APP_PATH: appPath,
DELTA_APP_PATH: appPath,
Comment on lines 7 to 12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hannojg yes. When we submit test result, then we specify branch and then we use the name of the branch to store the result.

Flashlight is running on the server, so we have to distinguish different builds (that's why I added BRANCH_MAIN/BRANCH_DELTA).

For dev config we run the same debug build (always main). That's why we specify main in both cases 🙂

RUNS: 8,
Expand Down
3 changes: 3 additions & 0 deletions tests/e2e/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ export default {
MAIN_APP_PATH: './app-e2eRelease.apk',
DELTA_APP_PATH: './app-e2edeltaRelease.apk',

BRANCH_MAIN: 'main',
BRANCH_DELTA: 'delta',

ENTRY_FILE: 'src/libs/E2E/reactNativeLaunchingTest.ts',

// The path to the activity within the app that we want to launch.
Expand Down
15 changes: 14 additions & 1 deletion tests/e2e/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ type TestDoneListener = () => void;

type TestResultListener = (testResult: TestResult) => void;

type AddListener<TListener> = (listener: TListener) => void;
type AddListener<TListener> = (listener: TListener) => () => void;

type ServerInstance = {
setTestConfig: (testConfig: TestConfig) => void;
getTestConfig: () => TestConfig;
addTestStartedListener: AddListener<TestStartedListener>;
addTestResultListener: AddListener<TestResultListener>;
addTestDoneListener: AddListener<TestDoneListener>;
forceTestCompletion: () => void;
setReadyToAcceptTestResults: (isReady: boolean) => void;
isReadyToAcceptTestResults: boolean;
start: () => Promise<void>;
stop: () => Promise<Error | undefined>;
};
Expand Down Expand Up @@ -115,6 +117,13 @@ const createServerInstance = (): ServerInstance => {
const setTestConfig = (testConfig: TestConfig) => {
activeTestConfig = testConfig;
};
const getTestConfig = (): TestConfig => {
if (!activeTestConfig) {
throw new Error('No test config set');
}

return activeTestConfig;
};

const server = createServer((req, res): ServerResponse<IncomingMessage> | void => {
res.statusCode = 200;
Expand Down Expand Up @@ -212,7 +221,11 @@ const createServerInstance = (): ServerInstance => {

return {
setReadyToAcceptTestResults,
get isReadyToAcceptTestResults() {
return isReadyToAcceptTestResults;
},
setTestConfig,
getTestConfig,
addTestStartedListener,
addTestResultListener,
addTestDoneListener,
Expand Down
Loading
Loading