Fork me on GitHub
#reagent
<
2022-03-14
>
Pepijn de Vos15:03:19

I'm trying to profile my app and seeing a ton of G_number with reason "rendered for the first time" what can I do to locate the problem?

p-himik15:03:42

Those G__number look like they're coming from (gensym). Why it's there - no clue. But the problem is not those tiny components but rather the nyancad.mosaic.editor.schematic_elements component - it takes the longest time by itself.

Pepijn de Vos15:03:57

Well okay I have a pretty good idea what the problem is, I'm using higher order functions as reagent components, so I think those are just the names of anonymous functions you're seeing I'm guessing.

Pepijn de Vos15:03:49

This is schematic_elements

(defn schematic-elements [schem]
  [:<>
   (for [[k v] schem
         :when (= "wire" (:cell v))]
     ^{:key k} [(get-model ::bg v) k v])
   (for [[k v] schem
         :when (not= "wire" (:cell v))]
     ^{:key k} [(get-model ::bg v) k v])
   (for [[k v] schem]
     ^{:key k} [(get-model ::sym v) k v])
   (for [[k v] schem]
     ^{:key k} [(get-model ::conn v) k v])])

p-himik15:03:12

Yeah, I know where the repo is. :) Looking at the overall code right now. Try using JS profiler instead of the React one and see what takes most of the time.

1
p-himik15:03:49

Also, why are you using #' in CLJS? I know that it's "that one weird trick" to make some functions REPL-friendly, but that's in CLJ.

Pepijn de Vos15:03:26

Because that function is an advance declaration that gets defined later I think so without that it'd just store undefined in the map I think

p-himik15:03:53

I'm 90% certain you're incorrect here.

p-himik15:03:11

Now 100% certain. :) Just tested it.

p-himik15:03:56

Ah, you're using set!... Not sure, hmm.

p-himik15:03:24

Tried with set! - works without #' just fine as well. But I would definitely reconsider that approach. Using the namespace as dynamic data storage is not great.

p-himik15:03:40

You can definitely move most if not all that stuff out of init. You run init right after loading the editor's JS bundle anyway, and initializing those particular vars doesn't require anything that's not available right then and there. When the namespace is being loaded, all that js/window ... stuff is already available. Unless you also call init with some custom arguments somewhere.

Pepijn de Vos16:03:35

Yea the idea is that I can re-init the app with a different design. Primarily when embedding in another app. It used to be statically initialized indeed.

Pepijn de Vos16:03:46

Though it's not currently used in an embedded context, so I could consider statically initializing and using set! if you absolutely need to reinitialise.

Pepijn de Vos16:03:40

I don't see schematic-elements in the JS profiler at all: https://share.firefox.dev/35X5IPd

p-himik16:03:33

Then I'd gather all such "almost static" values in a single ctx map and pass it around wherever it's needed. This would also have a benefit if being able to embed multiple editors within the same app. The avoid having to write long things like (get-in @...), just extract all them in their own getters. It's actually a decent rule of thumb - whenever you see yourself repeating (get-in x [:a :b :c]) more than a couple of times, extract it. Same with assoc et al. Those extracted functions would of course accept the ctx context as the first argument. Then working with all such values will become easy.

p-himik16:03:31

> I don't see schematic-elements in the JS profiler at all: https://share.firefox.dev/35X5IPd That's a very neat web page, I had no idea Firefox had something like that. With that being said, I have absolutely no clue what I'm looking at. Nothing there seem to even mention your code - there's only ... stuff. Feels like you managed to profile the DevTools? :)

Pepijn de Vos16:03:01

lol yea it's the fancy new debugger somehow I have no idea how it works

p-himik16:03:58

Alright, I managed to find some CLJS code being mentioned in there, with all file names replaced with <URL>. But that code is definitely in the minority, it's almost not there. Is it just the built-in FF profiler or something else?

Pepijn de Vos16:03:20

yea apparently they replaced the profiler with this

Pepijn de Vos16:03:25

I'll try what chrome says

p-himik16:03:39

Seems like _element takes a lot of time. And maybe it's because of your higher-order functions. Reagent can't cache components for them, so it creates a new React class every single time you use partial in Hiccup.

