Refactor detail loading

Author: Kobzol

site/frontend/src/pages/compare/compile/table/benchmark-detail.vue

@@ -13,7 +13,7 @@ import {GraphRenderOpts, renderPlots} from "../../../../graph/render";
 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";
+import {COMPILE_DETAIL_GRAPHS_RESOLVER} from "./detail-resolver";
 import PerfettoLink from "../../../../components/perfetto-link.vue";
 
 const props = defineProps<{
@@ -109,8 +109,8 @@ async function renderGraphs() {
     end,
     kinds: ["percentrelative", "raw"] as GraphKind[],
   };
-  const detail = await COMPILE_DETAIL_RESOLVER.loadDetail(selector);
-  if (detail.commits.length === 0) {
+  const graphsDetail = await COMPILE_DETAIL_GRAPHS_RESOLVER.load(selector);
+  if (graphsDetail.commits.length === 0) {
     return;
   }
 
@@ -119,13 +119,13 @@ async function renderGraphs() {
     kind: GraphKind
   ): [GraphData, GraphsSelector] {
     const data = {
-      commits: detail.commits,
+      commits: graphsDetail.commits,
       benchmarks: {
         [selector.benchmark]: {
           // The server returns profiles capitalized, so we need to match that
           // here, so that the graph code can find the expected profile.
           [capitalize(selector.profile)]: {
-            [selector.scenario]: detail.graphs[index],
+            [selector.scenario]: graphsDetail.graphs[index],
           },
         },
       },

site/frontend/src/pages/compare/compile/table/detail-resolver.ts

@@ -1,8 +1,9 @@
 import {GraphKind, Series} from "../../../../graph/data";
 import {getJson} from "../../../../utils/requests";
-import {COMPARE_COMPILE_DETAIL_DATA_URL} from "../../../../urls";
+import {COMPARE_COMPILE_DETAIL_GRAPHS_DATA_URL} from "../../../../urls";
+import {CachedDataLoader} from "./utils";
 
-export interface CompileDetailSelector {
+export interface CompileDetailGraphsSelector {
   start: string;
   end: string;
   stat: string;
@@ -13,7 +14,7 @@ export interface CompileDetailSelector {
 }
 
 // Compile benchmark detail received from the server
-export interface CompileDetail {
+export interface CompileDetailGraphs {
   commits: Array<[number, string]>;
   // One Series for each GraphKind in the CompileDetailSelector
   graphs: Series[];
@@ -24,33 +25,23 @@ export interface CompileDetail {
  * 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<CompileDetail> = {};
-
-  public async loadDetail(
-    selector: CompileDetailSelector
-  ): Promise<CompileDetail> {
-    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();
+export const COMPILE_DETAIL_GRAPHS_RESOLVER: CachedDataLoader<
+  CompileDetailGraphsSelector,
+  CompileDetailGraphs
+> = new CachedDataLoader(
+  (key: CompileDetailGraphsSelector) =>
+    `${key.benchmark};${key.profile};${key.scenario};${key.start};${key.end};${key.stat};${key.kinds}`,
+  loadDetail
+);
 
 async function loadDetail(
-  selector: CompileDetailSelector
-): Promise<CompileDetail> {
+  selector: CompileDetailGraphsSelector
+): Promise<CompileDetailGraphs> {
   const params = {
     start: selector.start,
     end: selector.end,
@@ -60,5 +51,8 @@ async function loadDetail(
     profile: selector.profile,
     kinds: selector.kinds.join(","),
   };
-  return await getJson<CompileDetail>(COMPARE_COMPILE_DETAIL_DATA_URL, params);
+  return await getJson<CompileDetailGraphs>(
+    COMPARE_COMPILE_DETAIL_GRAPHS_DATA_URL,
+    params
+  );
 }

site/frontend/src/pages/compare/compile/table/utils.ts

@@ -29,3 +29,24 @@ function format_ymd(date: Date): string {
 export function daysBetweenDates(a: Date, b: Date): number {
   return Math.floor((b.getTime() - a.getTime()) / (1000 * 60 * 60 * 24));
 }
+
+/**
+ * Provides a cache for `Output` items that can be loaded asynchronously, given a certain `Key`.
+ * If a value has already been loaded for the same key, it will be resolved from the cache.
+ */
+export class CachedDataLoader<Key, Output> {
+  private cache: Dict<Output> = {};
+
+  constructor(
+    private key_to_hash: (key: Key) => string,
+    private load_fn: (key: Key) => Promise<Output>
+  ) {}
+
+  public async load(key: Key): Promise<Output> {
+    const hash = this.key_to_hash(key);
+    if (!this.cache.hasOwnProperty(hash)) {
+      this.cache[hash] = await this.load_fn(key);
+    }
+    return this.cache[hash];
+  }
+}

site/frontend/src/urls.ts

@@ -7,5 +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-graphs`;
+export const COMPARE_COMPILE_DETAIL_GRAPHS_DATA_URL = `${BASE_URL}/compare-compile-detail-graphs`;
 export const SELF_PROFILE_DATA_URL = `${BASE_URL}/self-profile`;