Skip to content

Commit

Permalink
feat(profiler): prepare for flipper plugin publish (#42)
Browse files Browse the repository at this point in the history
* fix(profiler): ensure atrace can get restarted

Since the Flipper plugin starts/stops the ATrace process, we need to ensure that when restarting it, atrace gets restarted
ensureCppProfilerIsInstalled is responsible for that but is basing itself on the atraceProcess being null or not

* fix(profiler): ensure profiler can be initialized programatically

Since getCpuClockTick and getRAMPageSize were previously called at the root, just importing the package and running
commands would automatically install the cpp profiler (which is a task taking a few seconds)

Several issues with that:
- loading Flipper would load the Flipper plugin and try to install the c++ profiler which impacts Flipper load time a lot
- since atrace would be started when loading the Flipper plugin, the buffer would already be filled with lots of data when actually starting measures
- just less flexibility in general as we will see in the next commit

* style(profiler): remove unnecessary require

* test: add test for killing atrace
  • Loading branch information
Almouro authored Oct 25, 2022
1 parent d80efbc commit 43e97e3
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 24 deletions.
8 changes: 1 addition & 7 deletions packages/android-performance-profiler/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { program } from "commander";
import { detectCurrentAppBundleId } from "./commands/detectCurrentAppBundleId";
import { getPidId } from "./commands/getPidId";
import { getAbi } from "./commands/getAbi";
import { pollPerformanceMeasures } from "./commands/pollPerformanceMeasures";

const debugCppConfig = () => {
ensureCppProfilerIsInstalled();
Expand Down Expand Up @@ -68,13 +69,6 @@ program
"Display CPU Usage for a given threads (e.g. (mqt_js))"
)
.action((options) => {
const {
pollPerformanceMeasures,
// Makes sure we don't start Atrace by just requiring
// A better fix would be to not run atrace when requiring the file
// eslint-disable-next-line @typescript-eslint/no-var-requires
} = require("./commands/pollPerformanceMeasures");

const bundleId = options.bundleId || detectCurrentAppBundleId().bundleId;
const pid = getPidId(bundleId);

Expand Down
37 changes: 31 additions & 6 deletions packages/android-performance-profiler/src/commands/cppProfiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,19 @@ const startATrace = () => {

const stopATrace = () => {
aTraceProcess?.kill();
aTraceProcess = null;
};

/**
* Main setup function for the cpp profiler
*
* It will:
* - install the C++ profiler for the correct architecture on the device
* - Starts the atrace process (the c++ profiler will then starts another thread to read from it)
* - Populate needed values like CPU clock tick and RAM page size
*
* This needs to be done before measures and can take a few seconds
*/
export const ensureCppProfilerIsInstalled = () => {
const sdkVersion = parseInt(
executeCommand("adb shell getprop ro.build.version.sdk"),
Expand All @@ -55,27 +66,41 @@ export const ensureCppProfilerIsInstalled = () => {
const command = `adb push ${binaryPath} ${deviceProfilerPath}`;
executeCommand(command);
Logger.success(`C++ Profiler installed in ${deviceProfilerPath}`);

retrieveCpuClockTick();
retrieveRAMPageSize();
}
if (!aTraceProcess) startATrace();
hasInstalledProfiler = true;
};

export const getCpuClockTick = () => {
ensureCppProfilerIsInstalled();
return parseInt(
let cpuClockTick: number;
let RAMPageSize: number;

const retrieveCpuClockTick = () => {
cpuClockTick = parseInt(
executeCommand(`adb shell ${deviceProfilerPath} printCpuClockTick`),
10
);
};

export const getRAMPageSize = () => {
ensureCppProfilerIsInstalled();
return parseInt(
const retrieveRAMPageSize = () => {
RAMPageSize = parseInt(
executeCommand(`adb shell ${deviceProfilerPath} printRAMPageSize`),
10
);
};

export const getCpuClockTick = () => {
ensureCppProfilerIsInstalled();
return cpuClockTick;
};

export const getRAMPageSize = () => {
ensureCppProfilerIsInstalled();
return RAMPageSize;
};

type CppPerformanceMeasure = {
cpu: string;
ram: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import { CpuMeasure as Measure } from "@perf-profiler/types";
import { getCpuClockTick } from "../cppProfiler";
import { ProcessStat } from "./getCpuStatsByProcess";

const SYSTEM_TICK_IN_ONE_SECOND = getCpuClockTick();

export class CpuMeasureAggregator {
private previousTotalCpuTimePerProcessId: { [processId: string]: number } =
{};
Expand All @@ -16,8 +14,7 @@ export class CpuMeasureAggregator {
): {
[by: string]: number;
} {
const TICKS_FOR_TIME_INTERVAL =
(SYSTEM_TICK_IN_ONE_SECOND * timeInterval) / 1000;
const TICKS_FOR_TIME_INTERVAL = (getCpuClockTick() * timeInterval) / 1000;

const toPercentage = (value: number) =>
Math.min((value * 100) / TICKS_FOR_TIME_INTERVAL, 100);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { getRAMPageSize } from "../cppProfiler";

const BYTES_PER_MB = 1024 * 1024;
const RAM_PAGE_SIZE = getRAMPageSize();

export const getCommand = (pid: string) => `cat /proc/${pid}/statm`;

export const processOutput = (result: string) => {
return (parseInt(result.split(" ")[1], 10) * RAM_PAGE_SIZE) / BYTES_PER_MB;
return (parseInt(result.split(" ")[1], 10) * getRAMPageSize()) / BYTES_PER_MB;
};
2 changes: 2 additions & 0 deletions packages/android-performance-profiler/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import { getPidId } from "./commands/getPidId";
import { pollPerformanceMeasures } from "./commands/pollPerformanceMeasures";
import { parseGfxInfo } from "./commands/gfxInfo/parseGfxInfo";
import { compareGfxMeasures } from "./commands/gfxInfo/compareGfxMeasures";
import { ensureCppProfilerIsInstalled } from "./commands/cppProfiler";

export { Measure } from "@perf-profiler/types";
export { Measure as GfxInfoMeasure } from "./commands/gfxInfo/parseGfxInfo";

export {
ensureCppProfilerIsInstalled,
compareGfxMeasures,
detectCurrentAppBundleId,
getPidId,
Expand Down
6 changes: 5 additions & 1 deletion packages/e2e-performance/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
import { averageTestCaseResult } from "@perf-profiler/reporter";
import fs from "fs";
import { PerformanceMeasurer } from "./PerformanceMeasurer";
import { ensureCppProfilerIsInstalled } from "@perf-profiler/profiler";

export interface TestCase {
beforeTest?: () => Promise<void> | void;
Expand All @@ -17,7 +18,10 @@ export interface TestCase {
}

class PerformanceTester {
constructor(private bundleId: string, private testCase: TestCase) {}
constructor(private bundleId: string, private testCase: TestCase) {
// Important to ensure that the CPP profiler is initialized before we run the test!
ensureCppProfilerIsInstalled();
}

private async executeTestCase(): Promise<TestCaseIterationResult> {
const { beforeTest, run, afterTest, duration } = this.testCase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ jest.mock("child_process", () => {
return 4096;
case "adb shell getprop ro.product.cpu.abi":
return "arm64-v8a";
case "adb shell getprop ro.build.version.sdk":
return "30";
case "adb shell setprop debug.hwui.profile true":
case "adb shell atrace --async_stop 1>/dev/null":
return "";
Expand All @@ -48,14 +50,21 @@ jest.mock("child_process", () => {
};
});

const mockSpawn = (): { stdout: EventEmitter } => {
const mockSpawn = (): { stdout: EventEmitter; kill: () => void } => {
const mockProcess = new EventEmitter();
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
mockProcess.stdout = new EventEmitter();

jest
.spyOn(require("child_process"), "spawn")
.mockImplementationOnce((...args) => {
expect(args).toEqual([
"adb",
["shell", "atrace", "-c", "view", "-t", "999"],
]);
return mockProcess;
})
.mockImplementationOnce((...args) => {
expect(args).toEqual([
"adb",
Expand All @@ -71,9 +80,7 @@ const mockSpawn = (): { stdout: EventEmitter } => {

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
mockProcess.kill = () => {
//
};
mockProcess.kill = jest.fn();

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
Expand Down Expand Up @@ -146,4 +153,5 @@ test("displays FPS data and scoring", async () => {
expect(renderer.baseElement).toMatchSnapshot();

fireEvent.click(screen.getByText("Stop Measuring"));
expect(spawn.kill).toBeCalled();
});

0 comments on commit 43e97e3

Please sign in to comment.