New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Large values in app state slow down transactions #556

Closed
glv opened this Issue Dec 29, 2015 · 6 comments

Comments

Projects
None yet
3 participants
@glv
Copy link

glv commented Dec 29, 2015

Transacting large values into the application state in om next makes things very slow, even if those values are not involved in queries.

(ns slowomtrans.core
  (:require-macros [cljs.core.async.macros :refer [go go-loop]])
  (:require [cljs.core.async :as async]
            [goog.dom :as gdom]
            [om.next :as om :refer-macros [defui]]
            [om.dom :as dom]))

(enable-console-print!)

(declare reconciler)

(def init-data
  {:active false

   :counter1 0
   :blob1 nil

   :counter2 0
   :blob2 nil

   :counter3 0
   :blob3 nil})

(defn make-blob [size]
  {:n 0
   :values (into [] (for [i (range size)]
                      {:pos i}))})

(defn run-counter [blob bchan wrap?]
  (go-loop [{n :n :as blob} blob]
    (async/>! bchan (if wrap? (atom blob) blob))
    (if (< n 50)
      (recur (update blob :n inc))
      (async/close! bchan))))

(defn start-counter [blob-size counter-key blob-key wrap?]
  (om/transact! reconciler `[(set-val {:state-key :active :value true})])
  (let [bchan (async/chan)]
    (go-loop []
             (let [blob (async/<! bchan)]
               (if blob
                 (let [{n :n} (if wrap? @blob blob)]
                   (om/transact! reconciler `[(set-val {:state-key ~counter-key :value ~n})
                                              (set-val {:state-key ~blob-key :value ~blob})])
                   (async/<! (async/timeout 1))
                   (recur))
                 (om/transact! reconciler `[(set-val {:state-key :active :value false})]))))
    (run-counter (make-blob blob-size) bchan wrap?)))

;; -----------------------------------------------------------------------------
;; Parsing

(defn read [{:keys [state]} key _]
  (let [st @state]
    (if-let [[_ value] (find st key)]
      {:value value}
      {:value :not-found})))

(defn mutate [{:keys [state] :as env} key {:keys [state-key value] :as params}]
  (if (= 'slowomtrans.core/set-val key)
    {:value {:keys [state-key]}
     :action #(swap! state assoc state-key value)}
    {:value nil}))

;; -----------------------------------------------------------------------------
;; Components

(defui DemoPanel
       static om/IQuery
       (query [this]
              '[:active :counter1 :counter2 :counter3])
       Object
       (render [this]
               (let [{:keys [active counter1 counter2 counter3] :as state} (om/props this)]
                 (dom/div
                   nil
                   (dom/div
                     nil
                     (dom/button #js {:disabled active
                                      :onClick (fn [e] (start-counter 100 :counter1 :blob1 false))}
                                 "Counter 1")
                     (dom/input #js {:type "text"
                                     :value counter1
                                     :style #js {:width "25px"}
                                     :disabled true})
                     (dom/span nil " <-- Small vector"))
                   (dom/div
                     nil
                     (dom/button #js {:disabled active
                                      :onClick (fn [e] (start-counter 10000 :counter2 :blob2 true))}
                                 "Counter 2")
                     (dom/input #js {:type "text"
                                     :value counter2
                                     :style #js {:width "25px"}
                                     :disabled true})
                     (dom/span nil " <-- Large vector (hidden from the parser in an atom)"))
                   (dom/div
                     nil
                     (dom/button #js {:disabled active
                                      :onClick (fn [e] (start-counter 10000 :counter3 :blob3 false))}
                                 "Counter 3")
                     (dom/input #js {:type "text"
                                     :value counter3
                                     :style #js {:width "25px"}
                                     :disabled true})
                     (dom/span nil " <-- Large vector"))
                   ))))

(def demo-panel (om/factory DemoPanel))

;; -----------------------------------------------------------------------------
;; Initialization

(def reconciler
  (om/reconciler {:state init-data
                  :parser (om/parser {:read read :mutate mutate})
                  ;; set the logger to nil, because formatting the log output
                  ;; slows things down and obscures the real problem.
                  :logger nil
                  }))

(om/add-root! reconciler
              DemoPanel (gdom/getElement "app"))
@swannodette

This comment has been minimized.

Copy link
Member

swannodette commented Dec 29, 2015

Do not link to external repos. Please put all source inline.

@glv

This comment has been minimized.

Copy link

glv commented Dec 29, 2015

Gotcha. Here's some extra explanatory text from the index.html in that repo:

Each of the three counters is incremented in a background core.async go-loop, from 1 to 50. Each counter is accompanied by a vector of maps, which is transacted into the app state alongside the counter. However, the vectors are never referred to by a component query, nor are they referenced in any way during rendering. They are simply added to the application state each time the counter is updated.

If you run the counters in order, you will see that if the vector is small (100 elements) the loop runs quickly. If the vector is large (10K elements) but "hidden" from the parser by being wrapped in an atom, it also runs quickly. But when a 10K-element vector is transacted directly into the application state, it slows things down a lot.

This may seem contrived, but the real use comes from Snergly, a practice project where I'm implementing maze-related algorithms in Clojure and animating those algorithms in the browser. I use Om Next to implement the control panel where you choose the maze algorithm and grid size (and eventually other parameters) and then the algorithm runs using core.async, sending each grid update through a channel, at the other end of which it is transacted into the app state and then used to draw the current state of the grid on a canvas.

Interestingly, once counter 3 has been run after a reload, counter 1 and counter 2 become slow as well.

@swannodette

This comment has been minimized.

Copy link
Member

swannodette commented Dec 29, 2015

I suspect the problem is om.next.parser/path-meta, we should probably use the query do determine what values we should just skip. @glv reports that this was the hot spot under the Chrome profiler confirming my suspicion.

glv added a commit to glv/snergly that referenced this issue Dec 30, 2015

Wrap grid in atom to hide from parser.
This is to avoid the slowness in om next when transacting a very
large value into the application state (see omcljs/om#556). Since
we never need to query *within* these grid values, we don't need
om to go delving inside them to do its indexing.
@glv

This comment has been minimized.

Copy link

glv commented Dec 30, 2015

I've dug a bit deeper into this. Putting a few comments here for when you get around to the issue.

I'm not sure there's a way to automatically determine that a value should be skipped given the information currently available within the parser. But this is really an application design decision: there are a few values that I want to put in the application state (for uniformity, and because I'd like them to be part of the history) but they are large values and the internals will never need to be referenced directly by queries.

So it seems reasonable to let the application declare somehow that path-meta can ignore certain paths in the state … either in the result map returned by the mutate function or perhaps in the reconciler configuration. (The latter seems preferable to me.)

Alternatively, perhaps the workaround I'm using (hiding those large values within atoms) is actually the right solution to this, and should simply be documented as the right approach if you need to have large values in the application state that are opaque to queries.

@swannodette swannodette added this to the Beta 1 milestone Jan 13, 2016

@caleb

This comment has been minimized.

Copy link

caleb commented Jan 17, 2016

I think I am running into this issue as well, but I think my case is a little bit different.

As my state grows, it seems that calling om/transact! takes longer and longer. The difference between my app and glv's is that I am querying into that data via idents (so let's say I have 8 credit card statements, and each has 350 lines, my form's query will have only one statement shown at any particular time), and it seems like he has large blobs of data that he does not query into.

The result is if I run transact! in an onChange handler on an input in a line item, depending on how large my state is, there can be a noticeable delay even though I'm only updating field in a single line item.

This could be completely wrong, but what if your mutate function could indicate the scope of changes so that path-meta wouldn't have to crawl the whole graph?

@swannodette

This comment has been minimized.

Copy link
Member

swannodette commented Mar 15, 2016

fixed e238d47

glv added a commit to glv/snergly that referenced this issue Apr 5, 2016

No longer wrap the grid in an atom.
Thanks to a recently committed fix for omcljs/om#556, I no longer have
to wrap the grid in an atom within the application state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment