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

Add UI for comparing backend results #2010

Merged
merged 8 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
3 changes: 3 additions & 0 deletions site/frontend/src/pages/compare/compile/benchmarks.vue
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface BenchmarkProps {
benchmarkMap: CompileBenchmarkMap;
filter: CompileBenchmarkFilter;
stat: string;
showBackend: boolean;
}

const props = defineProps<BenchmarkProps>();
Expand Down Expand Up @@ -77,6 +78,7 @@ const secondaryHasNonRelevant = computed(
:commit-b="data.b"
:stat="stat"
:benchmark-map="benchmarkMap"
:show-backend="showBackend"
>
<template #header>
<Section title="Primary" link="secondary" :linkUp="false"></Section>
Expand All @@ -92,6 +94,7 @@ const secondaryHasNonRelevant = computed(
:commit-b="data.b"
:stat="stat"
:benchmark-map="benchmarkMap"
:show-backend="showBackend"
>
<template #header>
<Section title="Secondary" link="primary" :linkUp="true"></Section>
Expand Down
56 changes: 55 additions & 1 deletion site/frontend/src/pages/compare/compile/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export type CompileBenchmarkFilter = {
binary: boolean;
library: boolean;
};
selfCompareBackend: boolean;
} & BenchmarkFilter;

export const defaultCompileFilter: CompileBenchmarkFilter = {
Expand Down Expand Up @@ -57,6 +58,7 @@ export const defaultCompileFilter: CompileBenchmarkFilter = {
binary: true,
library: true,
},
selfCompareBackend: false,
};

export type Profile = "check" | "debug" | "opt" | "doc";
Expand Down Expand Up @@ -206,5 +208,57 @@ export function createCompileBenchmarkMap(
}

export function testCaseKey(testCase: CompileTestCase): string {
return `${testCase.benchmark};${testCase.profile};${testCase.scenario};${testCase.category}`;
return `${testCase.benchmark};${testCase.profile};${testCase.scenario};${testCase.backend};${testCase.category}`;
}

// Transform compile comparisons to compare LLVM vs Cranelift, instead of
// before/after. Assumes that the data comes from the same commit.
export function transformDataForBackendComparison(
comparisons: CompileBenchmarkComparison[]
): CompileBenchmarkComparison[] {
const benchmarkMap: Map<
string,
{
llvm: number | null;
cranelift: number | null;
benchmark: string;
profile: Profile;
scenario: string;
}
> = new Map();
for (const comparison of comparisons) {
const key = `${comparison.benchmark};${comparison.profile};${comparison.scenario}`;
if (!benchmarkMap.has(key)) {
benchmarkMap.set(key, {
llvm: null,
cranelift: null,
benchmark: comparison.benchmark,
profile: comparison.profile,
scenario: comparison.scenario,
});
}
const record = benchmarkMap.get(key);
if (comparison.backend === "llvm") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we normalize the case here somewhere I guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't, the name comes from here.

record.llvm = comparison.comparison.statistics[0];
} else if (comparison.backend === "cranelift") {
record.cranelift = comparison.comparison.statistics[0];
}
}

return Array.from(benchmarkMap, ([_, entry]) => {
const comparison: CompileBenchmarkComparison = {
benchmark: entry.benchmark,
profile: entry.profile,
scenario: entry.scenario,
// Treat LLVM as the baseline
backend: "llvm",
comparison: {
statistics: [entry.llvm, entry.cranelift],
is_relevant: true,
significance_factor: 1.0,
significance_threshold: 1.0,
},
};
return comparison;
});
}
66 changes: 65 additions & 1 deletion site/frontend/src/pages/compare/compile/compile-page.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
computeCompileComparisonsWithNonRelevant,
createCompileBenchmarkMap,
defaultCompileFilter,
transformDataForBackendComparison,
} from "./common";
import {BenchmarkInfo} from "../../../api";
import {importantCompileMetrics} from "../metrics";
Expand Down Expand Up @@ -101,6 +102,11 @@ function loadFilterFromUrl(
defaultFilter.artifact.library
),
},
selfCompareBackend: getBoolOrDefault(
urlParams,
"selfCompareBackend",
defaultFilter.selfCompareBackend
),
};
}

