perf(utils): improve the performance of scoring and reporting (#212)

MishaSeredenkoPushBased · web-flow · commit 41d7c0b442b7 · 2023-11-14T12:42:29.000+01:00
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
diff --git a/packages/core/src/lib/implementation/persist.ts b/packages/core/src/lib/implementation/persist.ts
@@ -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);
diff --git a/packages/utils/perf/implementations/optimized3.mjs b/packages/utils/perf/implementations/optimized3.mjs
@@ -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;
+}
diff --git a/packages/utils/perf/index.mjs b/packages/utils/perf/index.mjs
@@ -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
@@ -59,6 +60,7 @@ suite.add('scoreReport', _scoreReport);
 suite.add('scoreReportOptimized0', _scoreReportOptimized0);
 suite.add('scoreReportOptimized1', _scoreReportOptimized1);
 suite.add('scoreReportOptimized2', _scoreReportOptimized2);
+suite.add('scoreReportOptimized3', _scoreReportOptimized3);
 
 // ==================
 
@@ -109,6 +111,10 @@ function _scoreReportOptimized2() {
   scoreReportOptimized2(minimalReport());
 }
 
+function _scoreReportOptimized3() {
+  scoreReportOptimized3(minimalReport());
+}
+
 // ==============================================================
 
 function minimalReport(opt) {
diff --git a/packages/utils/src/lib/report.ts b/packages/utils/src/lib/report.ts
@@ -1,5 +1,6 @@
 import { join } from 'path';
 import {
+  AuditGroup,
   CategoryRef,
   IssueSeverity as CliIssueSeverity,
   Format,
@@ -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);
diff --git a/packages/utils/src/lib/scoring.ts b/packages/utils/src/lib/scoring.ts
@@ -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 };
@@ -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;
 }
diff --git a/packages/utils/src/lib/transformation.spec.ts b/packages/utils/src/lib/transformation.spec.ts
@@ -1,6 +1,7 @@
 import { describe, expect, it } from 'vitest';
 import {
   countOccurrences,
+  deepClone,
   distinct,
   objectToEntries,
   objectToKeys,
@@ -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);
+  });
+});
diff --git a/packages/utils/src/lib/transformation.ts b/packages/utils/src/lib/transformation.ts
