mirror of
https://github.com/penpot/penpot.git
synced 2026-11-10 03:50:33 +00:00
⚡ Send objects-map pages to the worker without a second encode (#12811)
* ⚡ 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 <luke_lau@icloud.com>
Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
parent
55360c52f5
commit
6846afd08c
@ -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"
|
||||
|
||||
@ -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)))))))
|
||||
|
||||
@ -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)))))
|
||||
|
||||
@ -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
|
||||
|
||||
32
frontend/test/frontend_tests/worker/messages_test.cljs
Normal file
32
frontend/test/frontend_tests/worker/messages_test.cljs
Normal file
@ -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"))))))
|
||||
Loading…
x
Reference in New Issue
Block a user