Skip to content

Commit

Permalink
Need to use full BenchmarkId to identify matching profiles
Browse files Browse the repository at this point in the history
- this is needed because the runId may be different because of the different command line when using a profiler
  • Loading branch information
smarr committed Jun 13, 2023
1 parent 69f42e0 commit 6f6f612
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 33 deletions.
3 changes: 2 additions & 1 deletion src/data-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,13 @@ export function benchmarkId(

export function dataSeriesIds(
ids: DataSeriesVersionComparison,
runId: number,
baseTrialId: number,
changeTrialId: number
): string {
// format is parsed in compare.ts:insertProfiles()
return (
`${ids.runId},${ids.base.commitId}/${baseTrialId},` +
`${runId},${ids.base.commitId}/${baseTrialId},` +
`${ids.change.commitId}/${changeTrialId}`
);
}
68 changes: 49 additions & 19 deletions src/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
TimelineResponse,
PlotData,
FullPlotData,
TimelineSuite
TimelineSuite,
BenchmarkId
} from './api.js';
import pg, { PoolConfig, QueryConfig, QueryResultRow } from 'pg';
import { getDirname, TotalCriterion } from './util.js';
Expand Down Expand Up @@ -202,9 +203,23 @@ export interface MeasurementData {
envid: number;
}

interface AvailableProfile {
runid: number;
export interface AvailableProfile extends BenchmarkId {
trialid: number;
runid: number;
}

function sameBenchId(a: BenchmarkId, b: BenchmarkId): boolean {
// using here == instead of === is intentional
// otherwise we don't equate null and undefined
return (
a.b == b.b &&
a.e == b.e &&
a.s == b.s &&
a.v == b.v &&
a.c == b.c &&
a.i == b.i &&
a.ea == b.ea
);
}

export class HasProfile {
Expand All @@ -218,17 +233,21 @@ export class HasProfile {
this.availableProfiles = availableProfiles;
}

public getTrialId(runId: number): [number, number] | false {
const idx = this.availableProfiles.findIndex((p) => p.runid === runId);
public get(
benchId: BenchmarkId
): [AvailableProfile, AvailableProfile?] | false {
const idx = this.availableProfiles.findIndex((id) =>
sameBenchId(id, benchId)
);
if (idx >= 0) {
assert(
this.availableProfiles[idx].runid === runId &&
this.availableProfiles[idx + 1].runid === runId
);
return [
this.availableProfiles[idx].trialid,
this.availableProfiles[idx + 1].trialid
];
if (
idx === this.availableProfiles.length - 1 ||
!sameBenchId(this.availableProfiles[idx + 1], benchId)
) {
return [this.availableProfiles[idx]];
}

return [this.availableProfiles[idx], this.availableProfiles[idx + 1]];
}
return false;
}
Expand Down Expand Up @@ -1627,17 +1646,28 @@ export abstract class Database {
): Promise<HasProfile> {
const q = {
name: 'fetchProfileAvailability',
text: `SELECT
runId, trialId
FROM ProfileData
JOIN Trial ON trialId = Trial.id
JOIN Experiment e ON expId = e.id
text: `SELECT
benchmark.name as b,
executor.name as e,
suite.name as s,
varValue as v,
cores as c,
inputSize as i,
extraArgs as ea,
trialId, runId
FROM ProfileData pd
JOIN Trial ON pd.trialId = Trial.id
JOIN Experiment e ON trial.expId = e.id
JOIN Source ON source.id = sourceId
JOIN Run ON runId = run.id
JOIN Suite ON run.suiteId = suite.id
JOIN Benchmark ON run.benchmarkId = benchmark.id
JOIN Executor ON execId = executor.id
WHERE
(commitId = $1 OR commitId = $2)
AND e.projectId = $3
ORDER BY
runId, trialId`,
b, e, s, v, c, i, ea, trialId, runId`,
values: [commitId1, commitId2, projectId]
};

Expand Down
10 changes: 5 additions & 5 deletions src/stats-data-prep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,17 +198,17 @@ function addOrGetCompareStatsRow(

let row = variants.get(key);
if (row === undefined) {
const profileIds = hasProfiles
? hasProfiles.getTrialId(measurements.runId)
: false;
const benchId = { b, e, s, v, c, i, ea };
const profileIds = hasProfiles ? hasProfiles.get(benchId) : false;

row = {
benchId: { b, e, s, v, c, i, ea },
benchId,
details: {
cmdline: measurements.runSettings.simplifiedCmdline,
envId,
profileTrialIdBase: profileIds ? profileIds[0] : false,
profileTrialIdChange: profileIds ? profileIds[1] : false,
profileTrialIdChange:
profileIds && profileIds[1] ? profileIds[1] : false,
hasWarmup: false,
numV: countsAndMissing === null ? 0 : countsAndMissing.numV,
numC: countsAndMissing === null ? 0 : countsAndMissing.numC,
Expand Down
6 changes: 4 additions & 2 deletions src/views/compare/stats-row-buttons-info.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@
{% }

if (d.hasWarmup) {
%}<button type="button" class="btn btn-sm btn-light btn-warmup" data-content="{%= f.dataSeriesIds(d.dataSeries, d.dataSeries.base.trialId, d.dataSeries.change.trialId) %}"></button>
%}<button type="button" class="btn btn-sm btn-light btn-warmup" data-content="{%= f.dataSeriesIds(d.dataSeries, d.dataSeries.runId, d.dataSeries.base.trialId, d.dataSeries.change.trialId) %}"></button>
{%
}

if (d.profileTrialIdBase) {
%}<button type="button" class="btn btn-sm btn-profile" data-content="{%= f.dataSeriesIds(d.dataSeries, d.profileTrialIdBase, d.profileTrialIdChange) %}"></button>
const baseTrial = d.profileTrialIdBase.trialid;
const changeTrial = d.profileTrialIdChange.trialid;
%}<button type="button" class="btn btn-sm btn-profile" data-content="{%= f.dataSeriesIds(d.dataSeries, d.profileTrialIdBase.runid, baseTrial, changeTrial) %}"></button>
{%
}
%}<button type="button" class="btn btn-sm btn-timeline" data-content='{%- JSON.stringify(
Expand Down
6 changes: 3 additions & 3 deletions src/views/view-types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BenchmarkId } from 'api';
import { CriterionData, Environment, RevisionData } from 'db';
import { AvailableProfile, CriterionData, Environment, RevisionData } from 'db';
import { ComparisonStatistics, SummaryStatsWithUnit } from 'stats';

declare type DataFormat = typeof import('../data-format');
Expand Down Expand Up @@ -74,8 +74,8 @@ export interface DetailedInfo {

hasWarmup: boolean;

profileTrialIdBase: number | false;
profileTrialIdChange: number | false;
profileTrialIdBase: AvailableProfile | false;
profileTrialIdChange: AvailableProfile | false;

dataSeries?: DataSeriesVersionComparison;

Expand Down
2 changes: 1 addition & 1 deletion tests/data-format.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,6 @@ describe('dataSeriesIds()', () => {
}
};

expect(dataSeriesIds(ids, 2, 4)).toBe('1,123456/2,123457/4');
expect(dataSeriesIds(ids, 1, 2, 4)).toBe('1,123456/2,123457/4');
});
});
4 changes: 2 additions & 2 deletions tests/views/compare-view.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ const environments: Environment[] = [
const details: DetailedInfo = {
cmdline: 'som/some-command with args',
envId: 1,
profileTrialIdBase: 11,
profileTrialIdChange: 12,
profileTrialIdBase: <any>{ trialid: 11, runid: 1 },
profileTrialIdChange: <any>{ trialid: 12, runid: 1 },
hasWarmup: true,
dataSeries: {
runId: 1,
Expand Down

0 comments on commit 6f6f612

Please sign in to comment.