Skip to content

Commit

Permalink
perf(utils): improve the performance of scoring and reporting (#212)
Browse files Browse the repository at this point in the history
Improved the performance of scoring and reporting.
Removed nested loops where possible, added dictionaries to go constant
time O(1) instead of linear O(n).
Made minor refactoring, added some checks.
@BioPhoton As task owner, let me know if I understood the task correctly
and implemented everything properly. Also, maybe you could propose
additional changes for further performance improvements.

Closes #132
  • Loading branch information
MishaSeredenkoPushBased committed Nov 14, 2023
1 parent ce4d975 commit 41d7c0b
Show file tree
Hide file tree
Showing 7 changed files with 223 additions and 95 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/lib/implementation/persist.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export async function persistReport(
const { persist } = config;
const outputDir = persist.outputDir;
const filename = persist.filename;
let { format } = persist;
format = format && format.length !== 0 ? format : ['stdout'];
const format =
persist.format && persist.format.length !== 0 ? persist.format : ['stdout'];
let scoredReport;
if (format.includes('stdout')) {
scoredReport = scoreReport(report);
Expand Down
85 changes: 85 additions & 0 deletions packages/utils/perf/implementations/optimized3.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
export function calculateScore(refs, scoreFn) {
const { numerator, denominator } = refs.reduce(
(acc, ref) => {
const score = scoreFn(ref);
return {
numerator: acc.numerator + score * ref.weight,
denominator: acc.denominator + ref.weight,
};
},
{ numerator: 0, denominator: 0 },
);
return numerator / denominator;
}

export function deepClone(obj) {
if (obj == null || typeof obj !== 'object') {
return obj;
}

const cloned = Array.isArray(obj) ? [] : {};
for (const key in obj) {
if (Object.prototype.hasOwnProperty.call(obj, key)) {
cloned[key] = deepClone(obj[key]);
}
}
return cloned;
}

export function scoreReportOptimized3(report) {
const scoredReport = deepClone(report);
const allScoredAuditsAndGroups = new Map();

scoredReport.plugins.forEach(plugin => {
const { audits } = plugin;
const groups = plugin.groups || [];

audits.forEach(audit => {
const key = `${plugin.slug}-${audit.slug}-audit`;
audit.plugin = plugin.slug;
allScoredAuditsAndGroups.set(key, audit);
});

function groupScoreFn(ref) {
const score = allScoredAuditsAndGroups.get(
`${plugin.slug}-${ref.slug}-audit`,
)?.score;
if (score == null) {
throw new Error(
`Group has invalid ref - audit with slug ${plugin.slug}-${ref.slug}-audit not found`,
);
}
return score;
}

groups.forEach(group => {
const key = `${plugin.slug}-${group.slug}-group`;
group.score = calculateScore(group.refs, groupScoreFn);
group.plugin = plugin.slug;
allScoredAuditsAndGroups.set(key, group);
});
plugin.groups = groups;
});

function catScoreFn(ref) {
const key = `${ref.plugin}-${ref.slug}-${ref.type}`;
const item = allScoredAuditsAndGroups.get(key);
if (!item) {
throw new Error(
`Category has invalid ref - ${ref.type} with slug ${key} not found in ${ref.plugin} plugin`,
);
}
return item.score;
}

const scoredCategoriesMap = new Map();

for (const category of scoredReport.categories) {
category.score = calculateScore(category.refs, catScoreFn);
scoredCategoriesMap.set(category.slug, category);
}

scoredReport.categories = Array.from(scoredCategoriesMap.values());

return scoredReport;
}
6 changes: 6 additions & 0 deletions packages/utils/perf/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { scoreReport } from './implementations/base.mjs';
import { scoreReportOptimized0 } from './implementations/optimized0.mjs';
import { scoreReportOptimized1 } from './implementations/optimized1.mjs';
import { scoreReportOptimized2 } from './implementations/optimized2.mjs';
import { scoreReportOptimized3 } from './implementations/optimized3.mjs';

const PROCESS_ARGUMENT_NUM_AUDITS_P1 = parseInt(
process.argv
Expand Down Expand Up @@ -59,6 +60,7 @@ suite.add('scoreReport', _scoreReport);
suite.add('scoreReportOptimized0', _scoreReportOptimized0);
suite.add('scoreReportOptimized1', _scoreReportOptimized1);
suite.add('scoreReportOptimized2', _scoreReportOptimized2);
suite.add('scoreReportOptimized3', _scoreReportOptimized3);

// ==================

Expand Down Expand Up @@ -109,6 +111,10 @@ function _scoreReportOptimized2() {
scoreReportOptimized2(minimalReport());
}

function _scoreReportOptimized3() {
scoreReportOptimized3(minimalReport());
}

// ==============================================================

function minimalReport(opt) {
Expand Down
36 changes: 28 additions & 8 deletions packages/utils/src/lib/report.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { join } from 'path';
import {
AuditGroup,
CategoryRef,
IssueSeverity as CliIssueSeverity,
Format,
Expand Down Expand Up @@ -126,16 +127,35 @@ export function countCategoryAudits(
refs: CategoryRef[],
plugins: ScoredReport['plugins'],
): number {
// Create lookup object for groups within each plugin
const groupLookup = plugins.reduce<
Record<string, Record<string, AuditGroup>>
>((lookup, plugin) => {
if (!plugin.groups.length) {
return lookup;
}

return {
...lookup,
[plugin.slug]: {
...plugin.groups.reduce<Record<string, AuditGroup>>(
(groupLookup, group) => {
return {
...groupLookup,
[group.slug]: group,
};
},
{},
),
},
};
}, {});

// Count audits
return refs.reduce((acc, ref) => {
if (ref.type === 'group') {
const groupRefs = plugins
.find(({ slug }) => slug === ref.plugin)
?.groups?.find(({ slug }) => slug === ref.slug)?.refs;

if (!groupRefs?.length) {
return acc;
}
return acc + groupRefs.length;
const groupRefs = groupLookup[ref.plugin]?.[ref.slug]?.refs;
return acc + (groupRefs?.length || 0);
}
return acc + 1;
}, 0);
Expand Down
145 changes: 60 additions & 85 deletions packages/utils/src/lib/scoring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
PluginReport,
Report,
} from '@code-pushup/models';
import { deepClone } from './transformation';

type EnrichedAuditReport = AuditReport & { plugin: string };
type ScoredCategoryConfig = CategoryConfig & { score: number };
Expand All @@ -24,103 +25,77 @@ export type ScoredReport = Omit<Report, 'plugins' | 'categories'> & {
categories: ScoredCategoryConfig[];
};

function groupRefToScore(
audits: AuditReport[],
): (ref: AuditGroupRef) => number {
return ref => {
const score = audits.find(audit => audit.slug === ref.slug)?.score;
if (score == null) {
throw new Error(
`Group has invalid ref - audit with slug ${ref.slug} not found`,
);
}
return score;
};
}

function categoryRefToScore(
audits: EnrichedAuditReport[],
groups: EnrichedScoredAuditGroup[],
): (ref: CategoryRef) => number {
return (ref: CategoryRef): number => {
switch (ref.type) {
case 'audit':
// eslint-disable-next-line no-case-declarations
const audit = audits.find(
a => a.slug === ref.slug && a.plugin === ref.plugin,
);
if (!audit) {
throw new Error(
`Category has invalid ref - audit with slug ${ref.slug} not found in ${ref.plugin} plugin`,
);
}
return audit.score;

case 'group':
// eslint-disable-next-line no-case-declarations
const group = groups.find(
g => g.slug === ref.slug && g.plugin === ref.plugin,
);
if (!group) {
throw new Error(
`Category has invalid ref - group with slug ${ref.slug} not found in ${ref.plugin} plugin`,
);
}
return group.score;
default:
throw new Error(`Type ${ref.type} is unknown`);
}
};
}

export function calculateScore<T extends { weight: number }>(
refs: T[],
scoreFn: (ref: T) => number,
): number {
const numerator = refs.reduce(
(sum, ref) => sum + scoreFn(ref) * ref.weight,
0,
const { numerator, denominator } = refs.reduce(
(acc, ref) => {
const score = scoreFn(ref);
return {
numerator: acc.numerator + score * ref.weight,
denominator: acc.denominator + ref.weight,
};
},
{ numerator: 0, denominator: 0 },
);
const denominator = refs.reduce((sum, ref) => sum + ref.weight, 0);
return numerator / denominator;
}

export function scoreReport(report: Report): ScoredReport {
const scoredPlugins = report.plugins.map(plugin => {
const { groups, audits } = plugin;
const preparedAudits = audits.map(audit => ({
...audit,
plugin: plugin.slug,
}));
const preparedGroups =
groups?.map(group => ({
...group,
score: calculateScore(group.refs, groupRefToScore(preparedAudits)),
plugin: plugin.slug,
})) || [];
const scoredReport = deepClone(report) as ScoredReport;
const allScoredAuditsAndGroups = new Map();

scoredReport.plugins?.forEach(plugin => {
const { audits } = plugin;
const groups = plugin.groups || [];

audits.forEach(audit => {
const key = `${plugin.slug}-${audit.slug}-audit`;
audit.plugin = plugin.slug;
allScoredAuditsAndGroups.set(key, audit);
});

function groupScoreFn(ref: AuditGroupRef) {
const score = allScoredAuditsAndGroups.get(
`${plugin.slug}-${ref.slug}-audit`,
)?.score;
if (score == null) {
throw new Error(
`Group has invalid ref - audit with slug ${plugin.slug}-${ref.slug}-audit not found`,
);
}
return score;
}

return {
...plugin,
audits: preparedAudits,
groups: preparedGroups,
};
groups.forEach(group => {
const key = `${plugin.slug}-${group.slug}-group`;
group.score = calculateScore(group.refs, groupScoreFn);
group.plugin = plugin.slug;
allScoredAuditsAndGroups.set(key, group);
});
plugin.groups = groups;
});

// @TODO intro dict to avoid multiple find calls in the scoreFn
const allScoredAudits = scoredPlugins.flatMap(({ audits }) => audits);
const allScoredGroups = scoredPlugins.flatMap(({ groups }) => groups);
function catScoreFn(ref: CategoryRef) {
const key = `${ref.plugin}-${ref.slug}-${ref.type}`;
const item = allScoredAuditsAndGroups.get(key);
if (!item) {
throw new Error(
`Category has invalid ref - ${ref.type} with slug ${key} not found in ${ref.plugin} plugin`,
);
}
return item.score;
}

const scoredCategoriesMap = new Map();
// eslint-disable-next-line functional/no-loop-statements
for (const category of scoredReport.categories) {
category.score = calculateScore(category.refs, catScoreFn);
scoredCategoriesMap.set(category.slug, category);
}

const scoredCategories = report.categories.map(category => ({
...category,
score: calculateScore(
category.refs,
categoryRefToScore(allScoredAudits, allScoredGroups),
),
}));
scoredReport.categories = Array.from(scoredCategoriesMap.values());

return {
...report,
categories: scoredCategories,
plugins: scoredPlugins,
};
return scoredReport;
}
27 changes: 27 additions & 0 deletions packages/utils/src/lib/transformation.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe, expect, it } from 'vitest';
import {
countOccurrences,
deepClone,
distinct,
objectToEntries,
objectToKeys,
Expand Down Expand Up @@ -96,3 +97,29 @@ describe('distinct', () => {
]);
});
});

describe('deepClone', () => {
it('should clone the object with nested array with objects, with null and undefined properties', () => {
const obj = {
a: 1,
b: 2,
c: [
{ d: 3, e: 4 },
{ f: 5, g: 6 },
],
d: null,
e: undefined,
};
const cloned = deepClone(obj);
expect(cloned).toEqual(obj);
expect(cloned).not.toBe(obj);
expect(cloned.c).toEqual(obj.c);
expect(cloned.c).not.toBe(obj.c);
expect(cloned.c[0]).toEqual(obj.c[0]);
expect(cloned.c[0]).not.toBe(obj.c[0]);
expect(cloned.c[1]).toEqual(obj.c[1]);
expect(cloned.c[1]).not.toBe(obj.c[1]);
expect(cloned.d).toBe(obj.d);
expect(cloned.e).toBe(obj.e);
});
});
Loading

0 comments on commit 41d7c0b

Please sign in to comment.