From 79aebcca49328c731e1ae3c81f8587d9a0074d63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bel=C3=A9n=20Albeza?= Date: Mon, 2 Nov 2026 12:12:57 +0100 Subject: [PATCH] :bug: Free the worker's copy of a page when leaving it (#12751) * :bug: Stop worker index updates from adding nil-keyed entries When a commit touched a page the worker had not indexed, or deleted the indexed page, `:index/update` ran the selection and snap updates with a nil page. That left entries under a nil key (and a stale selection index for a deleted page) for the rest of the session. Skip updates for pages the worker does not hold, since a later `:index/initialize` sends the whole page again, and drop the selection and snap indexes when the changes delete the page. AI-assisted-by: claude-opus-5-5 * :bug: Free the worker's copy of a page when leaving it The worker kept every visited page, with its selection and snap indexes, for the whole session. On large files each visited page added hundreds of MB to the worker heap. Add an `:index/finalize` worker command that drops a page and its indexes, and send it from `finalize-page`, which runs on every page switch and when leaving the workspace. Coming back to a page indexes it again from the full page, as before. Closes #12738 AI-assisted-by: claude-opus-5-5 --- .../src/app/main/data/workspace/pages.cljs | 7 +- frontend/src/app/worker/index.cljs | 45 ++++++--- frontend/test/frontend_tests/runner.cljs | 2 + .../frontend_tests/worker/index_test.cljs | 99 +++++++++++++++++++ 4 files changed, 138 insertions(+), 15 deletions(-) create mode 100644 frontend/test/frontend_tests/worker/index_test.cljs diff --git a/frontend/src/app/main/data/workspace/pages.cljs b/frontend/src/app/main/data/workspace/pages.cljs index c8b8180c41..ea76aa091c 100644 --- a/frontend/src/app/main/data/workspace/pages.cljs +++ b/frontend/src/app/main/data/workspace/pages.cljs @@ -147,7 +147,12 @@ :workspace-focus-selected))] (cond-> state - exit? (dissoc :workspace-drawing)))))) + exit? (dissoc :workspace-drawing)))) + + ptk/EffectEvent + (effect [_ _ _] + ;; Free the worker's copy of the page and its indexes + (mw/emit! {:cmd :index/finalize :page-id page-id})))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Workspace Page CRUD diff --git a/frontend/src/app/worker/index.cljs b/frontend/src/app/worker/index.cljs index 1117440563..cafcf0c591 100644 --- a/frontend/src/app/worker/index.cljs +++ b/frontend/src/app/worker/index.cljs @@ -35,27 +35,44 @@ (log/dbg :hint "page indexed" :id (:id page) :elapsed elapsed ::log/sync? true)))) nil)) +(defmethod impl/handler :index/finalize + [{:keys [page-id]}] + (swap! state #(-> % + (update :pages-index dissoc page-id) + (update ::selection dissoc page-id) + (update ::snap dissoc page-id))) + (log/dbg :hint "page index dropped" :id page-id ::log/sync? true) + nil) + (defmethod impl/handler :index/update [{:keys [page-id changes] :as message}] (let [tpoint (ct/tpoint-ms)] (try - (let [old-page (dm/get-in @state [:pages-index page-id]) - new-page (-> state - (swap! ch/process-changes changes false) - (dm/get-in [:pages-index page-id])) + (if-let [old-page (dm/get-in @state [:pages-index page-id])] + (let [new-page (-> state + (swap! ch/process-changes changes false) + (dm/get-in [:pages-index page-id]))] + (if (nil? new-page) + ;; The changes deleted the page + (swap! state #(-> % + (update ::snap dissoc page-id) + (update ::selection dissoc page-id))) - text-rects (dm/get-in @state [::text-rect page-id]) + (let [text-rects (dm/get-in @state [::text-rect page-id]) - ;; Update page objects with the text data - new-page - (reduce-kv - (fn [page id data] - (update-in page [:objects id] d/patch-object data)) - new-page - text-rects)] + ;; Update page objects with the text data + new-page + (reduce-kv + (fn [page id data] + (update-in page [:objects id] d/patch-object data)) + new-page + text-rects)] - (swap! state update ::snap snap/update-page old-page new-page) - (swap! state update ::selection selection/update-page old-page new-page)) + (swap! state update ::snap snap/update-page old-page new-page) + (swap! state update ::selection selection/update-page old-page new-page)))) + + ;; A later `:index/initialize` sends the whole page again + (log/dbg :hint "page not indexed, skipping update" :id page-id ::log/sync? true)) (catch :default cause (log/error :hint "error updating page index" :id page-id :cause cause)) (finally diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index c83530fae5..bf5fe1e8d5 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -161,6 +161,7 @@ [frontend-tests.util.dom.dnd-test] [frontend-tests.worker-snap-test] [frontend-tests.worker.exports-test] + [frontend-tests.worker.index-test] [goog.object :as gobj])) (enable-console-print!) @@ -195,6 +196,7 @@ 'frontend-tests.data.imports-test 'frontend-tests.data.jobs-test 'frontend-tests.worker.exports-test + 'frontend-tests.worker.index-test 'frontend-tests.data.svg-upload-test 'frontend-tests.data.uploads-test 'frontend-tests.data.viewer-test diff --git a/frontend/test/frontend_tests/worker/index_test.cljs b/frontend/test/frontend_tests/worker/index_test.cljs new file mode 100644 index 0000000000..a5e0f558f0 --- /dev/null +++ b/frontend/test/frontend_tests/worker/index_test.cljs @@ -0,0 +1,99 @@ +;; 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.index-test + (:require + [app.common.files.builder :as fb] + [app.common.geom.rect :as grc] + [app.common.types.shape :as cts] + [app.common.uuid :as uuid] + [app.worker.impl :as impl] + [app.worker.index :as idx] + [cljs.test :as t :include-macros true])) + +(t/use-fixtures :each + {:before #(reset! idx/state {:pages-index {}})}) + +(defn- make-page + [name] + (-> (fb/create-state) + (fb/add-file {:name "Test"}) + (fb/add-page {:name name}) + (fb/add-shape {:type :rect :x 0 :y 0 :width 100 :height 100}) + (fb/get-current-page))) + +(defn- initialize + [page] + (impl/handler {:cmd :index/initialize :page page})) + +(defn- update-index + [page-id changes] + (impl/handler {:cmd :index/update :page-id page-id :changes changes})) + +(defn- indexed-ids + [state] + {:pages (set (keys (:pages-index state))) + :selection (set (keys (::idx/selection state))) + :snap (set (keys (::idx/snap state)))}) + +(defn- add-rect-change + [page-id shape-id] + (let [shape (cts/setup-shape {:id shape-id :type :rect + :x 200 :y 200 :width 50 :height 50})] + {:type :add-obj + :id shape-id + :page-id page-id + :parent-id uuid/zero + :frame-id uuid/zero + :obj shape})) + +(t/deftest finalize-drops-only-that-page + (let [page-a (make-page "Page A") + page-b (make-page "Page B")] + (initialize page-a) + (initialize page-b) + (impl/handler {:cmd :index/finalize :page-id (:id page-a)}) + (t/is (= {:pages #{(:id page-b)} + :selection #{(:id page-b)} + :snap #{(:id page-b)}} + (indexed-ids @idx/state))))) + +(t/deftest finalize-unknown-page-is-noop + (let [page (make-page "Page 1")] + (initialize page) + (let [before @idx/state] + (impl/handler {:cmd :index/finalize :page-id (uuid/next)}) + (t/is (= before @idx/state))))) + +(t/deftest update-ignores-pages-not-indexed + (let [page (make-page "Page 1") + page-id (uuid/next)] + (initialize page) + (update-index page-id [(add-rect-change page-id (uuid/next))]) + (t/is (= {:pages #{(:id page)} + :selection #{(:id page)} + :snap #{(:id page)}} + (indexed-ids @idx/state))))) + +(t/deftest update-drops-indexes-of-deleted-page + (let [page (make-page "Page 1") + page-id (:id page)] + (initialize page) + (update-index page-id [{:type :del-page :id page-id :page-id page-id}]) + (t/is (= {:pages #{} :selection #{} :snap #{}} + (indexed-ids @idx/state))))) + +(t/deftest update-indexes-new-shapes-of-indexed-page + (let [page (make-page "Page 1") + page-id (:id page) + shape-id (uuid/next)] + (initialize page) + (update-index page-id [(add-rect-change page-id shape-id)]) + (t/is (contains? (get-in @idx/state [:pages-index page-id :objects]) shape-id)) + (t/is (contains? (set (impl/handler {:cmd :index/query-selection + :page-id page-id + :rect (grc/make-rect 190 190 70 70)})) + shape-id))))