✨ Cache the transit reader when decoding strings (#12704)

* ✨ Cache the transit reader when decoding strings

On the cljs side, we create a new transit reader on every call to decode-str.
This turns out to have a fair bit overhead in both transit-cljs and transit-js,
since it has to merge the handlers, call clj->js, allocate a new ReadCache,
etc.

By reusing it the measured decode time goes from ~58us -> ~41us, so about 1.4x
faster. This particularly helps for some operations like whenever a component
is edited and every object needs to be decoded, see
https://github.com/penpot/penpot/issues/9344, although they could probably be
made more efficient in other ways.

The reader usually clears its read cache after a successful read, but whenever
it throws it hangs around. So in this case we need to reset the cache
ourselves, otherwise the next read will be from a clobbered cache.

The tests were written by Claude Sonnet 5.5, and should cover the clobbering
behaviour above.

Signed-off-by: Luke Lau <luke@igalia.com>

* Keep one transit reader per type and rebuild it on add-handlers!

---------

Signed-off-by: Luke Lau <luke@igalia.com>
Co-authored-by: Andrey Antukh <niwi@niwi.nz>

common/src/app/common/transit.cljc

@@ -31,6 +31,24 @@
 (def write-handler-map (atom nil))
 (def read-handler-map (atom nil))
 
+;; Readers are built lazily on first use and reused afterwards. Building a
+;; reader on every call is expensive: transit merges the handlers, allocates
+;; a read cache and prepares the marshaller machinery. A reader captures the
+;; handler maps it was created with, so `add-handlers!` replaces them.
+#?(:cljs
+   (def ^:private ^:dynamic *json-reader* nil))
+
+#?(:cljs
+   (def ^:private ^:dynamic *json-verbose-reader* nil))
+
+#?(:cljs
+   (defn- rebuild-readers
+     []
+     (set! *json-reader*
+           (delay (t/reader :json {:handlers @read-handler-map})))
+     (set! *json-verbose-reader*
+           (delay (t/reader :json-verbose {:handlers @read-handler-map})))))
+
 ;; A generic pointer; mainly used for deserialize backend pointer-map
 ;; instances that serializes to pointer but may in other ways.
 (deftype Pointer [id metadata]
@@ -95,6 +113,10 @@
 
       (reset! write-handler-map #?(:clj (t/write-handler-map cwh) :cljs cwh))
       (reset! read-handler-map #?(:clj (t/read-handler-map crh) :cljs crh))
+      ;; A reader captures the handler maps, so build new ones from the maps
+      ;; just stored. A handler registered after a read then applies on the
+      ;; next read.
+      #?(:cljs (rebuild-readers))
       nil)))
 
 ;; --- HANDLERS
@@ -284,10 +306,19 @@
   ([data] (decode-str data nil))
   ([data opts]
    #?(:cljs
-      (let [type   (:type opts :json)
-            params {:handlers @read-handler-map}
-            reader (t/reader type params)]
-        (t/read reader data))
+      (let [reader (case (:type opts :json)
+                     :json         @*json-reader*
+                     :json-verbose @*json-verbose-reader*)]
+        (try
+          (t/read reader data)
+          (catch :default cause
+            ;; The reader clears its internal read cache after a successful
+            ;; read but not after a failed one, so a reused reader would
+            ;; resolve cache codes against the failed read. Clear it before
+            ;; propagating the error.
+            (when-let [cache (.-cache reader)]
+              (.clear cache))
+            (throw cause))))
       :clj
       (-> (str->bytes data)
           (decode opts)))))

common/test/common_tests/runner.cljc

@@ -96,6 +96,7 @@
    [common-tests.text-test]
    [common-tests.time-test]
    [common-tests.token-test]
+   [common-tests.transit-test]
    [common-tests.types-shape-tree-test]
    [common-tests.types.absorb-assets-test]
    [common-tests.types.color-test]
@@ -209,6 +210,7 @@
    'common-tests.text-test
    'common-tests.time-test
    'common-tests.token-test
+   'common-tests.transit-test
    'common-tests.types-shape-tree-test
    'common-tests.types.absorb-assets-test
    'common-tests.types.color-test

common/test/common_tests/transit_test.cljc

@@ -0,0 +1,45 @@
+;; 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 common-tests.transit-test
+  (:require
+   [app.common.transit :as tr]
+   [clojure.test :as t]))
+
+;; A handler that always fails, to make a read throw after the reader has
+;; already written to its read cache.
+#?(:cljs
+   (tr/add-handlers!
+    {:id "test/throws"
+     :rfn (fn [_] (throw (js/Error. "invalid representation")))}))
+
+(t/deftest decode-str-round-trip
+  (let [data {:a 1 :b [:c "d"] :e #{1 2}}]
+    (t/is (= data (-> data tr/encode-str tr/decode-str)))
+    (t/is (= data (-> data (tr/encode-str {:type :json-verbose})
+                      (tr/decode-str {:type :json-verbose}))))))
+
+;; `decode-str` reuses the readers across calls, but `add-handlers!` rebuilds
+;; them, so a handler registered after the first read still applies.
+#?(:cljs
+   (t/deftest decode-str-uses-handlers-added-after-the-first-read
+     (t/is (= [:x] (tr/decode-str "[\"~:x\"]")))
+     (tr/add-handlers! {:id "test/late" :rfn (fn [v] (str "late:" v))})
+     (t/is (= "late:x" (tr/decode-str "[\"~#test/late\",\"x\"]")))))
+
+;; In ClojureScript `decode-str` reuses the same reader across calls, and a
+;; read that throws leaves that reader's read cache dirty. The next message
+;; would then resolve its cache codes ("^0", "^1") to entries of the failed
+;; one if the cache is not cleared.
+#?(:cljs
+   (t/deftest decode-str-after-a-failed-read
+     (let [good     "[\"~:aaaa\",\"~:bbbb\",\"^0\",\"^1\"]"
+           bad      "[\"~:cccc\",[\"~#test/throws\",1]]"
+           expected [:aaaa :bbbb :aaaa :bbbb]]
+       (t/is (= expected (tr/decode-str good)))
+       (t/is (thrown? js/Error (tr/decode-str bad)))
+       (t/is (= expected (tr/decode-str good)))
+       (t/is (= expected (tr/decode-str good))))))