Combine measurements from multiple trials and experiments for the sam…

…e code revision (#164)

src/backend/compare/compare.ts

@@ -3,7 +3,7 @@ import { ParameterizedContext } from 'koa';
 import { Database } from '../db/db.js';
 import { completeRequest, startRequest } from '../perf-tracker.js';
 import type {
-  WarmupData,
+  ProfileRow,
   WarmupDataForTrial,
   WarmupDataPerCriterion
 } from '../../shared/view-types.js';
@@ -23,7 +23,7 @@ export async function getProfileAsJson(
 
   ctx.body = await getProfile(
     Number(ctx.params.runId),
-    Number(ctx.params.expId),
+    ctx.params.commitId,
     db
   );
   if (ctx.body === undefined) {
@@ -36,32 +36,35 @@ export async function getProfileAsJson(
 
 async function getProfile(
   runId: number,
-  expId: number,
+  commitId: number,
   db: Database
-): Promise<any> {
+): Promise<ProfileRow[]> {
   const result = await db.query({
-    name: 'fetchProfileDataByRunIdExpId',
+    name: 'fetchProfileDataByRunIdCommitId',
     text: `
-          SELECT substring(commitId, 1, 6) as commitid,
+          SELECT commitid,
             benchmark.name as bench, executor.name as exe, suite.name as suite,
             cmdline, varValue, cores, inputSize, extraArgs,
             invocation, numIterations, warmup, value as profile
           FROM ProfileData
             JOIN Trial ON trialId = Trial.id
-            JOIN Source ON source.id = sourceId
+            JOIN Source ON source.id = trial.sourceId
             JOIN Run ON runId = run.id
             JOIN Suite ON suiteId = suite.id
             JOIN Benchmark ON benchmarkId = benchmark.id
             JOIN Executor ON execId = executor.id
-          WHERE runId = $1 AND trial.expId = $2`,
-    values: [runId, expId]
+          WHERE runId = $1 AND source.commitId = $2`,
+    values: [runId, commitId]
   });
 
-  const data = result.rows[0];
-  try {
-    data.profile = JSON.parse(data.profile);
-  } catch (e) {
-    /* let's just leave it as a string */
+  const data: ProfileRow[] = [];
+  for (const row of result.rows) {
+    try {
+      row.profile = JSON.parse(row.profile);
+    } catch (e) {
+      /* let's just leave it as a string */
+    }
+    data.push(row);
   }
   return data;
 }
@@ -75,8 +78,8 @@ export async function getMeasurementsAsJson(
   ctx.body = await getMeasurements(
     ctx.params.projectSlug,
     Number(ctx.params.runId),
-    Number(ctx.params.expId1),
-    Number(ctx.params.expId2),
+    ctx.params.baseId,
+    ctx.params.changeId,
     db
   );
 
@@ -87,67 +90,52 @@ export async function getMeasurementsAsJson(
 async function getMeasurements(
   projectSlug: string,
   runId: number,
-  expId1: number,
-  expId2: number,
+  baseCommitId: string,
+  changeCommitId: string,
   db: Database
-): Promise<WarmupData | null> {
+): Promise<WarmupDataForTrial[] | null> {
   const q = {
-    name: 'fetchMeasurementsByProjectIdRunIdTrialId',
+    name: 'fetchMeasurementsByProjectIdRunIdCommitId',
     text: `SELECT
-              trialId,
+              trialId, source.commitId as commitId,
               invocation, iteration, warmup,
               criterion.name as criterion,
               criterion.unit as unit,
               value
             FROM
               Measurement
               JOIN Trial ON trialId = Trial.id
+              JOIN Source ON source.id = trial.sourceId
               JOIN Experiment ON Trial.expId = Experiment.id
               JOIN Criterion ON criterion = criterion.id
               JOIN Run ON runId = run.id
               JOIN Project ON Project.id = Experiment.projectId
             WHERE Project.slug = $1
               AND runId = $2
-              AND (Trial.expId = $3 OR Trial.expId = $4)
+              AND (source.commitId = $3 OR source.commitId = $4)
             ORDER BY trialId, criterion, invocation, iteration;`,
-    values: [projectSlug, runId, expId1, expId2]
+    values: [projectSlug, runId, baseCommitId, changeCommitId]
   };
   const result = await db.query(q);
   if (result.rows.length === 0) {
     return null;
   }
 
-  const trial1: WarmupDataForTrial = {
-    trialId: result.rows[0].trialid,
-    warmup: result.rows[0].warmup,
-    data: []
-  };
-  const trial2: WarmupDataForTrial = {
-    trialId: result.rows[result.rows.length - 1].trialid,
-    warmup: result.rows[result.rows.length - 1].warmup,
-    data: []
-  };
-  // preprocess rows, but should already have it like this in the database...
-
-  let trialId = 0;
+  const dataPerTrial: WarmupDataForTrial[] = [];
   let currentTrial: WarmupDataForTrial | null = null;
   let lastCriterion = null;
   let critObject: WarmupDataPerCriterion | null = null;
 
   for (const r of result.rows) {
-    if (trialId === 0) {
-      trialId = r.trialid;
-      currentTrial = trial1;
-      lastCriterion = null;
-    } else if (trialId !== r.trialid) {
-      if (currentTrial === trial2) {
-        throw Error(
-          'Unexpected trialId change. We only expect two different ones.'
-        );
-      }
-      trialId = r.trialid;
-      currentTrial = trial2;
+    if (currentTrial === null || currentTrial.trialId !== r.trialid) {
+      currentTrial = {
+        trialId: r.trialid,
+        warmup: r.warmup,
+        commitId: r.commitid,
+        data: []
+      };
       lastCriterion = null;
+      dataPerTrial.push(currentTrial);
     }
 
     if (lastCriterion === null || lastCriterion !== r.criterion) {
@@ -161,6 +149,7 @@ async function getMeasurements(
     }
 
     if (critObject) {
+      // this is fine, because we separate the data by trialId
       if (critObject.values[r.invocation - 1] === undefined) {
         critObject.values[r.invocation - 1] = [];
       }
@@ -169,7 +158,7 @@ async function getMeasurements(
     }
   }
 
-  return { trial1, trial2 };
+  return dataPerTrial;
 }
 
 export async function getTimelineDataAsJson(

src/backend/compare/db-data.ts

@@ -31,6 +31,11 @@ export function collateMeasurements(
   const runSettings = new Map<string, RunSettings>();
   const criteria = new Map<string, CriterionData>();
 
+  let lastInvocation = 0;
+  let lastTrialId = -1;
+  let lastMeasurements: Measurements | null = null;
+  let lastValues: number[] = [];
+
   for (const row of data) {
     const c = `${row.criterion}|${row.unit}`;
 
@@ -72,21 +77,35 @@ export function collateMeasurements(
       forExeBySuiteBench.set(row.suite, forSuiteByBench);
     }
 
-    const benchResult = findOrConstructProcessedResult(forSuiteByBench, row);
-
-    const m: Measurements = findOrConstructMeasurements(
-      benchResult,
-      row,
-      criterion,
-      runSetting,
-      forSuiteByBench
-    );
+    if (
+      lastMeasurements === null ||
+      !isSameInvocation(row, lastMeasurements, lastInvocation, lastTrialId)
+    ) {
+      const benchResult = findOrConstructProcessedResult(forSuiteByBench, row);
+
+      const m: Measurements = findOrConstructMeasurements(
+        benchResult,
+        row,
+        criterion,
+        runSetting,
+        forSuiteByBench
+      );
 
-    // adjust invocation and iteration to be zero-based
-    if (!m.values[row.invocation - 1]) {
-      m.values[row.invocation - 1] = [];
+      // We don't store the invocation number anymore
+      // I think this is fine, we don't really need it.
+      // We just need to distinguish iterations, but their ordering
+      // doesn't have any particular meaning.
+      // If we should need it for statistical analysis of inter-invocation
+      // effects, we may need to re-introduce it.
+      lastValues = [];
+      m.values.push(lastValues);
+      lastInvocation = row.invocation;
+      lastMeasurements = m;
+      lastTrialId = row.trialid;
     }
-    m.values[row.invocation - 1][row.iteration - 1] = row.value;
+
+    // adjusted to be zero-based
+    lastValues[row.iteration - 1] = row.value;
   }
 
   return sortResultsAlphabetically(byExeSuiteBench);
@@ -162,9 +181,7 @@ function findOrConstructMeasurements(
     envId: row.envid,
     commitId: row.commitid,
     runSettings: runSetting,
-    runId: row.runid,
-    trialId: row.trialid,
-    expId: row.expid
+    runId: row.runid
   };
   benchResult.measurements.push(m);
   forSuiteByBench.criteria[criterion.name] = criterion;
@@ -179,18 +196,32 @@ function findMeasurements(
   benchResult: ProcessedResult,
   row: MeasurementData
 ): Measurements | null {
-  let m: Measurements | null = null;
   for (const mm of benchResult.measurements) {
-    if (
-      mm.envId == row.envid &&
-      mm.commitId == row.commitid &&
-      mm.runId == row.runid &&
-      mm.trialId == row.trialid &&
-      mm.criterion.name == row.criterion
-    ) {
-      m = mm;
-      break;
+    if (isSameMeasurements(row, mm)) {
+      return mm;
     }
   }
-  return m;
+  return null;
+}
+
+function isSameMeasurements(row: MeasurementData, m: Measurements) {
+  return (
+    m.envId == row.envid &&
+    m.commitId == row.commitid &&
+    m.runId == row.runid &&
+    m.criterion.name == row.criterion
+  );
+}
+
+function isSameInvocation(
+  row: MeasurementData,
+  m: Measurements,
+  invocation: number,
+  trialId: number
+) {
+  return (
+    invocation == row.invocation &&
+    trialId == row.trialid &&
+    isSameMeasurements(row, m)
+  );
 }

src/backend/compare/html/stats-row-buttons-info.html

@@ -11,14 +11,12 @@
 {% }
 
    if (d.hasWarmup) {
-%}<button type="button" class="btn btn-sm btn-light btn-warmup" data-content="{%= f.dataSeriesIds(d.dataSeries, d.dataSeries.runId, d.dataSeries.base.expId, d.dataSeries.change.expId) %}"></button>
+%}<button type="button" class="btn btn-sm btn-light btn-warmup" data-content="{%= d.runId %}"></button>
 {%
    } 
 
-   if (d.profileBase) {
-      const baseExpId = d.profileBase.expid;
-      const changeExpId = d.profileChange.expid;
-%}<button type="button" class="btn btn-sm btn-profile" data-content="{%= f.dataSeriesIds(d.dataSeries, d.profileBase.runid, baseExpId, changeExpId) %}"></button>
+   if (d.profiles) {
+%}<button type="button" class="btn btn-sm btn-profile" data-content="{%= d.runId %}"></button>
 {%
    }
 %}<button type="button" class="btn btn-sm btn-timeline" data-content='{%- JSON.stringify(

src/backend/compare/prep-data.ts

@@ -26,8 +26,7 @@ import {
   CompareNavPartial,
   CompareStatsRow,
   CompareStatsTable,
-  CompareViewWithData,
-  DataSeriesVersionComparison
+  CompareViewWithData
 } from '../../shared/view-types.js';
 import {
   calculateInlinePlotHeight,
@@ -236,15 +235,15 @@ function addOrGetCompareStatsRow(
   let row = variants.get(key);
   if (row === undefined) {
     const benchId = { b, e, s, v, c, i, ea };
-    const profileIds = hasProfiles ? hasProfiles.get(benchId) : false;
+    const hasProfile = hasProfiles ? hasProfiles.has(benchId) : false;
 
     row = {
       benchId,
       details: {
         cmdline: measurements.runSettings.simplifiedCmdline,
         envId,
-        profileBase: profileIds ? profileIds[0] : false,
-        profileChange: profileIds && profileIds[1] ? profileIds[1] : false,
+        runId: measurements.runId,
+        profiles: hasProfile,
         hasWarmup: false,
         numV: countsAndMissing === null ? 0 : countsAndMissing.numV,
         numC: countsAndMissing === null ? 0 : countsAndMissing.numC,
@@ -453,30 +452,6 @@ export async function calculateChangeStatsForBenchmark(
   };
 }
 
-function getDataSeriesIds(
-  base: Measurements,
-  change: Measurements
-): DataSeriesVersionComparison {
-  if (base.runId !== change.runId) {
-    throw new Error(
-      `runIds are expected to be the same but` +
-        ` are: ${base.runId} and ${change.runId}` +
-        ` for ${base.criterion.name}`
-    );
-  }
-  return {
-    runId: base.runId,
-    base: {
-      commitId: base.commitId,
-      expId: base.expId
-    },
-    change: {
-      commitId: change.commitId,
-      expId: change.expId
-    }
-  };
-}
-
 export function getMsFlattenedAndSorted(
   base: Measurements,
   change: Measurements
@@ -525,13 +500,6 @@ async function computeStatisticsAndInlinePlot(
     // add the various details
     row.details.hasWarmup = siteConfig.canShowWarmup(change.values);
 
-    if (
-      (row.details.hasWarmup || row.details.profileBase) &&
-      !row.details.dataSeries
-    ) {
-      row.details.dataSeries = getDataSeriesIds(base, change);
-    }
-
     if (!row.versionStats) {
       row.versionStats = {};
     }