Skip to content
Snippets Groups Projects
Unverified Commit 7b14ccd9 authored by Oleksandr Yakushev's avatar Oleksandr Yakushev Committed by GitHub
Browse files

perf: Allocation improvements on sync-db/fingerprinting path (#46167)

* perf: Optimize metabase.lib.schema.util/unique-uuids?

* perf: Allocation improvements on sync-db/fingerprinting path
parent d2c2bccb
No related branches found
No related tags found
No related merge requests found
......@@ -800,15 +800,17 @@
(defn matching-locations
"Find the forms matching pred, returns a list of tuples of location (as used in get-in) and the match."
[form pred]
(loop [stack [[[] form]], matches []]
;; Surprisingly enough, a list works better as a stack here than a vector.
(loop [stack (list [[] form]), matches []]
(if-let [[loc form :as top] (peek stack)]
(let [stack (pop stack)
onto-stack #(into stack (map (fn [[k v]] [(conj loc k) v])) %)]
map-onto-stack #(transduce (map (fn [[k v]] [(conj loc k) v])) conj stack %)
seq-onto-stack #(transduce (map-indexed (fn [i v] [(conj loc i) v])) conj stack %)]
(cond
(pred form) (recur stack (conj matches top))
(map? form) (recur (onto-stack form) matches)
(sequential? form) (recur (onto-stack (map-indexed vector form)) matches)
:else (recur stack matches)))
(pred form) (recur stack (conj matches top))
(map? form) (recur (map-onto-stack form) matches)
(sequential? form) (recur (seq-onto-stack form) matches)
:else (recur stack matches)))
matches)))
(defn wrap-field-id-if-needed
......
......@@ -4,50 +4,45 @@
[metabase.lib.options :as lib.options]
[metabase.util.malli.registry :as mr]))
(declare collect-uuids)
(declare collect-uuids*)
(defn- collect-uuids-in-map [m]
(into (if-let [our-uuid (or (:lib/uuid (lib.options/options m))
(:lib/uuid m))]
[our-uuid]
[])
(comp (remove (fn [[k _v]]
(qualified-keyword? k)))
(mapcat (fn [[_k v]]
(collect-uuids v))))
m))
(defn- collect-uuids-in-map [m result]
(when-let [our-uuid (or (:lib/uuid (lib.options/options m))
(:lib/uuid m))]
;; Keep duplicates in metadata of the result.
(if (@result our-uuid)
(vswap! result vary-meta update :duplicates (fnil conj #{}) our-uuid)
(vswap! result conj our-uuid)))
(defn- collect-uuids-in-sequence [xs]
(into [] (mapcat collect-uuids) xs))
(reduce-kv (fn [_ k v]
(when (not (qualified-keyword? k))
(collect-uuids* v result)))
nil m))
(defn- collect-uuids-in-sequence [xs result]
(run! #(collect-uuids* % result) xs))
(defn- collect-uuids* [x result]
(cond
(map? x) (collect-uuids-in-map x result)
(sequential? x) (collect-uuids-in-sequence x result)
:else nil))
(defn collect-uuids
"Return all the `:lib/uuid`s in a part of an MBQL query (a clause or map) as a sequence. This will be used to ensure
there are no duplicates."
[x]
(cond
(map? x) (collect-uuids-in-map x)
(sequential? x) (collect-uuids-in-sequence x)
:else nil))
(let [result (volatile! #{})]
(collect-uuids* x result)
@result))
(defn- find-duplicate-uuid [x]
(transduce
identity
(fn
([]
#{})
([result]
(when (string? result)
result))
([seen a-uuid]
(if (contains? seen a-uuid)
(reduced a-uuid)
(conj seen a-uuid))))
(collect-uuids x)))
(:duplicates (meta (collect-uuids x))))
(defn unique-uuids?
"True if all the `:lib/uuid`s in something are unique."
[x]
(not (find-duplicate-uuid x)))
(empty? (find-duplicate-uuid x)))
;;; Malli schema for to ensure that all `:lib/uuid`s are unique.
(mr/def ::unique-uuids
......
......@@ -26,15 +26,23 @@
{:pre [(fn? match-fn) (vector? clause-parents)]}
(cond
(map? form)
(mapcat (fn [[k v]]
(match-fn (conj clause-parents k) v))
form)
(reduce-kv (fn [acc k v]
(if-let [match (match-fn (conj clause-parents k) v)]
;; Deliberately not using into to avoid converting to transient and back.
(reduce conj acc match)
acc))
[] form)
(sequential? form)
(mapcat (partial match-fn (if (keyword? (first form))
(conj clause-parents (first form))
clause-parents))
form)))
(let [fst (first form)
k (if (keyword? fst)
(conj clause-parents fst)
clause-parents)]
(reduce (fn [acc v]
(if-let [match (match-fn k v)]
(reduce conj acc match)
acc))
[] form))))
(defn replace-in-collection
"Inernal impl for `replace`. Recursively replace values in a collection using a `replace-fn`."
......
......@@ -64,12 +64,12 @@
[field-metadata :- TableMetadataFieldWithOptionalID
other-metadata :- [:set TableMetadataFieldWithOptionalID]]
(let [field-meta-canonical (canonical-name field-metadata)
matches (keep
(fn [other-field-metadata]
(when (= field-meta-canonical
(canonical-name other-field-metadata))
other-field-metadata))
other-metadata)]
matches (into [] (keep
(fn [other-field-metadata]
(when (= field-meta-canonical
(canonical-name other-field-metadata))
other-field-metadata)))
other-metadata)]
(case (count matches)
0
nil
......
......@@ -34,21 +34,20 @@
(deftest ^:parallel collect-uuids-test
(are [query expected-uuids] (= expected-uuids
(sort (lib.schema.util/collect-uuids query)))
(lib.schema.util/collect-uuids query))
query-with-no-duplicate-uuids
["00000000-0000-0000-0000-000000000001"
"00000000-0000-0000-0000-000000000002"
"00000000-0000-0000-0000-000000000010"
"00000000-0000-0000-0000-000000000020"]
#{"00000000-0000-0000-0000-000000000001"
"00000000-0000-0000-0000-000000000002"
"00000000-0000-0000-0000-000000000010"
"00000000-0000-0000-0000-000000000020"}
query-with-duplicate-uuids
["00000000-0000-0000-0000-000000000001"
"00000000-0000-0000-0000-000000000001"
"00000000-0000-0000-0000-000000000010"
"00000000-0000-0000-0000-000000000020"]))
#{"00000000-0000-0000-0000-000000000001"
"00000000-0000-0000-0000-000000000010"
"00000000-0000-0000-0000-000000000020"}))
(deftest ^:parallel collect-uuids-from-lib-options
(is (= ["f590f35f-9224-45f1-8334-422f15fc4abd"]
(is (= #{"f590f35f-9224-45f1-8334-422f15fc4abd"}
(lib.schema.util/collect-uuids
{:lib/type :mbql/query
:database 1
......@@ -63,7 +62,7 @@
(deftest ^:parallel unique-uuids-schema-test
(is (not (mc/explain ::lib.schema.util/unique-uuids query-with-no-duplicate-uuids)))
(is (mc/explain ::lib.schema.util/unique-uuids query-with-duplicate-uuids))
(is (= ["Duplicate :lib/uuid \"00000000-0000-0000-0000-000000000001\""]
(is (= ["Duplicate :lib/uuid #{\"00000000-0000-0000-0000-000000000001\"}"]
(me/humanize (mc/explain ::lib.schema.util/unique-uuids query-with-duplicate-uuids)))))
(deftest ^:parallel distinct-refs-test
......
......@@ -12,7 +12,7 @@
(is (not (mc/explain ::lib.schema/query lib.schema.util-test/query-with-no-duplicate-uuids))))
(testing "should not validate if UUIDs are duplicated"
(is (mc/explain ::lib.schema/query lib.schema.util-test/query-with-duplicate-uuids))
(is (= ["Duplicate :lib/uuid \"00000000-0000-0000-0000-000000000001\""]
(is (= ["Duplicate :lib/uuid #{\"00000000-0000-0000-0000-000000000001\"}"]
(me/humanize (mc/explain ::lib.schema/query lib.schema.util-test/query-with-duplicate-uuids))))))
(def ^:private valid-ag-1
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment