Skip to content

Commit

Permalink
refactor(results): Add refresh rate to results model
Browse files Browse the repository at this point in the history
For consistency, refresh rate is added to the DeviceSpecs property of the TestCaseResult and AveragedTestCaseResult

chore(test): update snapshots

refactor: remove unneeded changes

refactor: use logger for error
  • Loading branch information
MalcolmTomisin committed Sep 16, 2024
1 parent c1d658a commit c6d4ded
Show file tree
Hide file tree
Showing 17 changed files with 89 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,16 @@ Time taken to run the test.
Can be helpful to measure Time To Interactive of your app, if the test is checking app start for instance.
Average FPS
60 FPS
Frame Per Second. Your app should display
60
Frames Per Second to give an impression of fluidity. This number should be close to
60
, otherwise it will seem laggy.
Frame Per Second. Your app should display 60 Frames Per Second to give an impression of fluidity. This number should be close to 60, otherwise it will seem laggy.
See
this video
for more details
Average CPU usage
83 %
An app might run at 60FPS but might be using too much processing power, so it's important to check CPU usage.
An app might run at high frame rates, such as 60 FPS or higher, but might be using too much processing power, so it's important to check CPU usage.
Depending on the device, this value can go up to
100% x number of cores
. For instance, a Samsung A10s has 4 cores, so the max value would be 400%.
Expand Down Expand Up @@ -859,7 +856,7 @@ exports[`flashlight measure interactive it displays measures: Web app with measu
<div
class="text-neutral-400 text-sm"
>
An app might run at 60FPS but might be using too much processing power, so it's important to check CPU usage.
An app might run at high frame rates, such as 60 FPS or higher, but might be using too much processing power, so it's important to check CPU usage.
<br />
Depending on the device, this value can go up to
<code>
Expand Down Expand Up @@ -3816,19 +3813,16 @@ Time taken to run the test.
Can be helpful to measure Time To Interactive of your app, if the test is checking app start for instance.
Average FPS
-
Frame Per Second. Your app should display
60
Frames Per Second to give an impression of fluidity. This number should be close to
60
, otherwise it will seem laggy.
Frame Per Second. Your app should display 60 Frames Per Second to give an impression of fluidity. This number should be close to 60, otherwise it will seem laggy.
See
this video
for more details
Average CPU usage
-
An app might run at 60FPS but might be using too much processing power, so it's important to check CPU usage.
An app might run at high frame rates, such as 60 FPS or higher, but might be using too much processing power, so it's important to check CPU usage.
Depending on the device, this value can go up to
100% x number of cores
. For instance, a Samsung A10s has 4 cores, so the max value would be 400%.
Expand Down Expand Up @@ -4226,7 +4220,7 @@ exports[`flashlight measure interactive it displays measures: Web app with no me
<div
class="text-neutral-400 text-sm"
>
An app might run at 60FPS but might be using too much processing power, so it's important to check CPU usage.
An app might run at high frame rates, such as 60 FPS or higher, but might be using too much processing power, so it's important to check CPU usage.
<br />
Depending on the device, this value can go up to
<code>
Expand Down
3 changes: 3 additions & 0 deletions packages/commands/measure/src/server/socket/socketState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ export const addNewResultReducer = (state: SocketData, name: string): SocketData
name,
iterations: [],
status: "SUCCESS",
specs: {
refreshRate: state.refreshRate,
},
},
],
});
15 changes: 3 additions & 12 deletions packages/commands/measure/src/webapp/MeasureWebApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,8 @@ import { SocketState } from "./components/SocketState";
setThemeAtRandom();

