From 6846afd08c9570a8312e37501dd671d4a9fc6a83 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Tue, 10 Nov 2026 00:35:20 +0800 Subject: [PATCH] :zap: Send objects-map pages to the worker without a second encode (#12811) * :zap: Send objects-map pages to the worker without a second encode When switching between pages on a large file e.g Material Design Icons, there's some UI freeze from :index/initialize encoding object map shapes to send to the worker. An objects map already stores the encoded JSON for each shape, and `encode-str` was wrapping those strings in a second layer of JSON on the main thread. We can just transmit the already encoded JSON in a js/Map alongisde the payload, similar to :blob, so structured clone just copies the strings verbaitm without re-encoding. To do this this PR adds two functions to ObjectsMap that converts to and from a js/Map. The "Main components" page of the Material Design Icons has ~30k objects, so this reduces the freeze when switching pages from 306ms -> 189ms (1.63x) on a release build. AI-assisted-by: claude-sonnet-5-5 * Split property tests out into new deftest --------- Signed-off-by: Luke Lau Co-authored-by: Andrey Antukh --- common/src/app/common/types/objects_map.cljc | 17 +++++++++ .../common_tests/types/objects_map_test.cljc | 37 +++++++++++++++++++ frontend/src/app/worker/messages.cljs | 25 ++++++++++++- frontend/test/frontend_tests/runner.cljs | 2 + .../frontend_tests/worker/messages_test.cljs | 32 ++++++++++++++++ 5 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 frontend/test/frontend_tests/worker/messages_test.cljs diff --git a/common/src/app/common/types/objects_map.cljc b/common/src/app/common/types/objects_map.cljc index 1b4e17ce4c..2d809d01fa 100644 --- a/common/src/app/common/types/objects_map.cljc +++ b/common/src/app/common/types/objects_map.cljc @@ -504,6 +504,23 @@ (into (create)) (compact)))) +#?(:cljs + (defn ->js-map + "Builds a `js/Map` from the entries of an `ObjectsMap`." + [objects] + (reduce-kv (fn [^js result id shape] + (.set result (str id) shape)) + (js/Map.) + (get-data objects)))) + +#?(:cljs + (defn from-js-map + "Builds an `ObjectsMap` from a `js/Map`." + [^js entries] + (from-data (into {} + (map (fn [[id shape]] [(uuid id) shape])) + (js/Array.from entries))))) + #?(:clj (fres/add-handlers! {:name "penpot/objects-map/v2" diff --git a/common/test/common_tests/types/objects_map_test.cljc b/common/test/common_tests/types/objects_map_test.cljc index 7eae046e3a..e112a4e406 100644 --- a/common/test/common_tests/types/objects_map_test.cljc +++ b/common/test/common_tests/types/objects_map_test.cljc @@ -131,3 +131,40 @@ (omap/objects-map? (:objects data)) (omap/objects-map? (:objects res))))) {:num 100})) + +#?(:cljs + (t/deftest js-map-round-trip + (smt/check! + (smt/for [data (->> (sg/map-of (sg/uuid) (sg/generator cts/schema:shape)) + (sg/not-empty) + (sg/fmap omap/wrap))] + (let [res (-> data omap/->js-map js/structuredClone omap/from-js-map)] + (and (omap/objects-map? res) + (= data res)))) + {:num 100}))) + +#?(:cljs + (t/deftest js-map-conversion + (t/testing "entries stay encoded strings" + (let [id (uuid/custom 0 1) + ^js entries (omap/->js-map (omap/wrap {id {:foo 1}}))] + (t/is (= 1 (.-size entries))) + (t/is (string? (.get entries (str id)))))) + + (t/testing "added and removed shapes" + (let [id1 (uuid/custom 0 1) + id2 (uuid/custom 0 2) + id3 (uuid/custom 0 3) + obj (-> (omap/wrap {id1 {:foo 1} id2 {:bar 2}}) + (assoc id3 {:baz 3}) + (dissoc id1)) + res (-> obj omap/->js-map omap/from-js-map)] + (t/is (= 2 (count res))) + (t/is (not (contains? res id1))) + (t/is (= {:bar 2} (get res id2))) + (t/is (= {:baz 3} (get res id3))))) + + (t/testing "empty map" + (let [res (-> (omap/create) omap/->js-map omap/from-js-map)] + (t/is (omap/objects-map? res)) + (t/is (zero? (count res))))))) diff --git a/frontend/src/app/worker/messages.cljs b/frontend/src/app/worker/messages.cljs index ef06f9e102..bbc10d2485 100644 --- a/frontend/src/app/worker/messages.cljs +++ b/frontend/src/app/worker/messages.cljs @@ -9,6 +9,7 @@ (:require [app.common.data :as d] [app.common.transit :as t] + [app.common.types.objects-map :as omap] [app.util.object :as obj])) (defn encode @@ -16,7 +17,12 @@ Transit cannot carry `js/Blob`. If the payload map has `:blob`, or the message already has a top-level `:blob`, that value is sent as a - structured-clone field beside the Transit string — not inside it." + structured-clone field beside the Transit string — not inside it. + + The same goes for the objects of a `:page` that are an `ObjectsMap`: + every shape in it is already a Transit string, so Transit would only + wrap those strings in a second layer of JSON. They travel as the + `pageObjects` field, a `js/Map` of id -> encoded shape." [{:keys [sender-id reply-to payload buffer? blob completed error]}] (let [;; Peel `:blob` out of map payloads before Transit (selection/snap ;; replies are vectors and other non-maps; leave them untouched). @@ -25,6 +31,15 @@ payload (cond-> payload (some? inline-blob) (dissoc :blob)) + + page-objects (when (map? payload) + (let [objects (get-in payload [:page :objects])] + (when (omap/objects-map? objects) + objects))) + payload (cond-> payload + (some? page-objects) + (update :page dissoc :objects)) + obj #js {:cmd (d/name (:cmd payload)) :senderId (when sender-id (str sender-id)) :replyTo (when reply-to (str reply-to)) @@ -33,6 +48,8 @@ :completed (when completed true)}] (when (some? blob) (obj/set! obj "blob" blob)) + (when (some? page-objects) + (obj/set! obj "pageObjects" (omap/->js-map page-objects))) (when (some? error) (obj/set! obj "error" (clj->js error))) obj)) @@ -51,6 +68,7 @@ payload (obj/get data "payload") buffer (obj/get data "buffer") blob (obj/get data "blob") + page-objects (obj/get data "pageObjects") completed (obj/get data "completed") error (obj/get data "error") decoded @@ -63,4 +81,7 @@ :error (js->error-map error)})] (cond-> decoded (some? blob) - (update :payload assoc :blob blob)))) + (update :payload assoc :blob blob) + + (some? page-objects) + (assoc-in [:payload :page :objects] (omap/from-js-map page-objects))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index a9bf962423..85ff90970d 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -185,6 +185,7 @@ [frontend-tests.worker-snap-test] [frontend-tests.worker.exports-test] [frontend-tests.worker.index-test] + [frontend-tests.worker.messages-test] [frontend-tests.worker.trim-test] [goog.object :as gobj])) @@ -226,6 +227,7 @@ 'frontend-tests.data.jobs-test 'frontend-tests.worker.exports-test 'frontend-tests.worker.index-test + 'frontend-tests.worker.messages-test 'frontend-tests.worker.trim-test 'frontend-tests.data.svg-upload-test 'frontend-tests.data.uploads-test diff --git a/frontend/test/frontend_tests/worker/messages_test.cljs b/frontend/test/frontend_tests/worker/messages_test.cljs new file mode 100644 index 0000000000..91ff4771eb --- /dev/null +++ b/frontend/test/frontend_tests/worker/messages_test.cljs @@ -0,0 +1,32 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns frontend-tests.worker.messages-test + (:require + [app.common.files.builder :as fb] + [app.common.types.objects-map :as omap] + [app.common.uuid :as uuid] + [app.util.object :as obj] + [app.worker.messages :as wm] + [cljs.test :as t :include-macros true] + [clojure.string :as str])) + +(defn- make-page + [] + (-> (fb/create-state) + (fb/add-file {:name "Test"}) + (fb/add-page {:name "Page 1"}) + (fb/add-shape {:type :rect :name "unique-shape-name" :x 0 :y 0 :width 100 :height 100}) + (fb/get-current-page))) + +(t/deftest objects-map-shapes-skip-transit + (let [page (update (make-page) :objects omap/wrap) + encoded (wm/encode {:sender-id (uuid/next) + :payload {:cmd :index/initialize :page page}})] + (t/is (not (str/includes? (obj/get encoded "payload") "unique-shape-name"))) + (t/is (instance? js/Map (obj/get encoded "pageObjects"))) + (t/is (= (count (:objects page)) + (.-size (obj/get encoded "pageObjects"))))))