day8 / re-frame-10x

A debugging dashboard for re-frame. X-ray vision as tooling.
MIT License
632 stars 68 forks source link

[Bug]: Usage of `memoize` creates a memory leak that can crash the web page #322

Closed p-himik closed 2 years ago

p-himik commented 2 years ago

What happened?

Using anything that creates a ton of components will keep on consuming memory till the web page is killed. Well, at least that's my understanding of what's going on. In my particular case, I'm using react-virtuoso which creates a virtualized list.

According to pre-mortem memory profile in Google Chrome, this is the line that retains the most memory: https://github.com/day8/re-frame-10x/blob/master/src/day8/reagent/impl/component.cljs#L13 (was introduced in 89f342e8350f97a4b5e0a2d7442a8ae617081213)

The implementation itself suffers from using the linear last, str/split which is much slower than js/String.prototype.splt, and incurred memoize cache lookup overhead.

(def s "some > very > not > so > short > component > path")

(def bad-memoized-split (fn [s]
                          (last (str/split s #" > "))))

(defn good-regular-split [s]
  (let [a (.split s " > ")]
    (aget a (dec (alength a)))))

(js/console.log (bad-memoized-split s) (good-regular-split s))

(js/console.time "memoized split")
(dotimes [_ 10000]
  (bad-memoized-split s))
(js/console.timeEnd "memoized split")

(js/console.time "regular split")
(dotimes [_ 10000]
  (good-regular-split s))
(js/console.timeEnd "regular split")

Result:

path path
memoized split: 85.535888671875 ms
regular split: 3.27685546875 ms

But the thing is, that path splitting function is now useless. Back in 0.10.0, Reagent removed component-path completely and left us only with component-name - which doesn't use > at all. At least, not as a path separator (I imagine one could still use > as part of a component's name).

I suggest removing operation-name and instead using component/component-name.

10x Version

1.1.13

Reagent Version

1.1.0

React Version

17.0.2

re-frame Version

1.2.0

What browsers are you seeing the problem on?

Chrome

Relevant console output

No response