Expand Down Expand Up @@ -172,6 +178,11 @@ function storeFilterToUrl(
filter.artifact.library,
defaultFilter.artifact.library
);
storeOrReset(
"selfCompareBackend",
filter.selfCompareBackend,
defaultFilter.selfCompareBackend
);

changeUrl(urlParams);
}
Expand All @@ -182,6 +193,13 @@ function updateFilter(newFilter: CompileBenchmarkFilter) {
refreshQuickLinks();
}

// We pass the event target here, because Parcel cannot handle the `as`
// cast directly in the template.
function updateSelfCompareBackend(target: EventTarget) {
const element = target as HTMLInputElement;
updateFilter({...filter.value, selfCompareBackend: element.checked});
}

/**
* When the filter changes, the URL is updated.
* After that happens, we want to re-render the quick links component, because
Expand All @@ -197,15 +215,39 @@ const urlParams = getUrlParams();
const quickLinksKey = ref(0);
const filter = ref(loadFilterFromUrl(urlParams, defaultCompileFilter));

// Should we use the backend as the source of before/after data?
const selfCompareBackend = computed(() => {
const hasMultipleBackends =
new Set(props.data.compile_comparisons.map((c) => c.backend)).size > 1;
return (
comparesIdenticalCommits.value &&
filter.value.selfCompareBackend &&
hasMultipleBackends
);
});

function exportData() {
exportToMarkdown(comparisons.value, filter.value.showRawData);
}

// Are we currently comparing the same commit in the before/after toolchains?
const comparesIdenticalCommits = computed(() => {
return props.data.a.commit === props.data.b.commit;
});
const benchmarkMap = createCompileBenchmarkMap(props.data);

const compileComparisons = computed(() => {
// If requested, artificially restructure the data to create a comparison between backends
if (selfCompareBackend.value) {
return transformDataForBackendComparison(props.data.compile_comparisons);
} else {
return props.data.compile_comparisons;
}
});
const allComparisons = computed(() =>
computeCompileComparisonsWithNonRelevant(
filter.value,
props.data.compile_comparisons,
compileComparisons.value,
benchmarkMap
)
);
Expand All @@ -222,6 +264,17 @@ const filteredSummary = computed(() => computeSummary(comparisons.value));
:selected-metric="selector.stat"
:metrics="benchmarkInfo.compile_metrics"
/>
<div
v-if="comparesIdenticalCommits"
Kobzol marked this conversation as resolved.
Show resolved Hide resolved
:title="`Compare codegen backends for commit '${props.data.a.commit}'`"
>
Compare codegen backends for this commit:
<input
type="checkbox"
:checked="selfCompareBackend"
@change="(e) => updateSelfCompareBackend(e.target)"
/>
</div>
<Filters
:defaultFilter="defaultCompileFilter"
:initialFilter="filter"
Expand All @@ -230,12 +283,23 @@ const filteredSummary = computed(() => computeSummary(comparisons.value));
/>
<OverallSummary :summary="filteredSummary" />
<Aggregations :cases="comparisons" />
<div class="warning" v-if="selfCompareBackend">
Note: comparing results of the baseline LLVM backend to the Cranelift
backend.
</div>
<Benchmarks
:data="data"
:test-cases="comparisons"
:all-test-cases="allComparisons"
:filter="filter"
:stat="selector.stat"
:benchmark-map="benchmarkMap"
:show-backend="!selfCompareBackend"
></Benchmarks>
</template>
<style lang="scss" scoped>
.warning {
color: red;
font-weight: bold;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const props = defineProps<{
commitA: ArtifactDescription;
commitB: ArtifactDescription;
stat: string;
showBackend: boolean;
}>();

function prettifyRawNumber(number: number): string {
Expand Down Expand Up @@ -56,7 +57,7 @@ const unit = computed(() => {
<th>Benchmark</th>
<th>Profile</th>
<th>Scenario</th>
<th>Backend</th>
<th v-if="showBackend">Backend</th>
<th>% Change</th>
<th class="narrow">
Significance Threshold
Expand Down Expand Up @@ -95,7 +96,7 @@ const unit = computed(() => {
{{ comparison.testCase.profile }}
</td>
<td>{{ comparison.testCase.scenario }}</td>
<td>{{ comparison.testCase.backend }}</td>
<td v-if="showBackend">{{ comparison.testCase.backend }}</td>
<td>
<div class="numeric-aligned">
<span v-bind:class="percentClass(comparison.percent)">
Expand Down
Loading