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

fix: Compute program diffs based off of package name and reset reference report #3

Merged
merged 20 commits into from
Apr 25, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading