maputnik icon indicating copy to clipboard operation
maputnik copied to clipboard

Perf improvements with icepick-style

Open orangemug opened this issue 6 years ago • 4 comments
trafficstars

I've been busy over the last week trying to improve the Maputnik UI performance. Here's where I've got to so far

I've created a new immutable style store for Maputnik which uses icepick called icepick-style

My simple initial performance test is changing a single color in the UI. In particular, the background layer of the osm-liberty. You can read some further details over in https://github.com/maputnik/editor/issues/372#issuecomment-428677612

Here are the results so far in the development build of our app.

Before Screen Shot 2019-05-19 at 18 29 44

After Screen Shot 2019-05-19 at 18 28 28

229.48ms down to 76.04ms for 'react tree reconciliation' on a single run, but results seem consistent.

The app seems to mostly work as it did previously. There are a couple of bugs around updating sprites and sources, which I'll try to fix the next time I work on Maputnik.

It's still pretty slow but a big step in the right direction.

orangemug avatar May 19 '19 17:05 orangemug

@orangemug this looks promising, but I wonder why before it was 229.48ms because you mentioned in https://github.com/maputnik/editor/issues/372#issuecomment-428677612:

On my fairly fast laptop changing the background layer's color property for the osm-liberty takes about 84ms. This is in chrome which is a fair bit faster than Firefox at rendering our UI.

What has changed since then? Different machine? browser? or did you measure there something different than 'react tree reconciliation'?

pathmapper avatar May 19 '19 18:05 pathmapper

What has changed since then? Different machine? browser? or did you measure there something different than 'react tree reconciliation'?

So the two measurements are the same test in different versions of the code with the same browser/laptop

  • 229.48ms was tested in v1.5.0 of Maputnik without the immutable additions
  • 76.04ms is with my latest immutable style changes applied

It's a pretty simple test at the moment, but essentially the use of immutable data structure and PureComponent means that react doesn't bother calling the render methods for a bunch of the component tree. Which in turn means that there is a lot less work for the 'react tree reconciliation' to do.

Also worth noting that this is just the first step, we should be able to improve this significantly more. Saying that my first priority is getting these current improvements stable and well tested and merged into master

orangemug avatar May 20 '19 10:05 orangemug

Thanks, I assumed you were using v1.5.0 for https://github.com/maputnik/editor/issues/372#issuecomment-428677612 and achieved 84ms without the immutable additions.

pathmapper avatar May 20 '19 11:05 pathmapper

Thanks, I assumed you were using v1.5.0 for #372 (comment) and achieved 84ms without the immutable additions.

Nice spot @pathmapper I'll checkout the earlier versions of Maputnik see if the perf has dropped. It's possible I just measured it incorrectly in https://github.com/maputnik/editor/issues/372#issuecomment-428677612 but I'll do some hunting about and see what I can find out.

orangemug avatar May 20 '19 11:05 orangemug