From a8fefe398073dada08f6e8003f75d74b945a47fd Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Wed, 14 Oct 2026 02:23:01 +0200 Subject: [PATCH] :zap: Make component sync about twice as fast A profile of the benchmark worlds showed where a sync spends its time. - The token sync of shapes that apply no tokens took 38% of a sync in the 1000-copy world: for every synced shape, generate-update-tokens walked every sync attribute, resolved its group and its token attributes, and only then found that neither the main shape nor the copy applies a token. It now returns at once, and the token attributes of each sync attribute are computed once, at load. - A sync looks through every object of each page and component to find the copies it updates, so in a file with many shapes and few copies the search costs more than the sync itself. The new benchmark world main-edit-among-many-shapes syncs a main edit into 10 copies in a page with 20,000 plain rects; its baseline, measured before the change below, joins the recorded one with a note, and its result joins the golden master. generate-sync-container called the uses-assets? multimethod on every object through a lazy filter; the predicate is now chosen once per container (asset-user?) and the objects walked with reduce-kv into a vector, in the same order. - compare-children searched the remaining children (d/seek) for every child out of place, and dropped a child taken out of order with a lazy remove, so a reorder took quadratic time and nested one lazy sequence per child taken. It now looks children up by their reference or id (index-positions) and skips those taken out of order when the walk reaches them, with the same callbacks in the same order. The sync of a reorder stays slow: when a main with 1,000 children moves its first one to the end, each copy still gets 999 moves (F85), and applying them is what costs (4.75 s to 4.26 s for 10 copies). Fewer moves change what a sync writes, so that waits for Phase 4c. No behavior change: the golden masters and the suites are unchanged. Benchmark (lower quantile, ms; baseline / now / ratio): deepest-edit-through-three-levels 35.2 / 18.6 / 0.53 edit-through-a-chain-of-dependents 68.9 / 36.4 / 0.53 library-update-into-copies 83.1 / 43.0 / 0.52 main-edit-among-many-shapes 4.8 / 2.3 / 0.48 main-edit-into-10-copies 1.8 / 0.9 / 0.48 main-edit-into-100-copies 15.4 / 8.4 / 0.54 main-edit-into-1000-copies 153.1 / 90.1 / 0.59 variant-switch-into-copies 32.1 / 20.0 / 0.63 (main-edit-among-many-shapes: baseline measured before the search change.) AI-assisted-by: claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) --- .serena/memories/common/testing.md | 3 +- common/dev/bench/component_sync.clj | 39 +++- common/dev/bench/component_sync_baseline.edn | 4 + common/src/app/common/logic/libraries.cljc | 208 ++++++++++++------ .../test/common_tests/logic/golden_master.edn | 2 + 5 files changed, 183 insertions(+), 73 deletions(-) diff --git a/.serena/memories/common/testing.md b/.serena/memories/common/testing.md index c1172ad872..8504fffcac 100644 --- a/.serena/memories/common/testing.md +++ b/.serena/memories/common/testing.md @@ -59,7 +59,8 @@ its work. ## Component sync benchmark `common/dev/bench/component_sync.clj` times component sync on fixed-size worlds (one main into -10/100/1000 copies, 3 nesting levels, a chain of dependents, a variant switch, a library update). +10/100/1000 copies, 3 nesting levels, a chain of dependents, a variant switch, a library update, +10 copies among 20000 plain shapes). Each world checks its result (how many rects end red) so it never times a no-op. Run from `common/`: `scripts/bench-components` compares with `dev/bench/component_sync_baseline.edn` and exits 1 when a world's lower quantile is more than 10% slower; `scripts/bench-components record` diff --git a/common/dev/bench/component_sync.clj b/common/dev/bench/component_sync.clj index 142520c081..2bb67ba780 100644 --- a/common/dev/bench/component_sync.clj +++ b/common/dev/bench/component_sync.clj @@ -27,6 +27,10 @@ [app.common.test-helpers.ids-map :as thi] [app.common.test-helpers.shapes :as ths] [app.common.test-helpers.variants :as thv] + [app.common.types.file :as ctf] + [app.common.types.pages-list :as ctpl] + [app.common.types.shape :as cts] + [app.common.uuid :as uuid] [clojure.edn :as edn] [clojure.java.io :as io] [clojure.java.shell :as shell] @@ -110,6 +114,38 @@ :parent-label root-label) (thc/make-component label root-label))) +(defn- add-plain-shapes + "`file` with `n` plain rects at the root of its page, added in one step." + [file n] + (let [page (thf/current-page file) + rects (map (fn [i] + (cts/setup-shape {:type :rect :id (uuid/next) :name (str "Rect " i) + :x i :y i :width 10 :height 10 + :parent-id uuid/zero :frame-id uuid/zero})) + (range n))] + (ctf/update-file-data + file + (fn [data] + (ctpl/update-page data (:id page) + (fn [page] + (-> page + (update :objects into (map (juxt :id identity)) rects) + (update-in [:objects uuid/zero :shapes] into (map :id rects))))))))) + +(defn- main-edit-among-many-shapes + "One main whose child changes, synced into 10 copies in a page that also + holds 20000 plain shapes, which the sync must look through." + [] + (let [[file ids] (-> (thf/sample-file :file1) + (tho/add-simple-component :c0 :root0 :child0) + (add-plain-shapes 20000) + (instantiate-many :c0 "copy" 10) + (edit :child0 red))] + {:key :main-edit-among-many-shapes + :describe "a main child changes; 10 copies among 20000 plain shapes sync" + :run #(sync-cascade file {} ids) + :reds 11})) + (defn- main-edit-into-copies "One main whose child changes, synced into `n` copies." [n] @@ -219,7 +255,8 @@ deepest-edit-through-three-levels edit-through-a-chain-of-dependents variant-switch-into-copies - library-update-into-copies]) + library-update-into-copies + main-edit-among-many-shapes]) (defn- worlds [] diff --git a/common/dev/bench/component_sync_baseline.edn b/common/dev/bench/component_sync_baseline.edn index 3a6ec50fb1..2784b3b27c 100644 --- a/common/dev/bench/component_sync_baseline.edn +++ b/common/dev/bench/component_sync_baseline.edn @@ -13,6 +13,10 @@ {:mean-ms 75.119, :lower-q-ms 68.931, :passes 3}, :library-update-into-copies {:mean-ms 89.295, :lower-q-ms 83.083, :passes 1}, + ;; added in Phase 3 (2026-10-14), measured on the code before the object + ;; scan of a sync changed, on the same machine + :main-edit-among-many-shapes + {:mean-ms 5.465, :lower-q-ms 4.837, :passes 1}, :main-edit-into-10-copies {:mean-ms 1.847, :lower-q-ms 1.79, :passes 1}, :main-edit-into-100-copies diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index f9c6c1220a..b5fe2c2f34 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -91,7 +91,7 @@ (declare generate-sync-container) (declare generate-sync-shape) -(declare uses-assets?) +(declare asset-user?) (declare generate-sync-shape-direct) (declare sync-shape-recursive) @@ -187,8 +187,12 @@ (container-log :debug (:id container) :msg "Sync page in local file" :page-id (:id container)) (container-log :debug (:id container) :msg "Sync component in local library" :component-id (:id container))) - (let [linked-shapes (->> (vals (:objects container)) - (filter #(uses-assets? asset-type asset-id % library-id)))] + (let [asset-user? (asset-user? asset-type asset-id library-id) + linked-shapes (persistent! + (reduce-kv (fn [shapes _ shape] + (cond-> shapes (asset-user? shape) (conj! shape))) + (transient []) + (:objects container)))] (loop [shapes (seq linked-shapes) changes (-> changes (pcb/with-container container) @@ -210,27 +214,21 @@ changes))) changes)))) -(defmulti uses-assets? - "Checks if a shape uses some asset of the given type in the given library." - (fn [asset-type _ _ _] asset-type)) - -(defmethod uses-assets? :components - [_ component-id shape library-id] - (if (nil? component-id) - (ctk/uses-library-components? shape library-id) - (ctk/instance-of? shape library-id component-id))) - -(defmethod uses-assets? :colors - [_ color-id shape library-id] - (if (nil? color-id) - (cts/uses-library-colors? shape library-id) - (cts/uses-library-color? shape library-id color-id))) - -(defmethod uses-assets? :typographies - [_ typography-id shape library-id] - (if (nil? typography-id) - (cty/uses-library-typographies? shape library-id) - (cty/uses-library-typography? shape library-id typography-id))) +(defn- asset-user? + "A predicate of the shapes that use the asset `asset-id` of type + `asset-type` in the library `library-id`, or any asset of that type in it + when `asset-id` is nil." + [asset-type asset-id library-id] + (case asset-type + :components (if (nil? asset-id) + #(ctk/uses-library-components? % library-id) + #(ctk/instance-of? % library-id asset-id)) + :colors (if (nil? asset-id) + #(cts/uses-library-colors? % library-id) + #(cts/uses-library-color? % library-id asset-id)) + :typographies (if (nil? asset-id) + #(cty/uses-library-typographies? % library-id) + #(cty/uses-library-typography? % library-id asset-id)))) (defmulti generate-sync-shape "Generate changes to synchronize one shape from all assets of the given type @@ -716,6 +714,15 @@ ;; ---- Operation generation helpers ---- +(defn- index-positions + "A map of each value of `key-fn` among the elements of the vector `v` to + their positions, in order." + [key-fn v] + (reduce-kv (fn [index position x] + (update index (key-fn x) (fnil conj []) position)) + {} + v)) + (defn- compare-children "Pair the children of a copy shape with the children of its main and call the callback of each case. A child pairs with the main child it refers to; @@ -723,7 +730,12 @@ `both-cb` in a reset, and does not pair in an inverse sync, so the main child it replaces is removed and the swapped child is added in its place. An inverse sync (which passes `reset?`) never calls `swapped-cb`, so it - passes nil." + passes nil. + + The children are walked in order, side by side. A child out of place is + looked up by its reference (`index-positions`), not searched, so the walk + takes linear time; a child taken out of order is skipped when the walk + reaches it." [changes shape-inst children-inst children-main container-inst only-inst-cb only-main-cb both-cb swapped-cb moved-cb inverse? reset?] (shape-log :trace (:id shape-inst) container-inst :msg "Compare children") (let [pair? @@ -734,12 +746,48 @@ synced-pair? (fn [child-main child-inst] (and (ctk/is-main-of? child-main child-inst) - (or reset? (not (ctk/swapped? child-inst)))))] - (loop [children-inst (seq (or children-inst [])) - children-main (seq (or children-main [])) - changes changes] - (let [child-inst (first children-inst) - child-main (first children-main)] + (or reset? (not (ctk/swapped? child-inst))))) + + insts (vec children-inst) + mains (vec children-main) + insts-by-ref (index-positions :shape-ref insts) + mains-by-id (index-positions :id mains) + + ;; the first position from `position` on whose child was not taken + ;; out of order + next-position + (fn [v position taken] + (loop [position position] + (if (and (< position (count v)) + (contains? taken (:id (nth v position)))) + (recur (inc position)) + position))) + + ;; the first child from `position` on, with `key` in `index`, not + ;; taken and accepted by `pred` + find-child + (fn [index key v position taken pred] + (some (fn [p] + (let [child (nth v p)] + (when (and (>= p position) + (not (contains? taken (:id child))) + (pred child)) + child))) + (get index key))) + + remaining + (fn [v position taken] + (remove #(contains? taken (:id %)) (subvec v (min position (count v)))))] + + (loop [i 0 + j 0 + taken-insts #{} + taken-mains #{} + changes changes] + (let [i (next-position insts i taken-insts) + j (next-position mains j taken-mains) + child-inst (get insts i) + child-main (get mains j)] (shape-log :trace (:id shape-inst) container-inst :msg "Comparing" :main (str (:name child-main) " " (pretty-uuid (:id child-main))) @@ -749,37 +797,47 @@ changes (nil? child-inst) - (reduce only-main-cb changes children-main) + (reduce only-main-cb changes (remaining mains j taken-mains)) (nil? child-main) - (reduce only-inst-cb changes children-inst) + (reduce only-inst-cb changes (remaining insts i taken-insts)) :else (if (pair? child-main child-inst) - (recur (next children-inst) - (next children-main) + (recur (inc i) + (inc j) + taken-insts + taken-mains (if (synced-pair? child-main child-inst) (both-cb changes child-inst child-main) (swapped-cb changes child-inst child-main))) - (let [child-inst' (d/seek #(pair? child-main %) children-inst) - child-main' (d/seek #(pair? % child-inst) children-main)] + (let [child-inst' (find-child insts-by-ref (:id child-main) insts i taken-insts + #(pair? child-main %)) + child-main' (find-child mains-by-id (:shape-ref child-inst) mains j taken-mains + #(pair? % child-inst))] (cond (nil? child-inst') - (recur children-inst - (next children-main) + (recur i + (inc j) + taken-insts + taken-mains (only-main-cb changes child-main)) (nil? child-main') - (recur (next children-inst) - children-main + (recur (inc i) + j + taken-insts + taken-mains (only-inst-cb changes child-inst)) :else (if inverse? (let [synced? (synced-pair? child-main' child-inst)] - (recur (next children-inst) - (remove #(= (:id %) (:id child-main')) children-main) + (recur (inc i) + j + taken-insts + (conj taken-mains (:id child-main')) (cond-> changes synced? (both-cb child-inst child-main') @@ -788,8 +846,10 @@ :always (moved-cb child-inst child-main')))) (let [synced? (synced-pair? child-main child-inst')] - (recur (remove #(= (:id %) (:id child-inst')) children-inst) - (next children-main) + (recur i + (inc j) + (conj taken-insts (:id child-inst')) + taken-mains (cond-> changes synced? (both-cb child-inst' child-main) @@ -1208,37 +1268,43 @@ :ignore-touched true}]})) (pcb/apply-changes-local))))) +;; The token attributes of each attribute whose tokens sync; the +;; `layout-grid-cells` are not synced automatically. +(def ^:private token-attrs-by-sync-attr + (into {} + (comp (remove #(= :layout-grid-cells %)) + (map (juxt identity cto/shape-attr->token-attrs))) + (keys ctk/sync-attrs))) + (defn generate-update-tokens "Copy into `dest-shape` the tokens `origin-shape` applies to the attributes it syncs: with `omit-touched?`, only those whose group is not in `touched`; - with `valid-attrs`, only those attributes (all when nil)." + with `valid-attrs`, only those attributes (all when nil). Nothing to copy + or remove when neither shape applies tokens." [changes container dest-shape origin-shape touched omit-touched? valid-attrs] - (let [attrs - (->> (keys ctk/sync-attrs) - ;; We don't update the flex-child attrs - (remove #(= :layout-grid-cells %))) + (if (and (empty? (:applied-tokens origin-shape)) + (empty? (:applied-tokens dest-shape))) + changes + (let [;; the styles of a text live in its content, so its tokens follow + ;; the text override groups + text? (cfh/text-shape? dest-shape) + touched (cond-> touched + text? (-> set (into (txt/text-overrides touched)))) - ;; the styles of a text live in its content, so its tokens follow - ;; the text override groups - text? (cfh/text-shape? dest-shape) - touched (cond-> touched - text? (-> set (into (txt/text-overrides touched)))) - - applied-tokens - (reduce (fn [applied-tokens attr] - (let [sync-group (or (when text? (txt/style-unit-of attr)) - (ctk/resolve-sync-group (:type origin-shape) attr) - (ctk/resolve-sync-group (:type dest-shape) attr)) - token-attrs (cto/shape-attr->token-attrs attr)] - (if (and (or (not omit-touched?) (not (touched sync-group))) - (or (empty? valid-attrs) (contains? valid-attrs attr))) - (into applied-tokens token-attrs) - applied-tokens))) - #{} - attrs)] - (cond-> changes - (seq applied-tokens) - (update-tokens container dest-shape origin-shape applied-tokens)))) + applied-tokens + (reduce-kv (fn [applied-tokens attr token-attrs] + (let [sync-group (or (when text? (txt/style-unit-of attr)) + (ctk/resolve-sync-group (:type origin-shape) attr) + (ctk/resolve-sync-group (:type dest-shape) attr))] + (if (and (or (not omit-touched?) (not (touched sync-group))) + (or (empty? valid-attrs) (contains? valid-attrs attr))) + (into applied-tokens token-attrs) + applied-tokens))) + #{} + token-attrs-by-sync-attr)] + (cond-> changes + (seq applied-tokens) + (update-tokens container dest-shape origin-shape applied-tokens))))) (defn add-update-attr-changes "Add to `changes` the operations of `attr-operations` on `dest-shape`, and diff --git a/common/test/common_tests/logic/golden_master.edn b/common/test/common_tests/logic/golden_master.edn index dffc76d48f..0efaec6db5 100644 --- a/common/test/common_tests/logic/golden_master.edn +++ b/common/test/common_tests/logic/golden_master.edn @@ -134,6 +134,8 @@ {:transcript "create-component {:component \"main\", :id sync-main-component}\n make-variant-container {:variant-set \"vset\", :variant-id sync-vset-vcontainer, :count 4}\n make-nested-component-with-variant {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n make-nested-component {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n make-nested-component {:component \"main\", :level 2, :outer sync-main-outercomp-2}\n one-of -> skip\n switch-variant {:target-component sync-vset-v2-component}\n one-of -> skip\n test", :states ["43b4a591dc02b01b" "d844ece54beb0bfe"]} {:transcript "create-component {:component \"main\", :id sync-main-component}\n make-variant-container {:variant-set \"vset\", :variant-id sync-vset-vcontainer, :count 4}\n make-nested-component-with-variant {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n make-nested-component {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n make-nested-component {:component \"main\", :level 2, :outer sync-main-outercomp-2}\n one-of -> skip\n one-of -> skip\n switch-variant {:target-component sync-vset-v3-component}\n test", :states ["4e56e67caa87f262" "85d6d4a265a35a44"]} {:transcript "create-component {:component \"main\", :id sync-main-component}\n make-variant-container {:variant-set \"vset\", :variant-id sync-vset-vcontainer, :count 4}\n make-nested-component-with-variant {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n make-nested-component {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n make-nested-component {:component \"main\", :level 2, :outer sync-main-outercomp-2}\n one-of -> skip\n one-of -> skip\n one-of -> skip\n test", :states ["a3344fab60ad0124"]}] + :main-edit-among-many-shapes + [{:transcript "main-edit-among-many-shapes", :states ["ddbd413ae850e029"]}] :main-edit-into-10-copies [{:transcript "main-edit-into-10-copies", :states ["55176ea7a3052277"]}] :main-edit-into-100-copies