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

Add custom GHA report for package size #53241

Merged
merged 21 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from 19 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
58 changes: 35 additions & 23 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,38 +89,15 @@ jobs:

smoke:
runs-on: ubuntu-latest
defaults:
run:
working-directory: ./pr

steps:
- uses: actions/checkout@v3
with:
path: pr

- uses: actions/checkout@v3
with:
path: base
ref: ${{ github.base_ref }}
if: github.event_name == 'pull_request'

- uses: actions/setup-node@v3
with:
node-version: "*"
check-latest: true

# Pre-build the base branch so we can check lib folder size changes.
# Note that github.sha points to a merge commit, meaning we're testing
# the base branch versus the base branch with the PR applied.
- name: Build base LKG
if: github.event_name == 'pull_request'
run: |
npm ci
npx hereby lkg
rm -rf $GITHUB_WORKSPACE/pr/lib
mv ./lib $GITHUB_WORKSPACE/pr/
working-directory: ./base

- run: npm ci

- run: npx hereby lkg
Expand Down Expand Up @@ -177,6 +154,41 @@ jobs:
node ./smoke.js typescript
node ./smoke.js typescript/lib/tsserverlibrary

package-size:
runs-on: ubuntu-latest
if: github.event_name == 'pull_request'

steps:
- uses: actions/checkout@v3
with:
path: pr

- uses: actions/checkout@v3
with:
path: base
ref: ${{ github.base_ref }}

- uses: actions/setup-node@v3
with:
node-version: "*"
check-latest: true

- run: npm ci
working-directory: ./pr

- run: npm ci
working-directory: ./base

- run: npx hereby lkg
working-directory: ./pr

- run: npx hereby lkg
working-directory: ./base

- run: |
echo "See $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID for more info."
node ./pr/scripts/checkPackageSize.mjs ./base ./pr >> $GITHUB_STEP_SUMMARY

misc:
runs-on: ubuntu-latest

Expand Down
14 changes: 1 addition & 13 deletions Herebyfile.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { localizationDirectories } from "./scripts/build/localization.mjs";
import cmdLineOptions from "./scripts/build/options.mjs";
import { buildProject, cleanProject, watchProject } from "./scripts/build/projects.mjs";
import { localBaseline, localRwcBaseline, refBaseline, refRwcBaseline, runConsoleTests } from "./scripts/build/tests.mjs";
import { Debouncer, Deferred, exec, getDiffTool, getDirSize, memoize, needsUpdate, readJson } from "./scripts/build/utils.mjs";
import { Debouncer, Deferred, exec, getDiffTool, memoize, needsUpdate, readJson } from "./scripts/build/utils.mjs";

const glob = util.promisify(_glob);

Expand Down Expand Up @@ -833,19 +833,7 @@ export const produceLKG = task({
throw new Error("Cannot replace the LKG unless all built targets are present in directory 'built/local/'. The following files are missing:\n" + missingFiles.join("\n"));
}

/** @type {number | undefined} */
let sizeBefore;
if (fs.existsSync("lib")) {
sizeBefore = getDirSize("lib");
}
await exec(process.execPath, ["scripts/produceLKG.mjs"]);

if (sizeBefore !== undefined) {
const sizeAfter = getDirSize("lib");
if (sizeAfter > (sizeBefore * 1.10)) {
throw new Error("The lib folder increased by 10% or more. This likely indicates a bug.");
}
}
}
});

Expand Down
19 changes: 0 additions & 19 deletions scripts/build/utils.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import chalk from "chalk";
import { spawn } from "child_process";
import fs from "fs";
import JSONC from "jsonc-parser";
import path from "path";
import which from "which";

/**
Expand Down Expand Up @@ -140,24 +139,6 @@ export function getDiffTool() {
return program;
}

/**
* Find the size of a directory recursively.
* Symbolic links can cause a loop.
* @param {string} root
* @returns {number} bytes
*/
export function getDirSize(root) {
const stats = fs.lstatSync(root);

if (!stats.isDirectory()) {
return stats.size;
}

return fs.readdirSync(root)
.map(file => getDirSize(path.join(root, file)))
.reduce((acc, num) => acc + num, 0);
}

/**
* @template T
*/
Expand Down
206 changes: 206 additions & 0 deletions scripts/checkPackageSize.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
import assert from "assert";
import cp from "child_process";

const baseRepo = process.argv[2];
const headRepo = process.argv[3];

/** @type {{ size: number, unpackedSize: number; files: { path: string; size: number; }[]; }[]} */
jakebailey marked this conversation as resolved.
Show resolved Hide resolved
const [before, after] = JSON.parse(cp.execFileSync("npm", ["pack", "--dry-run", "--json", baseRepo, headRepo], { encoding: "utf8" }));

/** @param {{ path: string; size: number; }[]} files */
function filesToMap(files) {
return new Map(files.map(f => [f.path, f.size]));
}

const beforeFileToSize = filesToMap(before.files);
const afterFileToSize = filesToMap(after.files);

/**
* @param {number} before
* @param {number} after
*/
function failIfTooBig(before, after) {
if (after > (before * 1.1)) {
process.exitCode = 1;
}
}

/**
* @param {number} value
*/
function sign(value) {
return value > 0 ? "+" : "-";
}

const units = ["B", "KiB", "MiB", "GiB"];
/**
* @param {number} size
*/
function prettyPrintSize(size) {
assert(size >= 0);

let i = 0;
while (size > 1024) {
i++;
size /= 1024;
}

return `${size.toFixed(2)} ${units[i]}`;
}

/**
* @param {number} before
* @param {number} after
*/
function prettyPrintSizeDiff(before, after) {
const diff = after - before;
return sign(diff) + prettyPrintSize(Math.abs(diff));
}

/**
* @param {number} before
* @param {number} after
*/
function prettyPercentDiff(before, after) {
const percent = 100 * (after - before) / before;
return `${sign(percent)}${Math.abs(percent).toFixed(2)}%`;
}

/**
* @param {string[]} header
* @param {string[][]} data
*/
function logTable(header, data) {
/** @type {string[]} */
const lines = [];

/**
* @param {string[]} row
*/
function addRow(row) {
lines.push("| " + row.join(" | ") + " |");
}

addRow(header);
addRow(new Array(header.length).fill("-"));
for (const row of data) {
addRow(row);
}

console.log(lines.join("\n"));
}

console.log(`# Package size report`);
console.log();

console.log(`## Overall package size`);
console.log();

if (before.size === after.size && before.unpackedSize === after.unpackedSize) {
console.log("No change.");
}
else {
logTable(
["", "Before", "After", "Diff", "Diff (percent)"],
[
[
"Packed",
prettyPrintSize(before.size),
prettyPrintSize(after.size),
prettyPrintSizeDiff(before.size, after.size),
prettyPercentDiff(before.size, after.size),
],
[
"Unpacked",
prettyPrintSize(before.unpackedSize),
prettyPrintSize(after.unpackedSize),
prettyPrintSizeDiff(before.unpackedSize, after.unpackedSize),
prettyPercentDiff(before.unpackedSize, after.unpackedSize),
],
]
);
}

failIfTooBig(before.size, after.size);
failIfTooBig(before.unpackedSize, after.unpackedSize);

console.log();


/** @type {Map<string, number>} */
const fileCounts = new Map();
const inBefore = -1;
const inAfter = 1;

/**
* @param {Iterable<string>} paths
* @param {-1 | 1} marker
*/
function addFiles(paths, marker) {
for (const p of paths) {
fileCounts.set(p, (fileCounts.get(p) ?? 0) + marker);
}
}
addFiles(beforeFileToSize.keys(), inBefore);
addFiles(afterFileToSize.keys(), inAfter);

const allEntries = [...fileCounts.entries()];
const commonFiles = allEntries.filter(([, count]) => count === 0).map(([path]) => path);
const beforeOnly = allEntries.filter(([, count]) => count === inBefore).map(([path]) => path);
const afterOnly = allEntries.filter(([, count]) => count === inAfter).map(([path]) => path);

const commonData = commonFiles.map(path => {
const beforeSize = beforeFileToSize.get(path) ?? 0;
const afterSize = afterFileToSize.get(path) ?? 0;
return { path, beforeSize, afterSize };
})
.filter(({ beforeSize, afterSize }) => beforeSize !== afterSize)
.map(({ path, beforeSize, afterSize }) => {
// failIfTooBig(beforeSize, afterSize);
jakebailey marked this conversation as resolved.
Show resolved Hide resolved
return [
"`" + path + "`",
prettyPrintSize(beforeSize),
prettyPrintSize(afterSize),
prettyPrintSizeDiff(beforeSize, afterSize),
prettyPercentDiff(beforeSize, afterSize),
];
});

if (commonData.length > 0) {
console.log(`## Files`);
console.log();
logTable(["", "Before", "After", "Diff", "Diff (percent)"], commonData);
console.log();
}

if (afterOnly.length > 0) {
console.log(`## New files`);
console.log();
logTable(
["", "Size"],
afterOnly.map(path => {
const afterSize = afterFileToSize.get(path) ?? 0;
return { path, afterSize };
})
.map(({ path, afterSize }) => {
return ["`" + path + "`", prettyPrintSize(afterSize)];
}),
);
console.log();
}

if (beforeOnly.length > 0) {
console.log(`## Deleted files`);
console.log();
logTable(
["", "Size"],
beforeOnly.map(path => {
const afterSize = afterFileToSize.get(path) ?? 0;
return { path, afterSize };
})
.map(({ path, afterSize }) => {
return ["`" + path + "`", prettyPrintSize(afterSize)];
}),
);
console.log();
}