Merge pull request #6 from noir-lang/mv/specify-bytes-in-brillig-diff

feat: Allow specifying whether the Brillig diff is bytecode size in bytes or opcode count
noir-lang · Sep 25, 2024 · d88f752 · d88f752
2 parents 3fb8440 + ca7ea34
commit d88f752
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 14 deletions.
diff --git a/action.yml b/action.yml
@@ -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

diff --git a/dist/index.js b/dist/index.js
@@ -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 },
@@ -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();
@@ -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) {
@@ -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(" | ")
@@ -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(",");
@@ -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`);
@@ -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);
             }

diff --git a/dist/index.js.map b/dist/index.js.map
diff --git a/src/format/program.ts b/src/format/program.ts
@@ -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));
@@ -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))
   )
@@ -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;
@@ -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)

diff --git a/src/index.ts b/src/index.ts
@@ -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(",");
@@ -149,6 +150,7 @@ async function run() {
       summaryRows,
       fullReportRows,
       !brillig_report,
+      brillig_report_bytes == "true",
       refCommitHash,
       summaryQuantile
     );
@@ -165,6 +167,7 @@ async function run() {
         diffCircuitRows,
         summaryRowsShell,
         fullReportRowsShell,
+        brillig_report_bytes == "true",
         summaryQuantile
       );
     } else {
@@ -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);
     }

diff --git a/tests/program_report.test.ts b/tests/program_report.test.ts
@@ -55,6 +55,7 @@ describe("Markdown format", () => {
         summaryRows,
         fullReportRows,
         true,
+        false,
         undefined,
         0.8
       )
@@ -70,6 +71,7 @@ describe("Markdown format", () => {
         summaryRowsBrillig,
         fullReportRowsBrillig,
         false,
+        false,
         undefined,
         0.8
       )
@@ -92,7 +94,8 @@ describe("Markdown format", () => {
         "d62d23148ca73df77cd4378ee1b3c17f1f303dbf",
         summaryRows,
         fullReportRows,
-        true
+        true,
+        false
       )
     );
 
@@ -106,6 +109,7 @@ describe("Markdown format", () => {
         summaryRowsBrillig,
         fullReportRowsBrillig,
         false,
+        false,
         undefined,
         0.8
       )
@@ -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", () => {
@@ -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
@@ -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", () => {
@@ -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)
+    );
   });
 });