jonhoo / inferno

A Rust port of FlameGraph
Other
1.65k stars 118 forks source link

Slow rendering of flamegraphs #256

Open itamarst opened 2 years ago

itamarst commented 2 years ago

A user complained about slow rendering of flamegraphs, and showed me a video; it was quite slow. Loading the same data into speedscope (https://www.speedscope.app/) was not slow, so the problem was with the way Inferno rendered it.

I tried doing a little performance profiling in Chrome; for context, I'm using 12th-gen i7 desktop, so it's probably got almost double the single-core speed of many (most?) computers.

This suggests that coming up with a faster alternative to update_text() would improve rendering time for everyone, and hopefully fix rendering problems for people whose computers are extra slow.

Chrome's profiler suggested as a hint that maybe the problem had something to do with https://web.dev/avoid-large-complex-layouts-and-layout-thrashing/

itamarst commented 2 years ago

Reading the implementation it also seems not ideal, e.g. shrinking the string character by character until it's OK.

itamarst commented 2 years ago

If it is layout thrashing, fastdom is proposed by the linked article and does seem like it would allow fixing things without a massive refactoring.

itamarst commented 2 years ago

From an example (https://github.com/wilsonpage/fastdom/blob/master/examples/aspect-ratio.html):

    function reset(done) {
      n = input.value;
      divs = [];

      fastdom.measure(function() {
        var winWidth = window.innerWidth;

        fastdom.mutate(function() {
          container.innerHTML = '';

          for (var i = 0; i < n; i++) {
            var div = document.createElement('div');
            div.style.width = Math.round(Math.random() * winWidth) + 'px';
            container.appendChild(div);
            divs.push(div);
          }

          if (done) done();
        });
      });
    }
itamarst commented 2 years ago

Coincidentally I just found an example input file that triggers visible slowness on my computer with only 2000 lines of input. memory.zip

itamarst commented 2 years ago

Increasing flamegraph::Options::min_width helps a lot for the example above. So my guess: long text + tall stacks + extremely thin columns means a whole lot of looping to shrink the text until it fits.

Perhaps then one improvement would be increasing text width until it is too big, instead of shrinking it until it fits? By their nature you'd expect many more thin columns than large columns, so that should reduce useless spinning. And/or maybe use parent column's text width as starting point, since child by its nature is going to be narrower?

jonhoo commented 2 years ago

Alternatively it sounds like maybe we could do a binary search — that should speed it up pretty significantly.

itamarst commented 2 years ago

Oh yeah computer science.

itamarst commented 2 years ago

Another thought: divide frame width by font size to get approximate starting point, then do linear search up or down depending on how that fits.

SergejIsbrecht commented 2 years ago

Maybe switch from svg to d3 (https://d3js.org/) just like async-profiler did (https://github.com/jvm-profiling-tools/async-profiler)?

Following html FlameGraphs were created with async-profiler: https://gist.github.com/SergejIsbrecht/dda8abca7c3c1004f03f1507bc1e9240 . Just open with any browser.

itamarst commented 1 year ago

Also occurs to me that for monospace fonts the value could be calculated directly, instead of empirically, so I might as a first pass do a fast path for that case (which would suffice for my use case).

mrob95 commented 1 year ago

I've also noticed slow rendering with large graphs. Thanks for looking into it @itamarst.

I haven't done any detailed profiling of it but another potentially unnecessary thing that update_text is doing is running a regex every time to snip the sample count from the end of the title to get the function name. The function name (or just its length) could be stored somewhere to avoid this, e.g.

https://github.com/mrob95/inferno/commit/7fa333d5a698000a4631aebff08758fdbdd81d7c

jonhoo commented 1 year ago

@mrob95 You'll want to keep an eye on https://github.com/jonhoo/inferno/pull/262 — looks like that will bring some pretty major improvements!