From 008f9b0f26210a11debb1db5d6d874f5d0431eba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 11 Nov 2023 20:08:25 +0100 Subject: [PATCH] Use benchmark detail endpoint in the UI --- site/frontend/src/graph/data.ts | 1 + site/frontend/src/graph/resolver.ts | 29 ------- .../compile/table/benchmark-detail.vue | 75 +++++++++++++++---- .../compare/compile/table/detail-resolver.ts | 65 ++++++++++++++++ site/frontend/src/urls.ts | 1 + 5 files changed, 128 insertions(+), 43 deletions(-) delete mode 100644 site/frontend/src/graph/resolver.ts create mode 100644 site/frontend/src/pages/compare/compile/table/detail-resolver.ts diff --git a/site/frontend/src/graph/data.ts b/site/frontend/src/graph/data.ts index 908429dc6..bb3c0193c 100644 --- a/site/frontend/src/graph/data.ts +++ b/site/frontend/src/graph/data.ts @@ -19,5 +19,6 @@ export interface Series { // Graph data received from the server export interface GraphData { commits: [[number, string]]; + // benchmark -> profile -> scenario -> series benchmarks: Dict>>; } diff --git a/site/frontend/src/graph/resolver.ts b/site/frontend/src/graph/resolver.ts deleted file mode 100644 index 3f455e4ea..000000000 --- a/site/frontend/src/graph/resolver.ts +++ /dev/null @@ -1,29 +0,0 @@ -import {GraphData, GraphsSelector} from "./data"; -import {loadGraphs} from "./api"; - -/** - * Graph API resolver that contains a cache of downloaded graphs. - * This is important for Vue components that download a graph on mount. - * Without a cache, they would download a graph each time they are destroyed - * and recreated. - */ -export class GraphResolver { - private cache: Dict = {}; - - public async loadGraph(selector: GraphsSelector): Promise { - const key = `${selector.benchmark};${selector.profile};${selector.scenario};${selector.start};${selector.end};${selector.stat};${selector.kind}`; - if (!this.cache.hasOwnProperty(key)) { - this.cache[key] = await loadGraphs(selector); - } - - return this.cache[key]; - } -} - -/** - * This is essentially a global variable, but it makes the code simpler and - * since we currently don't have any unit tests, we don't really need to avoid - * global variables that much. If needed, it could be provided to Vue components - * from a parent via props or context. - */ -export const GRAPH_RESOLVER = new GraphResolver(); diff --git a/site/frontend/src/pages/compare/compile/table/benchmark-detail.vue b/site/frontend/src/pages/compare/compile/table/benchmark-detail.vue index 4f2826993..326587c63 100644 --- a/site/frontend/src/pages/compare/compile/table/benchmark-detail.vue +++ b/site/frontend/src/pages/compare/compile/table/benchmark-detail.vue @@ -10,10 +10,10 @@ import Tooltip from "../../tooltip.vue"; import {ArtifactDescription} from "../../types"; import {daysBetweenDates, getFutureDate, getPastDate} from "./utils"; import {GraphRenderOpts, renderPlots} from "../../../../graph/render"; -import {GRAPH_RESOLVER} from "../../../../graph/resolver"; -import {GraphKind} from "../../../../graph/data"; +import {GraphData, GraphKind, GraphsSelector} from "../../../../graph/data"; import uPlot from "uplot"; import CachegrindCmd from "../../../../components/cachegrind-cmd.vue"; +import {COMPILE_DETAIL_RESOLVER} from "./detail-resolver"; const props = defineProps<{ testCase: CompileTestCase; @@ -98,16 +98,6 @@ function drawCurrentDate(opts: GraphRenderOpts, date: Date) { // Render both relative and absolute graphs async function renderGraphs() { - // We want to be able to see noise "blips" vs. a previous artifact. - // The "percent relative from previous commit" graph should be the best to - // see these kinds of changes. - renderGraph("percentrelative" as GraphKind, relativeChartElement); - // We also want to see whether a change maintained its value or whether it was noise and has since - // returned to steady state. Here, an absolute graph ("raw") is best. - renderGraph("raw" as GraphKind, absoluteChartElement); -} - -async function renderGraph(kind: GraphKind, chartRef: Ref) { const {start, end, date} = graphRange.value; const selector = { benchmark: props.testCase.benchmark, @@ -116,9 +106,66 @@ async function renderGraph(kind: GraphKind, chartRef: Ref) { stat: props.metric, start, end, - kind, + kinds: ["percentrelative", "raw"], }; - const graphData = await GRAPH_RESOLVER.loadGraph(selector); + const detail = await COMPILE_DETAIL_RESOLVER.loadDetail(selector); + if (detail.commits.length === 0) { + return; + } + + function buildGraph( + index: number, + kind: GraphKind + ): [GraphData, GraphsSelector] { + const data = { + commits: detail.commits, + benchmarks: { + [selector.benchmark]: { + [selector.profile]: { + [selector.scenario]: detail.graphs[index], + }, + }, + }, + }; + const graphSelector = { + benchmark: selector.benchmark, + profile: selector.profile, + scenario: selector.scenario, + stat: selector.stat, + start: selector.start, + end: selector.end, + kind, + }; + + return [data, graphSelector]; + } + + const [percentRelativeData, percentRelativeSelector] = buildGraph( + 0, + "percentrelative" + ); + const [rawData, rawSelector] = buildGraph(0, "raw"); + + // We want to be able to see noise "blips" vs. a previous artifact. + // The "percent relative from previous commit" graph should be the best to + // see these kinds of changes. + renderGraph( + percentRelativeData, + percentRelativeSelector, + date, + relativeChartElement + ); + // We also want to see whether a change maintained its value or whether it was noise and has since + // returned to steady state. Here, an absolute graph ("raw") is best. + renderGraph(rawData, rawSelector, date, absoluteChartElement); +} + +async function renderGraph( + graphData: GraphData, + selector: GraphsSelector, + date: Date | null, + chartRef: Ref +) { const opts: GraphRenderOpts = { width: Math.min(window.innerWidth - 40, 465), renderTitle: false, diff --git a/site/frontend/src/pages/compare/compile/table/detail-resolver.ts b/site/frontend/src/pages/compare/compile/table/detail-resolver.ts new file mode 100644 index 000000000..5fd4322ed --- /dev/null +++ b/site/frontend/src/pages/compare/compile/table/detail-resolver.ts @@ -0,0 +1,65 @@ +import {GraphKind} from "../../../../graph/data"; +import {Series} from "uplot"; +import {getJson} from "../../../../utils/requests"; +import {COMPARE_COMPILE_DETAIL_DATA_URL} from "../../../../urls"; + +export interface CompileDetailSelector { + start: string; + end: string; + stat: string; + benchmark: string; + scenario: string; + profile: string; + kinds: GraphKind[]; +} + +// Compile benchmark detail received from the server +export interface CompileDetail { + commits: [[number, string]]; + // One Series for each GraphKind in the CompileDetailSelector + graphs: Series[]; +} + +/** + * Compile benchmark detail resolver that contains a cache of downloaded details. + * This is important for Vue components that download the benchmark detail on mount. + * Without a cache, they would download the detail each time they are destroyed + * and recreated. + */ +export class CompileBenchmarkDetailResolver { + private cache: Dict = {}; + + public async loadDetail( + selector: CompileDetailSelector + ): Promise { + const key = `${selector.benchmark};${selector.profile};${selector.scenario};${selector.start};${selector.end};${selector.stat};${selector.kinds}`; + if (!this.cache.hasOwnProperty(key)) { + this.cache[key] = await loadDetail(selector); + } + + return this.cache[key]; + } +} + +/** + * This is essentially a global variable, but it makes the code simpler and + * since we currently don't have any unit tests, we don't really need to avoid + * global variables that much. If needed, it could be provided to Vue components + * from a parent via props or context. + */ +export const COMPILE_DETAIL_RESOLVER = new CompileBenchmarkDetailResolver(); + +async function loadDetail( + selector: CompileDetailSelector +): Promise { + const params = { + start: selector.start, + end: selector.end, + stat: selector.stat, + benchmark: selector.benchmark, + scenario: selector.scenario, + profile: selector.profile, + kinds: selector.kinds, + }; + return await getJson(COMPARE_COMPILE_DETAIL_DATA_URL, params); +} diff --git a/site/frontend/src/urls.ts b/site/frontend/src/urls.ts index d3da488c0..7f9856b4d 100644 --- a/site/frontend/src/urls.ts +++ b/site/frontend/src/urls.ts @@ -7,4 +7,5 @@ export const STATUS_DATA_URL = `${BASE_URL}/status_page`; export const BOOTSTRAP_DATA_URL = `${BASE_URL}/bootstrap`; export const GRAPH_DATA_URL = `${BASE_URL}/graphs`; export const COMPARE_DATA_URL = `${BASE_URL}/get`; +export const COMPARE_COMPILE_DETAIL_DATA_URL = `${BASE_URL}/compare-compile-detail`; export const SELF_PROFILE_DATA_URL = `${BASE_URL}/self-profile`;