Skip to content
New issue

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

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

Already on GitHub? # to your account

Performance #5

Open
roman01la opened this issue Mar 18, 2016 · 3 comments
Open

Performance #5

roman01la opened this issue Mar 18, 2016 · 3 comments

Comments

@roman01la
Copy link

Hi. Thank you very much for this library! I've been looking for something like this to work with Reagent.
I've noticed that animations are not quite smooth and after profiling in DevTools I can see that perf is not very good since FPS is lower than 60.

My question is: do you plan to work more on perf improvements? Is there any way I can help you?

From what I've seen I can say that most of the time per frame is spent in Reagent atoms. Seems like minimising usage of atoms would help. Also I'm not quite sure why are you using setTimeout in spring, shouldn't it be requestAnimationFrame?

Currently I'm using JavaScript library react-motion, the perf there is better but the API is not very comfortable to use with Reagent. So I'm really looking forward to this library. Thanks!

@timothypratley
Copy link
Owner

Hi Roman,

Yes, I want to make it faster, and thank you for diagnosing the time spent in ratoms. That is surprising to me because the ratoms should just be a mechanism to trigger re-rendering. Maybe using React local-state would be better for the interpolation part.

Pull requests are very welcome, and it would be great if we can collaborate on how to solve this.

I think the rough plan based on your findings is:

  1. Focus on the spring function as that is the most important one
  2. Add a new page to the devcards examples with lots of springs on it
  3. Try to figure out why time is spent in the ratom
  4. Do more profiling

I'll give this a shot sometime this weekend. I think in principle we should be able to achieve the same performances as react-motion... I should really look at their implementation, maybe we can copy some ideas from there. I agree the main objective I have is to make the API more comfortable in Reagent.

Regards,
Timothy

@bendyorke
Copy link
Collaborator

bendyorke commented Apr 25, 2016

Hello!

I tried to run the dev build, but I got a lot of errors. I ended up copying the file down and tried testing some changes. Here's what I came up with:

Test code:

(def left (r/atom 0))

(defn runner []
  (let [s (g/spring left)]
    (fn []
      [:div {:style {:position :absolute
                     :left @s}}
        "x"])))

(defn benchmark []
  [:div {:style {:position :relative}}
   (for [n (range 100)]
     ^{:key n} [runner])
   [:div {:style {:padding-top 50}}
    [:button {:on-click #(swap! left + 50)} "Go!"]]])

Procedure: Load page, start timeline, click "Go!", wait till they stop, then repeat 2 more times.

Results (Averages):

Time % scripting FPS Description
1.83s 87.0 48 Baseline (current)
1.52s 89.3 59 Upgrade to reagent 6
1.32s 87.2 62 Switch to js/setTimeout to r/next-tick
1.60s 89.2 58 Use track! instead of reaction
_increased to 500 elements_
3.10s 88.0 28 reaction @ 500
3.21s 88 22 track! @ 500
2.75s 86.2 32 reaction @ 500 + update 'small' function (below)

I noticed the small function was using js/Math.abs, which I don't believe is necessary. Moving to a comparison function increased the speed by over 10%. See the following benchmarks:

(time (dotimes [n 10000000] (< -0.1 -0.0000001 0.1))))
"Elapsed time: 8.000000 msecs"

(time (dotimes [n 10000000] (< (js/Math.abs -0.0000001) 0.1)))
"Elapsed time: 1190.000000 msecs"

I'd be happy to work on pull request for just these if you're open to accepting them. I also have quite a few other changes I'd like to make, I'm not sure how open you are to changes in the api :). If not, I can fork it for now and just try them out on my own!

EDIT: rewrote (and (< ...) (> ...)) as (< ... ...), because it's faster, clearer, and way more clojure-y

@timothypratley
Copy link
Owner

Hi Ben,

That's awesome. Yes please send a pull request and I'll merge it. Thank you!

Regards,
Timothy

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants