Skip to content

Commit

Permalink
Merge pull request #3 from vezenovm/mv/fix-program-diff-computation
Browse files Browse the repository at this point in the history
fix: Compute program diffs based off of package name and reset reference report
  • Loading branch information
TomAFrench authored Apr 25, 2024
2 parents 017a479 + 6fff3e2 commit 1931aaa
Show file tree
Hide file tree
Showing 9 changed files with 281 additions and 183 deletions.
47 changes: 34 additions & 13 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ const core = __importStar(__nccwpck_require__(2186));
const github_1 = __nccwpck_require__(5438);
const program_1 = __nccwpck_require__(1578);
const report_1 = __nccwpck_require__(8269);
// import { isSortCriteriaValid, isSortOrdersValid } from "./types";
const token = process.env.GITHUB_TOKEN || core.getInput("token");
const report = core.getInput("report");
const header = core.getInput("header");
Expand Down Expand Up @@ -387,13 +386,15 @@ function run() {
core.info(`Loading gas reports from "${localReportPath}"`);
const compareContent = fs.readFileSync(localReportPath, "utf8");
referenceContent !== null && referenceContent !== void 0 ? referenceContent : (referenceContent = compareContent); // if no source gas reports were loaded, defaults to the current gas reports
core.info(`Mapping reference gas reports`);
const referenceReports = (0, report_1.loadReports)(referenceContent);
core.info(`Mapping compared gas reports`);
const compareReports = (0, report_1.loadReports)(compareContent);
core.info(`Got ${compareReports.programs.length} compare programs`);
core.info(`Mapping reference gas reports`);
const referenceReports = (0, report_1.loadReports)(referenceContent);
core.info(`Got ${compareReports.programs.length} reference programs`);
core.endGroup();
core.startGroup("Compute gas diff");
const diffRows = (0, report_1.computeProgramDiffs)(referenceReports.programs[0].functions, compareReports.programs[0].functions);
const diffRows = (0, report_1.computeProgramDiffs)(referenceReports.programs, compareReports.programs);
core.info(`Format markdown of ${diffRows.length} diffs`);
const markdown = (0, program_1.formatMarkdownDiff)(header, diffRows, repository, github_1.context.sha, refCommitHash, summaryQuantile);
core.info(`Format shell of ${diffRows.length} diffs`);
Expand Down Expand Up @@ -451,30 +452,33 @@ const loadReports = (content) => {
};
exports.loadReports = loadReports;
const computedWorkspaceDiff = (sourceReport, compareReport) => ({
programs: (0, exports.computeProgramDiffs)(sourceReport.programs[0].functions, compareReport.programs[0].functions),
programs: (0, exports.computeProgramDiffs)(sourceReport.programs, compareReport.programs),
contracts: (0, exports.computeContractDiffs)(sourceReport.contracts, compareReport.contracts),
});
exports.computedWorkspaceDiff = computedWorkspaceDiff;
const computeProgramDiffs = (sourceReports, compareReports) => {
const sourceReportNames = sourceReports.map((report) => report.name);
const sourceReportNames = sourceReports.map((report) => report.package_name);
const commonReportNames = compareReports
.map((report) => report.name)
.map((report) => report.package_name)
.filter((name) => sourceReportNames.includes(name));
return commonReportNames
.map((reportName) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const srcReport = sourceReports.find((report) => report.name == reportName);
const srcReport = sourceReports.find((report) => report.package_name == reportName);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const cmpReport = compareReports.find((report) => report.name == reportName);
return computeProgramDiff(srcReport, cmpReport);
const cmpReport = compareReports.find((report) => report.package_name == reportName);
// For now we fetch just the main of each program
return computeCircuitDiff(srcReport.functions[0], cmpReport.functions[0], reportName);
})
.filter((diff) => !isEmptyDiff(diff))
.sort((diff1, diff2) => Math.max(diff2.circuit_size.percentage) - Math.max(diff1.circuit_size.percentage));
};
exports.computeProgramDiffs = computeProgramDiffs;
const computeProgramDiff = (sourceReport, compareReport) => {
const computeCircuitDiff = (sourceReport, compareReport,
// We want the name of the package that represents the entire program in our report
reportName) => {
return {
name: sourceReport.name,
name: reportName,
acir_opcodes: (0, exports.variation)(compareReport.acir_opcodes, sourceReport.acir_opcodes),
circuit_size: (0, exports.variation)(compareReport.circuit_size, sourceReport.circuit_size),
};
Expand All @@ -499,7 +503,24 @@ const computeContractDiffs = (sourceReports, compareReports) => {
};
exports.computeContractDiffs = computeContractDiffs;
const computeContractDiff = (sourceReport, compareReport) => {
const functionDiffs = (0, exports.computeProgramDiffs)(sourceReport.functions[0].functions, compareReport.functions[0].functions);
// TODO(https://github.com/noir-lang/noir/issues/4720): Settle on how to display contract functions with non-inlined Acir calls
// Right now we assume each contract function does not have non-inlined functions.
// Thus, we simply re-assign each `CircuitReport` to a `ProgramReport` to easily reuse `computeProgramDiffs`
const sourceFunctionsAsProgram = sourceReport.functions.map((func) => {
const programReport = {
package_name: func.name,
functions: [func],
};
return programReport;
});
const compareFunctionsAsProgram = compareReport.functions.map((func) => {
const programReport = {
package_name: func.name,
functions: [func],
};
return programReport;
});
const functionDiffs = (0, exports.computeProgramDiffs)(sourceFunctionsAsProgram, compareFunctionsAsProgram);
return {
name: sourceReport.name,
functions: functionDiffs,
Expand Down
2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import { context, getOctokit } from "@actions/github";
import { formatMarkdownDiff, formatShellDiff } from "./format/program";
import { loadReports, computeProgramDiffs } from "./report";

// import { isSortCriteriaValid, isSortOrdersValid } from "./types";

const token = process.env.GITHUB_TOKEN || core.getInput("token");
const report = core.getInput("report");
const header = core.getInput("header");
Expand Down Expand Up @@ -106,10 +104,13 @@ async function run() {
const compareContent = fs.readFileSync(localReportPath, "utf8");
referenceContent ??= compareContent; // if no source gas reports were loaded, defaults to the current gas reports

core.info(`Mapping reference gas reports`);
const referenceReports = loadReports(referenceContent);
core.info(`Mapping compared gas reports`);
const compareReports = loadReports(compareContent);
core.info(`Got ${compareReports.programs.length} compare programs`);

core.info(`Mapping reference gas reports`);
const referenceReports = loadReports(referenceContent);
core.info(`Got ${compareReports.programs.length} reference programs`);
core.endGroup();

core.startGroup("Compute gas diff");
Expand Down
51 changes: 33 additions & 18 deletions src/report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
CircuitReport,
WorkspaceDiffReport,
WorkspaceReport,
ProgramReport,
} from "./types";

export const variation = (current: number, previous: number) => {
Expand All @@ -28,30 +29,28 @@ export const computedWorkspaceDiff = (
sourceReport: WorkspaceReport,
compareReport: WorkspaceReport
): WorkspaceDiffReport => ({
programs: computeProgramDiffs(
sourceReport.programs[0].functions,
compareReport.programs[0].functions
),
programs: computeProgramDiffs(sourceReport.programs, compareReport.programs),
contracts: computeContractDiffs(sourceReport.contracts, compareReport.contracts),
});

export const computeProgramDiffs = (
sourceReports: CircuitReport[],
compareReports: CircuitReport[]
sourceReports: ProgramReport[],
compareReports: ProgramReport[]
): DiffProgram[] => {
const sourceReportNames = sourceReports.map((report) => report.name);
const sourceReportNames = sourceReports.map((report) => report.package_name);
const commonReportNames = compareReports
.map((report) => report.name)
.map((report) => report.package_name)
.filter((name) => sourceReportNames.includes(name));

return commonReportNames
.map((reportName) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const srcReport = sourceReports.find((report) => report.name == reportName)!;
const srcReport = sourceReports.find((report) => report.package_name == reportName)!;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const cmpReport = compareReports.find((report) => report.name == reportName)!;
const cmpReport = compareReports.find((report) => report.package_name == reportName)!;

return computeProgramDiff(srcReport, cmpReport);
// For now we fetch just the main of each program
return computeCircuitDiff(srcReport.functions[0], cmpReport.functions[0], reportName);
})
.filter((diff) => !isEmptyDiff(diff))
.sort(
Expand All @@ -60,12 +59,14 @@ export const computeProgramDiffs = (
);
};

const computeProgramDiff = (
const computeCircuitDiff = (
sourceReport: CircuitReport,
compareReport: CircuitReport
compareReport: CircuitReport,
// We want the name of the package that represents the entire program in our report
reportName: string
): DiffProgram => {
return {
name: sourceReport.name,
name: reportName,
acir_opcodes: variation(compareReport.acir_opcodes, sourceReport.acir_opcodes),
circuit_size: variation(compareReport.circuit_size, sourceReport.circuit_size),
};
Expand Down Expand Up @@ -108,10 +109,24 @@ const computeContractDiff = (
sourceReport: ContractReport,
compareReport: ContractReport
): ContractDiffReport => {
const functionDiffs = computeProgramDiffs(
sourceReport.functions[0].functions,
compareReport.functions[0].functions
);
// TODO(https://github.com/noir-lang/noir/issues/4720): Settle on how to display contract functions with non-inlined Acir calls
// Right now we assume each contract function does not have non-inlined functions.
// Thus, we simply re-assign each `CircuitReport` to a `ProgramReport` to easily reuse `computeProgramDiffs`
const sourceFunctionsAsProgram = sourceReport.functions.map((func) => {
const programReport: ProgramReport = {
package_name: func.name,
functions: [func],
};
return programReport;
});
const compareFunctionsAsProgram = compareReport.functions.map((func) => {
const programReport: ProgramReport = {
package_name: func.name,
functions: [func],
};
return programReport;
});
const functionDiffs = computeProgramDiffs(sourceFunctionsAsProgram, compareFunctionsAsProgram);

return {
name: sourceReport.name,
Expand Down
5 changes: 4 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ export interface CircuitReport {
}

export interface ProgramReport {
// Name of the program package
package_name: string;
functions: CircuitReport[];
}

export interface ContractReport {
name: string;
functions: ProgramReport[];
// TODO(https://github.com/noir-lang/noir/issues/4720): Settle on how to display contract functions with non-inlined Acir calls
functions: CircuitReport[];
}

export interface WorkspaceReport {
Expand Down
1 change: 1 addition & 0 deletions tests/mocks/full_gas_report.json

Large diffs are not rendered by default.

Loading

0 comments on commit 1931aaa

Please sign in to comment.