mirror of
https://github.com/logseq/logseq.git
synced 2026-05-16 00:42:20 +00:00
fix: get-bidirectional-properties perf
Root cause: get-bidirectional-properties was recomputing bidirectional-property-attr? for every [e a] match, repeatedly calling d/entity for the same property attr keyword. That made cost scale with datom count, not unique properties. Fix: Added per-call memoization for property-attr bidirectional checks using a local volatile! cache, so each attr is resolved once per invocation.
This commit is contained in:
19
deps/db/src/logseq/db.cljs
vendored
19
deps/db/src/logseq/db.cljs
vendored
@@ -670,9 +670,11 @@
|
||||
(d/q '[:find ?e ?a
|
||||
:in $ ?v
|
||||
:where
|
||||
[?e ?a ?v]
|
||||
[?c :logseq.property.class/enable-bidirectional? ?c-enable?]
|
||||
[(true? ?c-enable?)]
|
||||
[?ea :logseq.property/classes ?c]
|
||||
[?ea :db/ident ?a]
|
||||
[?ea :logseq.property/classes]]
|
||||
[?e ?a ?v]]
|
||||
db
|
||||
v))
|
||||
|
||||
@@ -687,10 +689,19 @@
|
||||
(fn [acc class-id entity]
|
||||
(if class-id
|
||||
(update acc class-id (fnil conj #{}) entity)
|
||||
acc))]
|
||||
acc))
|
||||
*attr->bidirectional? (volatile! {})
|
||||
bidirectional-property-attr-cached?
|
||||
(fn [attr]
|
||||
(let [cache @*attr->bidirectional?]
|
||||
(if (contains? cache attr)
|
||||
(get cache attr)
|
||||
(let [result (bidirectional-property-attr? db attr)]
|
||||
(vswap! *attr->bidirectional? assoc attr result)
|
||||
result))))]
|
||||
(->> (get-ea-by-v db target-id)
|
||||
(keep (fn [[e a]]
|
||||
(when (bidirectional-property-attr? db a)
|
||||
(when (bidirectional-property-attr-cached? a)
|
||||
(when-let [entity (d/entity db e)]
|
||||
(when (and (not= (:db/id entity) target-id)
|
||||
(not (entity-util/class? entity))
|
||||
|
||||
55
deps/db/test/logseq/db_test.cljs
vendored
55
deps/db/test/logseq/db_test.cljs
vendored
@@ -148,3 +148,58 @@
|
||||
(is (= "People" (:title (first results))))
|
||||
(is (= ["Alice"]
|
||||
(map :block/title (:entities (first results))))))))
|
||||
|
||||
(defn- bidirectional-perf-conn
|
||||
[n property-titles]
|
||||
(let [target-page {:page {:block/title "Target"}}
|
||||
properties (into {}
|
||||
(map (fn [property-title]
|
||||
[property-title {:logseq.property/type :node
|
||||
:build/property-classes [:Person]}]))
|
||||
property-titles)
|
||||
person-properties (into {}
|
||||
(map (fn [property-title]
|
||||
[property-title [:build/page {:block/title "Target"}]]))
|
||||
property-titles)
|
||||
pages (vec (concat [target-page]
|
||||
(map (fn [i]
|
||||
{:page {:block/title (str "Person " i)
|
||||
:build/tags [:Person]
|
||||
:build/properties person-properties}})
|
||||
(range n))))]
|
||||
(db-test/create-conn-with-blocks
|
||||
{:properties properties
|
||||
:classes {:Person {:build/properties {:logseq.property.class/enable-bidirectional? true}}}
|
||||
:pages-and-blocks pages})))
|
||||
|
||||
(deftest ^:long get-bidirectional-properties-performance-single-property
|
||||
(testing "attribute lookups scale with unique properties, not entities"
|
||||
(let [conn (bidirectional-perf-conn 400 [:friend])
|
||||
target-id (:db/id (db-test/find-page-by-title @conn "Target"))
|
||||
original-entity d/entity
|
||||
attr-lookups (atom 0)
|
||||
results (with-redefs [d/entity (fn [db eid]
|
||||
(when (keyword? eid)
|
||||
(swap! attr-lookups inc))
|
||||
(original-entity db eid))]
|
||||
(ldb/get-bidirectional-properties @conn target-id))]
|
||||
(is (= 1 (count results)))
|
||||
(is (= 400 (count (:entities (first results)))))
|
||||
(is (<= @attr-lookups 8)
|
||||
(str "expected bounded attr lookups, got " @attr-lookups)))))
|
||||
|
||||
(deftest ^:long get-bidirectional-properties-performance-multi-property
|
||||
(testing "attribute lookups stay bounded with multiple matching properties"
|
||||
(let [conn (bidirectional-perf-conn 300 [:friend :colleague])
|
||||
target-id (:db/id (db-test/find-page-by-title @conn "Target"))
|
||||
original-entity d/entity
|
||||
attr-lookups (atom 0)
|
||||
results (with-redefs [d/entity (fn [db eid]
|
||||
(when (keyword? eid)
|
||||
(swap! attr-lookups inc))
|
||||
(original-entity db eid))]
|
||||
(ldb/get-bidirectional-properties @conn target-id))]
|
||||
(is (= 1 (count results)))
|
||||
(is (= 300 (count (:entities (first results)))))
|
||||
(is (<= @attr-lookups 12)
|
||||
(str "expected bounded attr lookups, got " @attr-lookups)))))
|
||||
|
||||
Reference in New Issue
Block a user