Replace posh with ratom #784
Conversation
Original code alows executing a pull for `[:block/uid nil]` which is not a valid lookup ref (and throws an exception in DataScript). It was not a problem up to now, because Posh does allow this unspecified behavior (and just returns nil). Refactored the code to separate page-title and navigation concerns.
Solution was originally proposed by @lambduhh in athensresearch#665. See PR discussion for details why posh is considered slow and a mismatch for Athens UI performance right now. This version of the solution introduces a new namespace `athens.posh` as a form of indirection. This way the rest of the codebase does not need any further changes (aside from the namespace require) and also allows easy switching between the two different databases to compare performance and correctness (via `athens.posh/version`). The assumption is that later this hybrid duality can be elided and `athens.posh` refactored into a more robust data fetching layer.
Added athens.tracing which depends on tufte for profiling CLJS functions. It's not a great integration, as currently Athens is on an old version of re-frame-10x that does not have the appropriate monkeypatchin hooks. The assumption is that a developer will enable it by uncommenting the `athens.tracing` in shadow-cljs preloads and checking the results in console.log.
pithyless
force-pushed
the
replace-posh-with-ratom
branch
from
ffdaa4d
to
c2f3367
Compare
|
Results from testing open/close of a bullet on my personal DB. posh dbrx
Results from testing open/close of a bullet the ego db. Can test by adding Posh
dbrx
Time seems to be about the same or in some cases higher? |
|
@tangjeff0 any chance you did not restart the app after changing the https://www.youtube.com/watch?v=8lAth0JH3TQ Also, as I'm posting this now, I realize I made a mistake in how I interpret the results for the new dbrx But this should not change the observations about the performance of From my point of view, there are two open questions:
|
|
My results testing on my actual db. Demo'd here posh dbrx |
This is a continuation of the work done by @lambduhh on #665. Kudos to @lambduhh for the idea!
The supporting code has changed significantly, though, so I thought it would be easier to open a separate PR.
How to reproduce
The TL;DR to run this branch:
athens.tracinginshadow-cljs.edn:(-> :renderer :devtools :preloads)athens.posh/version(:poshis original flavor,:dbrxis brand new fizzy flavor)lein dev) and run Athens (yarn run electron .), open the DevTools Console, and click around.Two reports will show up.
Epoch Statsreport percentiles for profiled functions between epochs (as understood by re-frame-10x).Total Statsis a running total for the entire session. Epochs can help you identify the performance characteristics of a specific action, while Totals can help approximate the general performance of the app during regular usage.What are the results?
For my tests, I'm using a basic Welcome db + a large tagged markdown file.
:posh- original flavorOpen/Close sections of the Welcome page:
Open/Close sections of the Markdown report:
transact!. So, with additional parsing and rendering we have failed the Doherty Threshold!:dbrx- brand new fizzy flavorOpen/Closing sections of the Welcome page:
Open/Close sections of the Markdown report:
💯 Max 40 ms response times.
Next Steps
This should also benefit #570
Athens is still noticeably slow when working with large files, but this is the first of several problems that needed to be fixed; the next biggest bottleneck may be in parsing and/or rendering.
In the meantime, we need to test and make sure this approach doesn't break on some edge cases. More 👀 and user reports would be most welcome!
PR Code Review
The last two commits are intended for profiling and may be optionally merged, depending on preference. I split them out to make it easier to cherry-pick this branch.