export const MeasureWebApp = () => {
const {
autodetect,
bundleId,
start,
stop,
results,
isMeasuring,
reset,
setBundleId,
refreshRate,
} = useMeasures();
const { autodetect, bundleId, start, stop, results, isMeasuring, reset, setBundleId } =
useMeasures();

return (
<div className="bg-light-charcoal h-full text-black">
Expand All @@ -46,7 +37,7 @@ export const MeasureWebApp = () => {
</div>
) : null}
</AppBar>
<IterationsReporterView deviceSpecs={{ refreshRate }} results={results} />
<IterationsReporterView results={results} />
</div>
);
};
8 changes: 2 additions & 6 deletions packages/commands/report/src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react";
import { DeviceSpecs, TestCaseResult } from "@perf-profiler/types";
import { TestCaseResult } from "@perf-profiler/types";
import {
IterationsReporterView,
PageBackground,
Expand All @@ -13,10 +13,6 @@ let testCaseResults: TestCaseResult[] =
// Use very long string so that Parcel won't use it more than once, would be nice to find a better solution
"THIS_IS_A_VERY_LONG_STRING_THAT_IS_UNLIKELY_TO_BE_FOUND_IN_A_TEST_CASE_RESULT";

const deviceSpecs: DeviceSpecs = {
refreshRate: 60,
};

// Uncomment with when locally testing
// // Without videos
// testCaseResults = [
Expand Down Expand Up @@ -65,7 +61,7 @@ export function App() {
return (
<>
<PageBackground />
<IterationsReporterView results={testCaseResults} deviceSpecs={deviceSpecs} />
<IterationsReporterView results={testCaseResults} />
</>
);
}
2 changes: 1 addition & 1 deletion packages/commands/test/src/SingleIterationTester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface TestCase {
run: () => Promise<void> | void;
afterTest?: () => Promise<void> | void;
duration?: number;
getScore?: (result: AveragedTestCaseResult, refreshRate: number) => number;
getScore?: (result: AveragedTestCaseResult) => number;
}

export interface Options {
Expand Down
4 changes: 2 additions & 2 deletions packages/commands/test/src/writeReport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const writeReport = (
}: {
filePath: string;
title: string;
overrideScore?: (result: AveragedTestCaseResult, refreshRate: number) => number;
overrideScore?: (result: AveragedTestCaseResult) => number;
}
) => {
const testCase: TestCaseResult = {
Expand All @@ -34,7 +34,7 @@ export const writeReport = (
*/
if (overrideScore) {
const averagedResult: AveragedTestCaseResult = averageTestCaseResult(testCase);
testCase.score = Math.max(0, Math.min(overrideScore(averagedResult, 60), 100));
testCase.score = Math.max(0, Math.min(overrideScore(averagedResult), 100));
}

fs.writeFileSync(filePath, JSON.stringify(testCase));
Expand Down
23 changes: 11 additions & 12 deletions packages/core/reporter/src/reporting/Report.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TestCaseResult, AveragedTestCaseResult, DeviceSpecs } from "@perf-profiler/types";
import { TestCaseResult, AveragedTestCaseResult } from "@perf-profiler/types";
import { roundToDecimal } from "../utils/round";
import { averageTestCaseResult } from "./averageIterations";
import { getScore } from "./getScore";
Expand Down Expand Up @@ -29,13 +29,11 @@ export class Report {
private result: TestCaseResult;
private averagedResult: AveragedTestCaseResult;
private averageMetrics: ReportMetrics;
private deviceSpecs: DeviceSpecs;

constructor(result: TestCaseResult, deviceSpecs: DeviceSpecs) {
constructor(result: TestCaseResult) {
this.result = Report.filterSuccessfulIterations(result);
this.averagedResult = averageTestCaseResult(this.result);
this.averageMetrics = Report.getAverageMetrics(this.averagedResult);
this.deviceSpecs = deviceSpecs;
}

private static getAverageMetrics(averagedResult: AveragedTestCaseResult): ReportMetrics {
Expand Down Expand Up @@ -77,7 +75,7 @@ export class Report {
}

public get score() {
return this.averagedResult.score ?? getScore(this.averagedResult, this.deviceSpecs.refreshRate);
return this.averagedResult.score ?? getScore(this.averagedResult);
}

public getIterationCount() {
Expand All @@ -93,13 +91,10 @@ export class Report {
}

public selectIteration(iterationIndex: number): Report {
return new Report(
{
...this.result,
iterations: [this.result.iterations[iterationIndex]],
},
this.deviceSpecs
);
return new Report({
...this.result,
iterations: [this.result.iterations[iterationIndex]],
});
}

public getAveragedResult() {
Expand All @@ -122,4 +117,8 @@ export class Report {
threads: getThreadsStats(iterations),
};
}

public getRefreshRate() {
return this.result.specs?.refreshRate ?? 60;
}
}
4 changes: 2 additions & 2 deletions packages/core/reporter/src/reporting/getScore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { canComputeHighCpuUsage } from "./highCpu";
*/
const calculateCpuScore = (x: number) => Math.min(Math.max(0, -0.31666666666667 * x + 116), 100);

export const getScore = (result: AveragedTestCaseResult, refreshRate: number) => {
export const getScore = (result: AveragedTestCaseResult) => {
const averageUIFPS = getAverageFPSUsage(result.average.measures);
const averageCPUUsage = getAverageCpuUsage(result.average.measures);
const totalTimeThreadlocked = Object.keys(result.averageHighCpuUsage).reduce(
Expand All @@ -26,7 +26,7 @@ export const getScore = (result: AveragedTestCaseResult, refreshRate: number) =>
const scores = [cpuScore];

if (averageUIFPS !== undefined) {
const fpsScore = (averageUIFPS * 100) / refreshRate;
const fpsScore = (averageUIFPS * 100) / (result?.specs?.refreshRate ?? 60);
scores.push(fpsScore);
}

Expand Down
2 changes: 2 additions & 0 deletions packages/core/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface TestCaseResult {
status: TestCaseResultStatus;
iterations: TestCaseIterationResult[];
type?: TestCaseResultType;
specs?: DeviceSpecs;
}

export interface AveragedTestCaseResult {
Expand All @@ -49,6 +50,7 @@ export interface AveragedTestCaseResult {
average: TestCaseIterationResult;
averageHighCpuUsage: { [processName: string]: number };
type?: TestCaseResultType;
specs?: DeviceSpecs;
}

// Shouldn't really be here but @perf-profiler/types is imported by everyone and doesn't contain any logic
Expand Down
28 changes: 5 additions & 23 deletions packages/core/web-reporter-ui/ReporterView.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { useMemo } from "react";
import { Measure, POLLING_INTERVAL, TestCaseResult, DeviceSpecs } from "@perf-profiler/types";
import { Measure, POLLING_INTERVAL, TestCaseResult } from "@perf-profiler/types";
import { CPUReport } from "./src/sections/CPUReport";
import { ReportSummary } from "./src/sections/ReportSummary/ReportSummary.component";
import { RAMReport } from "./src/sections/RAMReport";
Expand Down Expand Up @@ -30,17 +30,12 @@ const theme = createTheme({
const Report = ({
results: rawResults,
additionalMenuOptions,
deviceSpecs,
}: {
results: TestCaseResult[];
additionalMenuOptions?: MenuOption[];
deviceSpecs: DeviceSpecs;
}) => {
const results = mapThreadNames(rawResults);
const reports = useMemo(
() => results.map((result) => new ReportModel(result, deviceSpecs)),
[results, deviceSpecs]
);
const reports = useMemo(() => results.map((result) => new ReportModel(result)), [results]);
const minIterationCount = Math.min(...reports.map((report) => report.getIterationCount()));
const iterationSelector = useIterationSelector(minIterationCount);

Expand Down Expand Up @@ -71,7 +66,7 @@ const Report = ({
]}
/>
<Padding />
<ReportSummary reports={selectedReports} deviceSpecs={deviceSpecs} />
<ReportSummary reports={selectedReports} />
<div className="h-16" />

{hasMeasures ? (
Expand Down Expand Up @@ -108,34 +103,21 @@ const Report = ({
export const IterationsReporterView = ({
results,
additionalMenuOptions,
deviceSpecs,
}: {
results: TestCaseResult[];
additionalMenuOptions?: MenuOption[];
deviceSpecs: DeviceSpecs;
}) => {
return results.length > 0 ? (
<ThemeProvider theme={theme}>
<Report
deviceSpecs={deviceSpecs}
results={results}
additionalMenuOptions={additionalMenuOptions}
/>
<Report results={results} additionalMenuOptions={additionalMenuOptions} />
</ThemeProvider>
) : null;
};

export const ReporterView = ({
measures,
deviceSpecs,
}: {
measures: Measure[];
deviceSpecs: DeviceSpecs;
}) => (
export const ReporterView = ({ measures }: { measures: Measure[] }) => (
<>
{measures.length > 1 ? (
<IterationsReporterView
deviceSpecs={deviceSpecs}
results={[
{
name: "Results",
Expand Down
10 changes: 2 additions & 8 deletions packages/core/web-reporter-ui/__tests__/ReporterView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,9 @@ describe("<ReporterView />", () => {
require("../../../commands/report/src/example-reports/results1.json"),
require("../../../commands/report/src/example-reports/results2.json"),
];
const deviceSpecs = {
refreshRate: 60,
};

const { asFragment, baseElement } = render(
<IterationsReporterView deviceSpecs={deviceSpecs} results={testCaseResults} />
<IterationsReporterView results={testCaseResults} />
);
expect(screen.getAllByLabelText("Score")[0].textContent).toEqual("69");

Expand Down Expand Up @@ -50,12 +47,9 @@ describe("<ReporterView />", () => {
require("../../../commands/report/src/example-reports/video/results_417dd25e-d901-4b1e-9d43-3b78305a48e2.json"),
require("../../../commands/report/src/example-reports/video/results_c7d5d17d-42ed-4354-8b43-bb26e2d6feee.json"),
];
const deviceSpecs = {
refreshRate: 60,
};

const { asFragment, baseElement } = render(
<IterationsReporterView deviceSpecs={deviceSpecs} results={testCaseResults} />
<IterationsReporterView results={testCaseResults} />
);
expect(screen.getAllByLabelText("Score")[0].textContent).toEqual("51");

Expand Down
Loading

0 comments on commit c6d4ded

Please sign in to comment.