From 2d6d12c8d6ee5e33a92215c138074d1ea35fa84f Mon Sep 17 00:00:00 2001
From: Braden Shepherdson <braden@metabase.com>
Date: Thu, 1 Feb 2024 16:03:27 -0500
Subject: [PATCH] [MLv2] Cache JS -> CLJS metadata conversion (#38375)
This previous got rebuilt from scratch over and over, since different
queries being handed the same JS `Metadata` instance were returning
completely separate `MetadataProvider`s.
The FE replaces the `Metadata` instance as new metadata arrives from the
BE, so the `metabase.lib.cache/side-channel-cache` which is attached to
the input object is implicitly invalidated when the old `Metadata` is
thrown away.
---
src/metabase/lib/cache.cljc | 33 ++++++++++++++++---------------
src/metabase/lib/js/metadata.cljs | 13 ++++++++++--
2 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/src/metabase/lib/cache.cljc b/src/metabase/lib/cache.cljc
index ec573c0ec60..391e98486bf 100644
--- a/src/metabase/lib/cache.cljc
+++ b/src/metabase/lib/cache.cljc
@@ -10,19 +10,20 @@
If there is not already a key `subkey` in the map, calls `(f x)` and caches the value at `subkey`.
If there is a value at `subkey`, it is returned directly."
- [subkey x f]
- (comment subkey) ; Avoids lint warning for half-unused `subkey`.
- #?(:clj (f x)
- :cljs (if (or (object? x) (map? x))
- (do
- (when-not (.-__mbcache ^js x)
- (set! (.-__mbcache ^js x) (atom {})))
- (if-let [cache (.-__mbcache ^js x)]
- (if-let [cached (get @cache subkey)]
- cached
- ;; Cache miss - generate the value and cache it.
- (let [value (f x)]
- (swap! cache assoc subkey value)
- value))
- (f x)))
- (f x))))
+ ([subkey x f] (side-channel-cache subkey x f false))
+ ([subkey x f force?]
+ (comment subkey, force?) ; Avoids lint warning for half-unused inputs.
+ #?(:clj (f x)
+ :cljs (if (or force? (object? x) (map? x))
+ (do
+ (when-not (.-__mbcache ^js x)
+ (set! (.-__mbcache ^js x) (atom {})))
+ (if-let [cache (.-__mbcache ^js x)]
+ (if-let [cached (get @cache subkey)]
+ cached
+ ;; Cache miss - generate the value and cache it.
+ (let [value (f x)]
+ (swap! cache assoc subkey value)
+ value))
+ (f x)))
+ (f x)))))
diff --git a/src/metabase/lib/js/metadata.cljs b/src/metabase/lib/js/metadata.cljs
index e5f1a2897a7..1f83f68c0c1 100644
--- a/src/metabase/lib/js/metadata.cljs
+++ b/src/metabase/lib/js/metadata.cljs
@@ -6,6 +6,7 @@
[goog]
[goog.object :as gobject]
[medley.core :as m]
+ [metabase.lib.cache :as lib.cache]
[metabase.lib.metadata.protocols :as lib.metadata.protocols]
[metabase.lib.util :as lib.util]
[metabase.util :as u]
@@ -24,6 +25,7 @@
;;;
;;; where keys are a map of String ID => metadata
+;; TODO: This is always wrapped with `keyword` in its usage so that may as well be memoized too.
(def ^:private ^{:arglists '([k])} memoized-kebab-key
"Even tho [[u/->kebab-case-en]] has LRU memoization, plain memoization is significantly faster, and since the keys
we're parsing here are bounded it's fine to memoize this stuff forever."
@@ -506,8 +508,8 @@
(object-get "settings")
(object-get (name setting-key))))
-(defn metadata-provider
- "Use a `metabase-lib/metadata/Metadata` as a [[metabase.lib.metadata.protocols/MetadataProvider]]."
+(defn- metadata-provider*
+ "Inner implementation for [[metadata-provider]], which wraps this with a cache."
[database-id unparsed-metadata]
(let [metadata (parse-metadata unparsed-metadata)]
(log/debug "Created metadata provider for metadata")
@@ -535,6 +537,13 @@
form))
metadata)))))
+(defn metadata-provider
+ "Use a `metabase-lib/metadata/Metadata` as a [[metabase.lib.metadata.protocols/MetadataProvider]]."
+ [database-id unparsed-metadata]
+ (lib.cache/side-channel-cache (str database-id) unparsed-metadata
+ (partial metadata-provider* database-id)
+ true #_force?))
+
(def parse-column
"Parses a JS column provided by the FE into a :metadata/column value for use in MLv2."
(parse-object-fn :field))
--
GitLab