freesound-explorer icon indicating copy to clipboard operation
freesound-explorer copied to clipboard

Improve performance

Open giuband opened this issue 9 years ago • 13 comments

Much can be done to improve overall performance, some ideas:

  • [ ] When moving the map, compute the position for only the points who will be visible
  • [ ] Computation of points coordinates through tsne should happen in a separate worker
  • [x] Some Paths components subscribe to state.sounds.byID, which means they will frequently re-render
  • [ ] Use React.Perf to identify other causes of unneeded renderings
  • [ ] Space thumbnails probably re-render while moving the map (when tab is shown)
  • [ ] Use react-virtualized on heavy tabs (like spaces one)
  • [x] Refactor CSS code for sidebar

giuband avatar Sep 11 '16 08:09 giuband

@ffont: >

I'm not sure how this could be helped, but the very first implementation of FSE used raw HTML canvas and it was blazing fast. Maybe we could try with a library build on top of HTML canvas (for example, I tried Fabric.js at some point and it seems to work fine) to draw the circles and see what happens. Maybe using one of these libraries the code changes necessary to experiment with that would not be that big. In any case this is just an idea for future.

-- This was copied from #53

I measured a lot during the last days. The most performance hungy functions where:

  • perform (React has a lot to re-render, maybe this could be optimized, I just failed until here)
  • computeTsneSolution - maybe try a webassembly solution? (only when building a new space)
  • paint - I read tricks about putting the svg inside a container and moving this instead of the

I tried to try HTML-GL but failed here, too. It also seemed to not feature retina displays... I considered tackling this when all critical features for my thesis are implemented.

Question - why don't we use the canvas from the beginning, if it is so fast?

noVaSon avatar May 03 '18 14:05 noVaSon

I managed to reduce load in the soundlist PR #46, as the whole table was re-rendering with each map move. Just used shouldComponentUpdate and a deep comparision.

But for the spaces tab I didn't figure it out yet. Tried to set shoudComponentUpdate to false everywhere, but it still updated...

noVaSon avatar May 03 '18 14:05 noVaSon

The computeTsneSolution part I don't think it needs to be optimized for now as this is only done at "load results" time. However once the map is computed, navigating and moving it is quite slow if there are many sounds. I think the real deal here is the drawing part (although I have no data to support that).

We moved away from canvas because the interaction with canvas elements was rather limited (even though we never tried a library on top of HTML canvas). Also SVG seemed better because vector graphics bla bla bla.

@giuband do you have any ideas what could we do here? Would it make sense to try some canvas-based visualization for the map? What would be the best approach to try something like that? (if it makes any sense at all).

ffont avatar May 03 '18 14:05 ffont

Migrating back to canvas would give the greatest positive impact on performance, as it's much more performant than SVGs when there are thousands of elements. But it would take quite a lot of time and probably some of the things will have to be hacked in order to work as they do now (like hovering or clicking on elements).

Other things that can be tried without being that complicated and time demanding:

  • using CSS transform: translate3d() and transform: scale3d() on the entire SVG element instead of the single circles when moving around the map or zooming on it. It could get a bit tricky but I'd give it a try.
  • getting a bit creative with optimization. For instance, while the user is moving the map only a subset of points could be shown. Or even some clustering technique could be tried out. The ultimate goal in both cases would be of drastically reducing the elements being rendered in the document.

Honestly, I'd try with all of them and see how feasible they are. The third option is the easiest one and I'd start from that just to have some benefit in the short term.

giuband avatar May 03 '18 14:05 giuband

A search in Chrome with 300 resulting sounds results in a max 185 ms frame movement (5.5 fps!) when dragging the map.

A short measure shows: roughly 7.5% of calucation time a frame is caused by Recaluclate Style (in CSSPropertyOperations) 12.8% is caused by the sound byID reducer.js L84 (UPDATE_MAP_POSITION) 82% is caused by React/Redux state updates (connect)

I think we'd need to improve the whole thing about state changes. Styles and painting will only save 20 ms per frame (which is definetely noticeable...)

screenshot 2018-05-03 16 57 34 screenshot 2018-05-03 16 57 15

noVaSon avatar May 03 '18 14:05 noVaSon

I like your ideas @giuband. To start we should try "randomly" hiding some objects when moving the map to see how performance is affected. To do it less randomly we could hide the objects which are far from the mouse pointer.

Another idea, we could replace the whole map for a rasterized version of it when moving. I guess this is lot of work but I think this is how PDF viewers do it for example.

ffont avatar May 03 '18 15:05 ffont

Challenging isn't it? :)

noVaSon avatar May 03 '18 15:05 noVaSon

That’s the spirit!

ffont avatar May 03 '18 15:05 ffont

My first and quickest idea: use a debounce in the MapCircleContainers to avoid the unnecessary plethora of re-renders caused by the events. Something like this should work: https://www.npmjs.com/package/react-debounce-render

noVaSon avatar May 03 '18 20:05 noVaSon

Sounds like a good idea, although I guess the problem is that circles receive too many updates of x, y positions and using debouncing might stop drawing them until the end, but it will be an interesting idea. Another idea, could we “sample” the solution of the tsne at a lower sampling rate (eg, 250ms) and animate the circles from one position to another (interpolate)?

ffont avatar May 04 '18 06:05 ffont

If it is even possible with the React Library, it could dramatically improve animation performance to use the object pool design pattern for the Circle Objects. Currently we are re-instanciating each circle each frame of movement. Don’t know how to achieve this yet. Maybe with the use of the react lifecycle methods.

noVaSon avatar Feb 06 '20 16:02 noVaSon

Hey, welcome back! :) The project has not evolved since your latest changes @noVaSon, but it would be awesome to improve performance :) Any comments @giuband ??

ffont avatar Feb 07 '20 10:02 ffont

Can't check this out properly at the moment, but keep in mind that using the key prop properly is the way to let react create an unambiguous one-to-one bound between component and dom element, so that react won't create again a new dom node every time the component updates. I think we are already doing this correctly in this repo by giving each circle the id of its sound as circle, aren't we? In that case react is not remounting the circles everytime.

giuband avatar Feb 10 '20 09:02 giuband