Fork me on GitHub
#fulcro
<
2021-12-29
>
zeitstein12:12:25

My app renders fairly large trees – currently I plan to store thousands of nodes in the client db. However, since any branch could be toggled visible/hidden, the denormalization algorithm would constantly do needless work (extremely so when rendering from tree's root). I'm wondering whether it would be worth it to modify the algorithm to 'teach' it stop the recursion once it reaches a (currently) hidden branch? (I'm imagining this would not be too difficult.) Began thinking about it because I noticed minor sluggishness (playing around in a performance-dev build with several thousand nodes), when basically nothing gets rendered. (To be clear, I'm impressed how well it's scaled. The UI is just toggle buttons and text at the moment, so the performance can only degrade from here.) Of course I should properly test and measure down the line, all that – I'm just wondering whether this is one of those situations where I shouldn't even bother thinking about it? Looking for quick answers and gut checks 🙂

tony.kay16:12:23

Did you measure where the problem starts to be? For example, high nodes should probbably return true from shouldComponentUpdate to prevent a large comparison that will mostly return true anyway. Do you know it is denormalization that is the problem? If it is, then the next question is: Is this read-only data? Does it need to be normalized?

tony.kay16:12:26

certainly you will eventually reach a point of performance problem if you have enough nodes. Avoiding normalization for those cases is an easy fix if the nodes are not actually written to...e.g. keep a set of expanded nodes in some other place for rendering, etc.

tony.kay16:12:58

Did you read the book sections on performance?

tony.kay16:12:56

But at the end of the day if you want to plug in a renderer of your own, you can...and that rendering algorithm could use a custom db->tree that you write for your purposes that stops when it realizes there's no more useful work to do.

tony.kay16:12:46

just look at keyframe or keyframe2 renderer as a base. The multi-root renderer should be considered deprecated in favor of hooks

zeitstein07:12:43

> Did you read the book sections on performance? I have, though this time around I was looking for denormalization-related stuff. I see now what you mean about shouldComponentUpdate. > Do you know it is denormalization that is the problem? No 🙂 Denormalization is one thing that came to mind and wanted to check whether it was worth pursuing. It is not read-only data. What I'm hearing is that I should try other optimizations first, but if I decide to follow my idea the way to do it would be a custom renderer. (Probably end up making just a few modification to defaults.) Thanks, Tony!

Jakub Holý (HolyJak)15:12:39

Keep us posted 🙏

👍 1
zeitstein11:01:46

Happy New Year! I've enabled https://book.fulcrologic.com/#_render_middleware and have been doing tests. The setup is: • keyframe2 renderer • I'm using only-refresh on the toggle transaction. • I'm measuring time passed between toggle button press and a console.log call in modified component's shouldComponentUpdate, trying to isolate denormalization times. (This will include the time to run the toggle mutation itself, but that is around 10ms.) • Using a performance-dev build. • Standard transaction processing. • SCU returns true for top-most components. (Though this is not important with only-refresh.) Observations: 1. The measured interval scales with the amount of nodes relevant to the denormalization algorithm (e.g. from where in the data tree the UI starts rendering from). For 4k relevant nodes, this interval is ~90ms, and it roughly doubles from there when doubling the number of nodes. So, it seems there is room for tailoring the denormalization algorithm. 2. However, given the (correct) FDG advice that we should change CSS instead of DOM, I've realised that my original idea of modifying the algorithm would not work – it would change the DOM (potentially thousands of nodes). 3. Given that the denormalization times are constant (for fixed amount of nodes) and the reasonable expectation that the UI will always render orders of magnitude less nodes than what is in db, there might be an inflection point where my original idea makes sense. But I'm doubtful – for each data node there will be many DOM nodes, DOM is slow, etc. 4. only-refresh doesn't seem to work with synchronous transaction processing (still re-renders from root), so it ends up being slower than async. I'm assuming this is not surprising? (I have to read more on the transaction system.) So, I'm left not knowing what to do. On the one hand, it is not terribly slow. On the other – it's a constant penalty paid on every interaction. One obvious thing is to try to keep the client db light. This is a much more complicated task, balancing recursion limits (UI and load), incremental loading, the fact that trees can grow very fast, etc.

zeitstein13:01:06

Don't know why I haven't thought of this before, but: I could get a feel for the size of the problem by measuring db->tree. So, denormalizing a tree with 4k nodes takes about 60ms (`simple-benchmark`). (Just as a reference point, pulling the same tree directly from Datascript (1.3.0) is 3 times faster.)

zeitstein14:01:01

@U0CKQ19AQ, I've tracked down why the following happens: > only-refresh doesn't seem to work with synchronous transaction processing (still re-renders from root)... The local action works as expected, but the remote action triggers a render from root.

tony.kay15:01:39

that option definitely depends on the optimized renderer installed

tony.kay15:01:08

oh, you mean you install sync tx processing as the tx processor? not called a !! variant?

tony.kay15:01:29

Yeah, that's probably a bug in passing through the txn options

tony.kay15:01:09

On the performance: I'm very open to an optimized db->tree. I have neither had the time nor inclination (since it has always been fast enough for my uses) to do it. I don't remember if I left a hook for configuring that (I don't think I did), but we could revamp the system to let you plug in a custom version, and then it would be guaranteed not to be a breaking change. At one point in the source we had generative tests against that algorithm...possibly still there. Wilker did them. We were A-B comparing a db->tree written with pathom vs the stock one. The fact that Datascript is 3x faster is surprising to me...are you certain you are doing an equivalent test, and that you're getting the exact same output from the two?

zeitstein15:01:52

> oh, you mean you install sync tx processing as the tx processor? not called a !! variant? Yup, that's what I mean. Everything else is equal (`transact!` with only-refresh option), I just switch out the standard tx processing with synchronous.

zeitstein16:01:43

> The fact that Datascript is 3x faster is surprising to me...are you certain you are doing an equivalent test, and that you're getting the exact same output from the two? I double-checked and yes – same output (not counting Fulcro-specific stuff like routing data). Datascript 1.3.0 improved pull performance drastically. There's https://github.com/tonsky/datascript/releases/tag/1.3.0: "pull has much more information ahead of time, which could be leveraged for optimizations", which I wonder if could be relevant for Fulcro (pull syntax ~ EQL and all that). I imagine Fulcro's algorithm has more stuff it needs to do. > we could revamp the system to let you plug in a custom version, and then it would be guaranteed not to be a breaking change. That would be awesome! (It would also allow me to fiddle with recursion, though there's stuff I still need to figure out there.) Though, I'm pessimistic about how much could I optimize. As I've mentioned before, I've realised that making the algorithm forego joining nodes which would be invisible on screen wouldn't work – it would change the DOM too much (and that must be more expensive than needless denormalization.)

tony.kay16:01:08

"I double-checked and yes – same output (not counting Fulcro-specific stuff like routing data)." is not making me very confident that you did an apples-to-apples comparison

tony.kay16:01:01

e.g. you use the exact same query against the exact same graph of data? The output should be identical, otherwise you are not measuring an equivalent thing

zeitstein16:01:19

I have thought of one thing that might be beneficial, maybe for other users of Fulcro as well. As I understand it, even when using only-refresh the whole subtree of the component being refreshed still gets denormalized (and tunnelled through), correct? Maybe there's a way to short-circuit that?

tony.kay16:01:44

in 99% of apps it really makes no appreciable difference

tony.kay16:01:13

I can count the number of times I've used only-refresh on, like, one finger

😁 1
tony.kay16:01:40

most applications do not have 4k nodes...that is an extremely rare case

tony.kay16:01:31

and since it is a rare case, I have a general practice of NOT complicating the platform to make an extreme corner case better

tony.kay16:01:49

the way React works, you simply cannot update a parent of a subtree without it, by definition, rendering the children. There is no way to have that "automatically controlled".

tony.kay16:01:17

the optimizations around that are built-in and automatic, but the fact is YOUR code has to render the VDOM so it can diff it

tony.kay16:01:42

shouldComponentUpdate and memoization (in hooks) are the tools we've got

tony.kay16:01:13

It would be useful if you could publish your performance comaprison code as a stand-alone repo

tony.kay16:01:29

(Datascript vs db->tree)

zeitstein17:01:15

> in 99% of apps it really makes no appreciable difference ... most applications do not have 4k nodes...that is an extremely rare case I don't doubt that at all. And even with 4k nodes, it's quite fast! 🙂 > I can count the number of times I've used `only-refresh` on, like, one finger And I think I'll be using it on most user interactions 🙂 (Which is why I've considered using the ident-optimized renderer, but realise that's not it reading more closely.) > and since it is a rare case, I have a general practice of NOT complicating the platform to make an extreme corner case better I absolutely understand :) > It would be useful if you could publish your performance comaprison code as a stand-alone repo Will do.

tony.kay17:01:01

a runnable repo would be best instead of a gist

zeitstein15:01:01

I have to do a bunch of reading to digest you deep tree demo, but wanted to get back to this in the meantime, try to make it clear: > As I understand it, even when using `only-refresh` the whole subtree of the component being refreshed still gets denormalized (and tunnelled through), correct? Maybe there's a way to short-circuit that? So: deep recursive tree, just want to update a single node (and I mean literally only the data in its (normalized) db entity). only-refresh only updates the single node (other instances of it, as well) and the only issue remaining is that it needs to denormalize the whole subtree starting at that node. So, what I'm thinking, for my use case: take the previous props of the node, simply assoc new stuff into it (and update db correspondingly), pass that as new props to the node. No denormalization required, and React does its diffing from there. That sounds quite doable? (Maybe traced-db->tree does something similar, but I'm yet to actually read through the code.) > Here it is: https://github.com/zeitstein/fulcro-datascript-denormalization-benchmark Let me know when you've had a chance to look at this, whether I'm making a mistake somewhere. If not, I'd be interested to know whether you think this means there's room to optimize the denormalization algorithm. (Which, to be clear, I would not expect you to do. Just wondering whether you think it might be worth pursuing.)

tony.kay17:01:27

So, you are right that Datascript manages to be about 2.4x faster on a raw pull, which is surprising to me on the surface. I assume it is because it has indexes to work with, so that some of the work is pushed off at transaction time instead of query. It may also have some smarter ways of doing loop detection.

tony.kay18:01:01

That said, 35ms for a 4k node denormalization is very fast. Dropping it to 15ms or so is nice, but of the same basic magnitude in user space. E.g. the scalability of Datascript isn't really any better (it goes up linearly with number of nodes). So, while you can say it is 2x faster in a micro benchmark, it really doesn't matter. At some point there is a limit where there are too many nodes, and denormalization during a render (of all of them) is just too expensive.

tony.kay18:01:24

Most UIs have 100s of nodes, not thousands.

tony.kay18:01:36

When you get to this specific problem, your choice is clear: reduce the number of nodes that have to be denormalized in a render. That said, you do not have to be so extreme as to make that number "1". If you can get it down into the 100's, then it'll generally be fine...which is what my demo shows. The tree can have 100,000 nodes in it, and as long as you don't try to open them all, it works fine, because it only has to denormalize what is open.

Timofey Sitnikov18:12:45

I built a tree to be displayed on the top of main component in fulcro-template, but I must be missing something and almost ready to pull my hair out. Here is the source:

(defsc Solution [_this {:solution/keys [id summary detail] :as _props}]
  {:query [:solution/id :solution/summary :solution/detail]
   :ident :solution/id}
  (dom/div
   (div (str "Solution ID: " id))
   (div (str "Summary: " summary))
   (div (str "Detail: " detail))))

(def ui-solution (comp/factory Solution {:keyfn :solution/id}))

(defsc Problem [_this {:problem/keys [id title summary solutions] :as _props}]
  {:query [:problem/id
           :problem/title
           :problem/summary
           :problem/solutions
           {:problem/solutions (comp/get-query Solution)}]
   :ident :problem/id}
  (dom/div
   (h2 (str "Problem ID - " id))
   (div (str "Title: " title))
   (div (str "Summary: " summary))
   (h3 "Solutions:")
   (ul (map ui-solution solutions))))

(def ui-problem (comp/factory Problem {:keyfn :problem/id}))

(defsc Main [_this {:main/keys [problems]}]
  {:query         [{:main/problems (comp/get-query Problem)}]
   :ident         (fn [] [:component/id :main])
   :route-segment ["main"]}
  (div :.ui.container.segment
    (h3 "Main - Problems:")
    (ul (map ui-problem problems))))
I use this command to add data:
(merge/merge-component! SPA Problem {:problem/id    1
                                       :problem/title "First Problem Title"
                                       :problem/summary "First Problem Summary"}
                          :append [:component/id :main  :problems])

  (merge/merge-component! SPA Problem {:problem/id    2
                                       :problem/title "Second Problem Title"
                                       :problem/summary "Second Problem Summary"}
                          :append [:component/id :main  :problems])
I am expecting to see listing of the problems on the website, but I get nothing.

tony.kay18:12:57

Did you compose Main into root, and Main should have initial-state of at least {}

tony.kay18:12:44

Your merge is fine (obviously)...it's the path from root that is causing you a problem. Things HAVE to compose in DATA AND QUERY to ROOT. Everything is relative to root...

tony.kay18:12:55

Unless you use floating roots or react hooks use-component

tony.kay18:12:18

The rendering composition, or course, will run on render and can partially succeed with nil props, thus you may see parts of your UI even thought they have no "data". This is fooling you into thinking you've composed Main in correctly...when in fact if you trace data data from root you'll find a "gap" in the linkage, because you did not provide initial state for Main, therefore it did not compose it into the data model.

tony.kay18:12:49

I'm pretty sure Fulcro is giving you warnings about components that don't have initial state in the console

tony.kay18:12:27

add :initial-state {} to Main and reload

Timofey Sitnikov18:12:30

> I'm pretty sure Fulcro is giving you warnings about components that don't have initial state in the console Absolutely right:

tony.kay18:12:08

does that fix it?

Timofey Sitnikov18:12:57

Here is what I get in console, but still no data:

tony.kay19:12:25

(merge/merge-component! SPA Problem {:problem/id    1
                                       :problem/title "First Problem Title"
                                       :problem/summary "First Problem Summary"}
                          :append [:component/id :main  :problems])

tony.kay19:12:29

does NOT match your query

1
tony.kay19:12:45

[:component/id :main :main/problems] is the path your code is expecting

tony.kay19:12:20

(for the :append)

Timofey Sitnikov19:12:40

OK, that worked!

Timofey Sitnikov19:12:10

WOW, thank you so much, will have to digest it.

tony.kay19:12:27

yeah, you have to be meticulous about your keywords 😄

tony.kay19:12:49

and almost anything that has a static ident should have initial state

tony.kay19:12:32

Most problems I diagnose where "it isn't rendering what I expect" boil down to: 1. Did you compose your query to root? 2. Did you compose your initial state from root down to the statically-available leaves of the UI? 3. Did you properly destructure the props you received at each node, and pass them to the children? That's literally the full checklist.

tony.kay19:12:36

It's like the joke that all IT problems boil down the the same diagnostic: "Did you turn it off and on again?"

Timofey Sitnikov19:12:25

Yeh, Fulcro is awesome, but there is a lot to know all at once to use it effectively. Thats the price you have to pay for its power.

tony.kay19:12:17

well, the point I'm trying to make is that rendering is very very very simple. it's those three things.

tony.kay19:12:56

which allow the system to, on startup create a data graph, then at render pull a tree of props that match your tree of UI. It's all the same composed tree...once represented in a data graph, and once represented (in mirror form) in a code graph

tony.kay19:12:45

app starts, make a data graph (initial state). App renders, pull the data graph, and pass it to the code, which pulls the bits apart as it walks the tree of DOM

tony.kay19:12:29

Mutations: Refine the tree...UI code can use conditional logic to reshape/respond

Timofey Sitnikov19:12:00

Understand, actually, what I think tripped me is problem/id is just a single keyword, I was thinking that Fulcro is parsing it into 2 branches [prblem id] thats is why I have been having a hard time dealing with the data tree.

tony.kay19:12:58

you mean this?

tony.kay19:12:17

using a single keyword is just a shorthand

tony.kay19:12:36

:k -> (fn [this props] [:k (:k props)])

Timofey Sitnikov19:12:17

Yes, looking back now, my assumption was silly.

Timofey Sitnikov19:12:53

Major progress today, this is awesome, I have been at this for few days. I only get to learn it about 1 hr per day.

Jakub Holý (HolyJak)16:12:00

Have your followed https://blog.jakubholy.net/2020/troubleshooting-fulcro/ when troubleshooting your problem? How far did you get/where did you get stuck? => what can I improve?

Timofey Sitnikov16:12:12

Actually, I did not, I forgot about it.

Jakub Holý (HolyJak)16:12:48

Frontend 3.a.i would lead you to trying db->tree uchu would show you it is a data problem not UI/rendering one. Though spotting the keyword error is not trivial I guess...

Timofey Sitnikov16:12:10

Yes, actually, the way the keywords work together is super easy once you get it. I was making a wrong assumption about keywords that is why I had so much trouble. This cleared it up.

Timofey Sitnikov16:12:45

I almost think that there could be a more visual way to tie it all together. I have been thinking of a way to create a diagram that would show how the keyword paths work, but have not been able to come up with something clear and easy to understand.

👍 1