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.

Sign up for GitHub

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

Jump to bottom

feat: Allow specifying whether the Brillig diff is bytecode size in bytes or opcode count #6

Merged
merged 3 commits into from
Sep 25, 2024
Merged

feat: Allow specifying whether the Brillig diff is bytecode size in bytes or opcode count #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8da4b0f
allow specifying whether we have bytecode size in bytes or opcodes
vezenovm Sep 24, 2024
ef8aaf4
add brillig_report_bytes flag to action
vezenovm Sep 25, 2024
ca7ea34
check numDiffs for posting markdown not diffCircuitRows.length
vezenovm Sep 25, 2024
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
3 changes: 3 additions & 0 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ inputs:
brillig_report:
description: States whether we want to generate a report of ACIR opcodes or Brillig opcodes.
default: false
brillig_report_bytes:
description: States whether the Brillig report is done with bytecode sizes rather than opcodes.
default: false
# sortCriteria:
# description: The list of criteria to order diff rows by in the report (name | min | avg | median | max | calls), separated by a comma. Must have the same length as sortOrders.
# required: false
Expand Down
21 changes: 15 additions & 6 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ const formatShellBrilligRows = (diffs, summaryQuantile = 0.8) => {
return [summaryRows, fullReportRows];
};
exports.formatShellBrilligRows = formatShellBrilligRows;
const formatShellDiffBrillig = (diffs, summaryRows, fullReportRows, summaryQuantile = 0.8) => {
const formatShellDiffBrillig = (diffs, summaryRows, fullReportRows, brillig_report_bytes, summaryQuantile = 0.8) => {
const maxProgramLength = Math.max(8, ...diffs.map(({ name }) => name.length));
const SHELL_SUMMARY_COLS = [
{ txt: "", length: 0 },
Expand All @@ -128,6 +128,10 @@ const formatShellDiffBrillig = (diffs, summaryRows, fullReportRows, summaryQuant
{ txt: "Brillig opcodes (+/-)", length: 33 },
{ txt: "", length: 0 },
];
if (brillig_report_bytes) {
SHELL_SUMMARY_COLS[2].txt = "Bytecode size in bytes (+/-)";
SHELL_DIFF_COLS[2].txt = "Bytecode size in bytes (+/-)";
}
const summaryHeader = SHELL_SUMMARY_COLS.map((entry) => colors_1.default.bold((0, utils_1.center)(entry.txt, entry.length || 0)))
.join(" | ")
.trim();
Expand Down Expand Up @@ -243,10 +247,10 @@ const formatBrilligRows = (diffs, summaryQuantile = 0.8) => {
exports.formatBrilligRows = formatBrilligRows;
const formatMarkdownDiff = (header, repository, commitHash, summaryRows, fullReportRows,
// Flag to distinguish the markdown columns that should be used
circuitReport, refCommitHash, summaryQuantile = 0.8) => {
circuitReport, brillig_report_bytes, refCommitHash, summaryQuantile = 0.8) => {
const diffReport = [header, "", (0, utils_1.generateCommitInfo)(repository, commitHash, refCommitHash)];
if (fullReportRows.length === 0)
return diffReport.concat(["", "### There are no changes in circuit sizes"]).join("\n").trim();
return diffReport.concat(["", "### There are no changes in sizes"]).join("\n").trim();
let MARKDOWN_SUMMARY_COLS;
let MARKDOWN_DIFF_COLS;
if (circuitReport) {
Expand All @@ -256,6 +260,10 @@ circuitReport, refCommitHash, summaryQuantile = 0.8) => {
else {
MARKDOWN_SUMMARY_COLS = MARKDOWN_SUMMARY_COLS_BRILLIG;
MARKDOWN_DIFF_COLS = MARKDOWN_DIFF_COLS_BRILLIG;
if (brillig_report_bytes) {
MARKDOWN_SUMMARY_COLS[2].txt = "Bytecode size in bytes (+/-)";
MARKDOWN_DIFF_COLS[2].txt = "Bytecode size in bytes (+/-)";
}
}
const summaryHeader = MARKDOWN_SUMMARY_COLS.map((entry) => entry.txt)
.join(" | ")
Expand Down Expand Up @@ -396,6 +404,7 @@ const token = process.env.GITHUB_TOKEN || core.getInput("token");
const report = core.getInput("report");
const header = core.getInput("header");
const brillig_report = core.getInput("brillig_report");
const brillig_report_bytes = core.getInput("brillig_report_bytes");
const summaryQuantile = parseFloat(core.getInput("summaryQuantile"));
// const sortCriteria = core.getInput("sortCriteria").split(",");
// const sortOrders = core.getInput("sortOrders").split(",");
Expand Down Expand Up @@ -512,13 +521,13 @@ function run() {
}
core.info(`Format markdown of ${numDiffs} diffs`);
// const [summaryRows, fullReportRows] = formatCircuitRows(diffCircuitRows, summaryQuantile);
const markdown = (0, program_1.formatMarkdownDiff)(header, repository, github_1.context.sha, summaryRows, fullReportRows, !brillig_report, refCommitHash, summaryQuantile);
const markdown = (0, program_1.formatMarkdownDiff)(header, repository, github_1.context.sha, summaryRows, fullReportRows, !brillig_report, brillig_report_bytes == "true", refCommitHash, summaryQuantile);
core.info(`Format shell of ${numDiffs} diffs`);
let shell;
if (brillig_report) {
core.info(`Format Brillig diffs`);
const [summaryRowsShell, fullReportRowsShell] = (0, program_1.formatShellBrilligRows)(diffBrilligRows, summaryQuantile);
shell = (0, program_1.formatShellDiffBrillig)(diffCircuitRows, summaryRowsShell, fullReportRowsShell, summaryQuantile);
shell = (0, program_1.formatShellDiffBrillig)(diffCircuitRows, summaryRowsShell, fullReportRowsShell, brillig_report_bytes == "true", summaryQuantile);
}
else {
core.info(`Format ACIR diffs`);
Expand All @@ -527,7 +536,7 @@ function run() {
}
core.endGroup();
console.log(shell);
if (diffCircuitRows.length > 0) {
if (numDiffs > 0) {
core.setOutput("shell", shell);
core.setOutput("markdown", markdown);
}
Expand Down
2 changes: 1 addition & 1 deletion dist/index.js.map
View file
Open in desktop

Large diffs are not rendered by default.

13 changes: 12 additions & 1 deletion src/format/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ export const formatShellDiffBrillig = (
diffs: DiffBrillig[],
summaryRows: string[],
fullReportRows: string[],
brillig_report_bytes: boolean,
summaryQuantile = 0.8
) => {
const maxProgramLength = Math.max(8, ...diffs.map(({ name }) => name.length));
Expand All @@ -209,6 +210,11 @@ export const formatShellDiffBrillig = (
{ txt: "", length: 0 },
];

if (brillig_report_bytes) {
SHELL_SUMMARY_COLS[2].txt = "Bytecode size in bytes (+/-)";
SHELL_DIFF_COLS[2].txt = "Bytecode size in bytes (+/-)";
}

const summaryHeader = SHELL_SUMMARY_COLS.map((entry) =>
colors.bold(center(entry.txt, entry.length || 0))
)
Expand Down Expand Up @@ -389,12 +395,13 @@ export const formatMarkdownDiff = (
fullReportRows: string[],
// Flag to distinguish the markdown columns that should be used
circuitReport: boolean,
brillig_report_bytes: boolean,
refCommitHash?: string,
summaryQuantile = 0.8
) => {
const diffReport = [header, "", generateCommitInfo(repository, commitHash, refCommitHash)];
if (fullReportRows.length === 0)
return diffReport.concat(["", "### There are no changes in circuit sizes"]).join("\n").trim();
return diffReport.concat(["", "### There are no changes in sizes"]).join("\n").trim();

let MARKDOWN_SUMMARY_COLS;
let MARKDOWN_DIFF_COLS;
Expand All @@ -404,6 +411,10 @@ export const formatMarkdownDiff = (
} else {
MARKDOWN_SUMMARY_COLS = MARKDOWN_SUMMARY_COLS_BRILLIG;
MARKDOWN_DIFF_COLS = MARKDOWN_DIFF_COLS_BRILLIG;
if (brillig_report_bytes) {
MARKDOWN_SUMMARY_COLS[2].txt = "Bytecode size in bytes (+/-)";
MARKDOWN_DIFF_COLS[2].txt = "Bytecode size in bytes (+/-)";
}
}

const summaryHeader = MARKDOWN_SUMMARY_COLS.map((entry) => entry.txt)
Expand Down
5 changes: 4 additions & 1 deletion src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const token = process.env.GITHUB_TOKEN || core.getInput("token");
const report = core.getInput("report");
const header = core.getInput("header");
const brillig_report = core.getInput("brillig_report");
const brillig_report_bytes = core.getInput("brillig_report_bytes");
const summaryQuantile = parseFloat(core.getInput("summaryQuantile"));
// const sortCriteria = core.getInput("sortCriteria").split(",");
// const sortOrders = core.getInput("sortOrders").split(",");
Expand Down Expand Up @@ -149,6 +150,7 @@ async function run() {
summaryRows,
fullReportRows,
!brillig_report,
brillig_report_bytes == "true",
refCommitHash,
summaryQuantile
);
Expand All @@ -165,6 +167,7 @@ async function run() {
diffCircuitRows,
summaryRowsShell,
fullReportRowsShell,
brillig_report_bytes == "true",
summaryQuantile
);
} else {
Expand All @@ -185,7 +188,7 @@ async function run() {

console.log(shell);

if (diffCircuitRows.length > 0) {
if (numDiffs > 0) {
core.setOutput("shell", shell);
core.setOutput("markdown", markdown);
}
Expand Down
22 changes: 17 additions & 5 deletions tests/program_report.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe("Markdown format", () => {
summaryRows,
fullReportRows,
true,
false,
undefined,
0.8
)
Expand All @@ -70,6 +71,7 @@ describe("Markdown format", () => {
summaryRowsBrillig,
fullReportRowsBrillig,
false,
false,
undefined,
0.8
)
Expand All @@ -92,7 +94,8 @@ describe("Markdown format", () => {
"d62d23148ca73df77cd4378ee1b3c17f1f303dbf",
summaryRows,
fullReportRows,
true
true,
false
)
);

Expand All @@ -106,6 +109,7 @@ describe("Markdown format", () => {
summaryRowsBrillig,
fullReportRowsBrillig,
false,
false,
undefined,
0.8
)
Expand All @@ -125,7 +129,9 @@ describe("Shell format", () => {
console.log(formatShellDiff(circuitDiffs, summaryRows, fullReportRows));

const [summaryRowsBrillig, fullReportRowsBrillig] = formatShellBrilligRows(brilligDiffs);
console.log(formatShellDiffBrillig(brilligDiffs, summaryRowsBrillig, fullReportRowsBrillig));
console.log(
formatShellDiffBrillig(brilligDiffs, summaryRowsBrillig, fullReportRowsBrillig, false)
);
});

it("should compare 1 to 2", () => {
Expand All @@ -139,7 +145,9 @@ describe("Shell format", () => {
console.log(formatShellDiff(circuitDiffs, summaryRows, fullReportRows));

const [summaryRowsBrillig, fullReportRowsBrillig] = formatShellBrilligRows(brilligDiffs);
console.log(formatShellDiffBrillig(brilligDiffs, summaryRowsBrillig, fullReportRowsBrillig));
console.log(
formatShellDiffBrillig(brilligDiffs, summaryRowsBrillig, fullReportRowsBrillig, false)
);
});

// This test is just to make sure that we are accurately resetting our reference
Expand All @@ -165,7 +173,9 @@ describe("Shell format", () => {
console.log(formatShellDiff(circuitDiffs, summaryRows, fullReportRows));

const [summaryRowsBrillig, fullReportRowsBrillig] = formatShellBrilligRows(brilligDiffs);
console.log(formatShellDiffBrillig(brilligDiffs, summaryRowsBrillig, fullReportRowsBrillig));
console.log(
formatShellDiffBrillig(brilligDiffs, summaryRowsBrillig, fullReportRowsBrillig, false)
);
});

it("should compare 2 to 1", () => {
Expand All @@ -179,6 +189,8 @@ describe("Shell format", () => {
console.log(formatShellDiff(circuitDiffs, summaryRows, fullReportRows));

const [summaryRowsBrillig, fullReportRowsBrillig] = formatShellBrilligRows(brilligDiffs);
console.log(formatShellDiffBrillig(brilligDiffs, summaryRowsBrillig, fullReportRowsBrillig));
console.log(
formatShellDiffBrillig(brilligDiffs, summaryRowsBrillig, fullReportRowsBrillig, false)
);
});
});
Loading