Pepijn de Vos16:03:34

Right yea in Chrome I also really don't see my own code show up

p-himik16:03:40

To fix that, instead of creating and returning a new function every time, you can return the same function along with an argument vector/map the eventual Hiccup vector will need. The code that needs to build a Hiccup vector out of it will then conj something to that vector or assoc something to that map and simply do (into [component] args) or [component args]`.

p-himik16:03:13

I'd go with the map option since combining arguments and creating Hiccup vectors becomes incredibly easy.

Pepijn de Vos16:03:12

I'll have a look. Thanks a lot 🙂

👍 1
Pepijn de Vos17:03:27

This morning I was trying to do stuff with Conda and CouchDB and everyone is just telling me what not to do but not giving a useful way out. And here you are "oh I know where your code is so I'm taking a look" haha quite a contrast.

😄 1
Pepijn de Vos17:03:26

Uh... I removed all the #' and now all my symbols are gone, so that doesn't work... I mean it does if you live reload but not from a fresh start I guess

Pepijn de Vos17:03:51

But using vectors instead of HOF made a ton of stuff not render at least.

p-himik17:03:39

Regarding #' - weird. Maybe I tested incorrectly. Either way, I would definitely recommend getting rid of all that stuff with declare, set!, #'. > But using vectors instead of HOF made a ton of stuff not render at least. In a good way? Or do you mean that some things are just missing?

Pepijn de Vos17:03:33

Not that performance is a lot better... still spending a lot of time in the function itself supposedly.

Pepijn de Vos17:03:44

But according to the chrome profiler it really isn't my function itself that's taking all the time

p-himik17:03:26

Can you share the profile result file?

Pepijn de Vos17:03:54

On the reagent side there is sill some stuff that's rerendering because "props changed" which I'm not sure why that is exactly so lots more digging to do

Pepijn de Vos17:03:29

The upside is, I only noticed this after a day of testing adding endless amounts of wires to fix some bugs. Under normal use the delays should be much more reasonable. But still, making it faster is always good. In particular because my target audience is kinda conservative and skeptical about web technology.

p-himik17:03:43

Just as an extra 2 c of information - (fn []) is not equal to (fn []). So maybe you're passing dynamically constructed functions around, that would make some components re-render without a real need.

Pepijn de Vos17:03:55

Yea probably most of these rerenders are HOF hacks I shouldn't be doing.

p-himik17:03:53

Also, you're profiling a dev build - the results will differ significantly from a prod build. E.g. in your case as-element takes a huge amount of time, relatively speaking, because it uses prewalk at some point. In prod, it will simply call str instead.

Pepijn de Vos17:03:38

ah yea when will I learn not to profile dev builds :face_palm:

p-himik17:03:02

I do it all the time myself. :D When something is slow, it's will often be slow regardless of the build.

Pepijn de Vos17:03:13

> Profiling support requires either a development or profiling build of React v16.5+.

Pepijn de Vos17:03:51

Anyway enough profiling for today...

p-himik17:03:20

Yeah, I rarely use React profiling tools. Only when I know exactly that the problem is on that side and not with some algorithm somewhere.

p-himik17:03:46

React profiling will help you figure out why a component is being re-rendered. JS profiling will help you with everything else.

1
Stuart21:03:06

I have a re-frame app that I'm trying to add cytoscape to. On an event firing I want to add elements and edges to a graph in cytoscape. I tried this with a form-3 reagent function.

(defn graph []
  (rg/create-class
   {:component-did-update
    (fn []
      (let [cy (cytoscape #js {:container (.getElementById js/document "cy"),
                               :layout    #js {:name "grid" :rows 2 :columns 2}})
            elems @(rf/subscribe [:elements])]
        (js/console.log "here!!!")
        (.add cy elems)))
    :reagent-render
    (fn []
      [:div#cy])}))
The problem here is, this bit:
cy (cytoscape #js {:container (.getElementById js/document "cy"),
                               :layout    #js {:name "grid" :rows 2 :columns 2}})
Gives me a new instance of cytoscape each time, therefore anything I add just gets added to a new empty cytoscape graph. I thought I could just have a re-frame reg-fx, that I could then use from my reg-event-db, something like:
(rf/reg-fx
 :scroll-current-code-into-view
 (fn [elem last-elem]
   ;; do other stuff here, like find last item I added and change its style, etc...
   (.add cy elem)))
But then at that point cy is undefined. I thought I could have a def , something like:
(def cy (cytoscape #js {:container (.getElementById js/document "cy")
                        :layout    #js {:name "grid" :rows 2 :columns 2}}))
But where do I put this, if I put at the bottom of app.cljs, I get an error in the console. I presume because it's looking for cy element in the dom before the DOM is ready. The error:
main.js:2231 An error occurred when loading exfn.app.js
TypeError: Cannot read properties of null (reading 'className')
    at Renderer.BRp$f.init (cytoscape.cjs.js:26944:9)
    at  (cytoscape.cjs.js:26924:3)
    at new Renderer (cytoscape.cjs.js:31699:7)
    at Core.initRenderer (cytoscape.cjs.js:14454:28)
    at new Core (cytoscape.cjs.js:18642:3)
    at cytoscape (cytoscape.cjs.js:31886:12)
    at eval (app.cljs:123:57)
    at eval (<anonymous>)
    at Object.goog.globalEval (main.js:836:21)
    at Object.env.evalLoad (main.js:2229:12)
Also tried a defonce but again, no bueno. I could have it in a form-3 function, and then have it re-render on each change to my graph elements, but that is potentially re-rendering the graph a LOT (I'd like to updating the graph every 25 ms or so, at least). So I think it will be a lot faster if I can use the cytoscape functions like add on an existing graph. Help, I'm confused 😕

p-himik21:03:25

You're on the right path with rg/create-class:

(defn graph []
  (let [cy (cytoscape ...)]
    (rg/create-class ...)))
^ that's a common pattern when dealing with stateful JS-based components. The React lifecycle methods should update that cy.

p-himik21:03:56

Ah, scratch that - you need a DOM node there.

Stuart21:03:48

yeah, its that pesky cy div that is causing me issues i think!

Stuart21:03:19

Have you any experience with any libraries that can visualise graphs that might be better than cytoscape ?

p-himik21:03:37

;; Notice how I pass `elements` directly - don't miss re-frame and
;; stateful JS components. Put Reagent in between.
;; Just wrap this `graph` in another component and use `subscribe` there.
;; Pass the real value to this component.
(defn graph [#_{:keys [elements]}]
  (let [cy (atom nil)
        ref (react/createRef)]
    (rg/create-class
      {:reagent-render
       (fn [_props]
         [:div {:ref ref}])

       :component-did-mount
       (fn [_this]
         (when-some [node (.-current ref)]
           (reset! cy (cytoscape #js {:container node
                                      :layout    #js {:name "grid" :rows 2 :columns 2}}))))

       :component-did-update
       (fn [this [_ old-props]]
         (let [{:keys [elements]} (rg/props this)]
           (when (not= elements (:elements old-props))
             ;; Maybe you gotta remove the old ones first?
             (.add @cy elements))))})))

p-himik21:03:40

Something like that.

p-himik21:03:22

I've never had to deal with Cytoscape so don't know whether what I had to deal with is better. In CLJS, I have only had to work with Vega. It was alright, nothing special. But the usage pattern is exactly like above.

Stuart21:03:58

Thanks for that, your code makes sense to me! Never realised I could put the rg/create-class within the let

p-himik21:03:16

Forgot to mention - never use .getElementById. Except for the very first usage - when you render your whole app. React refs are the thing that should be used instead.

👍 1
Stuart21:03:34

I'm just trying to visualise some code as its running as a graph that im updating as it runs

Stuart21:03:49

I'll have a look at vega too! thanks

p-himik21:03:48

Maybe Vega would be enough there, no clue. But again - won't be different in its usage. It's just the pain of combining stateful JS components with Reagent. But you'll get comfortable with it soon enough, it's no big deal. Also, I definitely recommend going through Reagent examples. It has every single usage scenario that I've seen before covered - including this one.

Edward Ciafardini01:03:46

Reagent examples meaning the intro page on the Reagent documentation?

p-himik07:03:11

The examples dir in the repo. Also the docs dir there.