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

Slightly improve perf report #5501

Merged
merged 5 commits into from
Apr 30, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/performance-report.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
name: Performance Report
env:
BUILD_BOOTSTRAP_CJS: mv dist dist-build && node dist-build/bin/rollup --config rollup.config.ts --configPlugin typescript --configTest --forceExit && rm -rf dist-build

on:
pull_request:
pull_request_target:
types:
- synchronize
- opened
- reopened
- labeled

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
Expand All @@ -18,6 +17,7 @@ permissions:

jobs:
build-artefacts:
if: ${{ github.event.pull_request.head.repo.full_name == 'rollup/rollup' || contains( toJson(github.event.pull_request.labels), 'x⁸ ⚙️ build repl artefacts' ) }}
strategy:
matrix:
settings:
Expand Down Expand Up @@ -62,7 +62,7 @@ jobs:
if: steps.cache-node-modules.outputs.cache-hit != 'true'
run: npm ci --ignore-scripts
- name: Build artefacts 123
run: npm exec -- concurrently -c green,blue 'npm:build:napi -- --release' 'npm:build:cjs' && npm run build:copy-native && ${{env.BUILD_BOOTSTRAP_CJS}} && npm run build:copy-native
run: npm exec -- concurrently -c green,blue 'npm:build:napi -- --release' 'npm:build:cjs' && npm run build:copy-native && npm run build:bootstrap:cjs && npm run build:copy-native
- name: Upload artifact
uses: actions/upload-artifact@v4
with:
Expand All @@ -73,7 +73,7 @@ jobs:
report:
needs: build-artefacts
permissions:
pull-requests: write
pull-requests: write # for peter-evans/find-comment and peter-evans/create-or-update-comment
runs-on: ubuntu-latest
name: Report Performance
steps:
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"dev": "vitepress dev docs",
"build:cjs": "rollup --config rollup.config.ts --configPlugin typescript --configTest --forceExit",
"build:bootstrap": "shx mv dist dist-build && node dist-build/bin/rollup --config rollup.config.ts --configPlugin typescript --forceExit && shx rm -rf dist-build",
"build:bootstrap:cjs": "shx mv dist dist-build && node dist-build/bin/rollup --config rollup.config.ts --configPlugin typescript --configTest --forceExit && shx rm -rf dist-build",
"build:docs": "vitepress build docs",
"build:ast-converters": "node scripts/generate-ast-converters.js",
"preview:docs": "vitepress preview docs",
Expand All @@ -66,7 +67,7 @@
"lint:markdown:nofix": "prettier --check \"**/*.md\"",
"lint:rust": "cd rust && cargo fmt && cargo clippy --fix --allow-dirty",
"lint:rust:nofix": "cd rust && cargo fmt --check && cargo clippy",
"perf": "npm run build && node --expose-gc scripts/perf-report/index.js",
"perf": "npm run build:bootstrap:cjs && node --expose-gc scripts/perf-report/index.js",
"prepare": "husky && node scripts/check-release.js || npm run build:prepare",
"prepublishOnly": "node scripts/check-release.js && node scripts/prepublish.js",
"postpublish": "node scripts/postpublish.js",
Expand Down
46 changes: 29 additions & 17 deletions scripts/perf-report/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ const ENTRY = new URL('entry.js', PERF_DIRECTORY);
const THREEJS_COPIES = 10;
const { bold, underline, cyan, red, green } = createColors();
const MIN_ABSOLUTE_TIME_DEVIATION = 10;
const RELATIVE_DEVIATION_FOR_COLORING = 5;
const RUNS_TO_AVERAGE = 5;
const DISCARDED_RESULTS = 2;
const MIN_RELATIVE_DEVIATION_PERCENT = 2;
const RELATIVE_DEVIATION_PERCENT_FOR_COLORING = 5;
const RUNS_TO_AVERAGE = 7;
const DISCARDED_LARGE_RESULTS = 2;
const DISCARDED_SMALL_RESULTS = 1;

await ensureBenchmarkExists();
await calculatePrintAndPersistTimings();
Expand Down Expand Up @@ -62,8 +64,8 @@ async function calculatePrintAndPersistTimings() {
console.info(
bold(
`Comparing against rollup@${previousVersion}.\nCalculating the average of ${cyan(RUNS_TO_AVERAGE)} runs discarding the ${cyan(
DISCARDED_RESULTS
)} largest results.`
DISCARDED_LARGE_RESULTS
)} largest and the ${cyan(DISCARDED_SMALL_RESULTS)}s smallest results.`
)
);
chdir(fileURLToPath(PERF_DIRECTORY));
Expand Down Expand Up @@ -136,12 +138,14 @@ function getAverage(accumulatedMeasurements, runs) {
memory: getSingleAverage(
accumulatedMeasurements[label].map(timing => timing[2]),
runs,
DISCARDED_RESULTS
DISCARDED_LARGE_RESULTS,
DISCARDED_SMALL_RESULTS
),
time: getSingleAverage(
accumulatedMeasurements[label].map(timing => timing[0]),
runs,
DISCARDED_RESULTS
DISCARDED_LARGE_RESULTS,
DISCARDED_SMALL_RESULTS
)
};
}
Expand All @@ -151,16 +155,19 @@ function getAverage(accumulatedMeasurements, runs) {
/**
* @param {number[]} times
* @param {number} runs
* @param {number} discarded
* @param {number} discardedLarge
* @param {number} discardedSmall
* @return {number}
*/
function getSingleAverage(times, runs, discarded) {
const actualDiscarded = Math.min(discarded, runs - 1);
function getSingleAverage(times, runs, discardedLarge, discardedSmall) {
const actualDiscarded = Math.min(discardedLarge + discardedSmall, runs - 1);
const actualDiscardedSmall = Math.max(actualDiscarded - discardedLarge, 0);
const actualDiscardedLarge = actualDiscarded - actualDiscardedSmall;
return (
times
.sort()
.reverse()
.slice(actualDiscarded)
.slice(actualDiscardedLarge, runs - actualDiscardedSmall)
.reduce((sum, time) => sum + time, 0) /
(runs - actualDiscarded)
);
Expand Down Expand Up @@ -255,13 +262,16 @@ function getFormattedTime(measuredTime, baseTime = measuredTime) {
let color = identity,
formattedTime = `${measuredTime.toFixed(0)}ms`;
const absoluteDeviation = Math.abs(measuredTime - baseTime);
if (absoluteDeviation > MIN_ABSOLUTE_TIME_DEVIATION) {
const relativeDeviation = 100 * (absoluteDeviation / baseTime);
if (
absoluteDeviation > MIN_ABSOLUTE_TIME_DEVIATION &&
relativeDeviation > MIN_RELATIVE_DEVIATION_PERCENT
) {
const sign = measuredTime >= baseTime ? '+' : '-';
const relativeDeviation = 100 * (absoluteDeviation / baseTime);
formattedTime += ` (${sign}${absoluteDeviation.toFixed(
0
)}ms, ${sign}${relativeDeviation.toFixed(1)}%)`;
if (relativeDeviation > RELATIVE_DEVIATION_FOR_COLORING) {
if (relativeDeviation > RELATIVE_DEVIATION_PERCENT_FOR_COLORING) {
color = measuredTime >= baseTime ? red : green;
}
}
Expand All @@ -280,11 +290,13 @@ function getFormattedMemory(currentMemory, persistedMemory = currentMemory) {
let color = identity,
formattedMemory = prettyBytes(currentMemory);
const absoluteDeviation = Math.abs(currentMemory - persistedMemory);
const sign = currentMemory >= persistedMemory ? '+' : '-';
const relativeDeviation = 100 * (absoluteDeviation / persistedMemory);
if (relativeDeviation > RELATIVE_DEVIATION_FOR_COLORING) {
if (relativeDeviation > MIN_RELATIVE_DEVIATION_PERCENT) {
const sign = currentMemory >= persistedMemory ? '+' : '-';
formattedMemory += ` (${sign}${relativeDeviation.toFixed(0)}%)`;
color = currentMemory >= persistedMemory ? red : green;
if (relativeDeviation > RELATIVE_DEVIATION_PERCENT_FOR_COLORING) {
color = currentMemory >= persistedMemory ? red : green;
}
}
return color(formattedMemory);
}
Expand Down
10 changes: 8 additions & 2 deletions scripts/perf-report/report-collector.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@ export default new (class ReportCollector {
*/
push(message) {
if (!this.#isRecording) return;
if (message.startsWith('#')) {
message = '##' + message;
if (message.startsWith('# ')) {
message = message.replace(/^# /, '- ');
} else if (message.startsWith('## ')) {
message = message.replace(/^## /, ' - ');
} else if (message.startsWith('- ')) {
message = ' ' + message;
} else {
message = ' - ' + message;
}
this.#messageList.push(message);
}
Expand Down
14 changes: 8 additions & 6 deletions src/utils/timers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ interface Timer {

let timers = new Map<string, Timer>();

function getPersistedLabel(label: string, level: number): string {
type LabelLevel = 1 | 2 | 3 | 4;

function getPersistedLabel(label: string, level: LabelLevel): string {
switch (level) {
case 1: {
return `# ${label}`;
Expand All @@ -30,12 +32,12 @@ function getPersistedLabel(label: string, level: number): string {
return label;
}
default: {
return `${' '.repeat(level - 4)}- ${label}`;
return `- ${label}`;
}
}
}

function timeStartImpl(label: string, level = 3): void {
function timeStartImpl(label: string, level: LabelLevel = 3): void {
label = getPersistedLabel(label, level);

const startMemory = process.memoryUsage().heapUsed;
Expand All @@ -57,7 +59,7 @@ function timeStartImpl(label: string, level = 3): void {
}
}

function timeEndImpl(label: string, level = 3): void {
function timeEndImpl(label: string, level: LabelLevel = 3): void {
label = getPersistedLabel(label, level);

const timer = timers.get(label);
Expand All @@ -79,8 +81,8 @@ export function getTimings(): SerializedTimings {
return newTimings;
}

export let timeStart: (label: string, level?: number) => void = doNothing;
export let timeEnd: (label: string, level?: number) => void = doNothing;
export let timeStart: (label: string, level?: LabelLevel) => void = doNothing;
export let timeEnd: (label: string, level?: LabelLevel) => void = doNothing;

const TIMED_PLUGIN_HOOKS: readonly (keyof PluginHooks)[] = [
'augmentChunkHash',
Expand Down