Wednesday, 11 January 2017

React-ing to the need for a modern MapGuide viewer (Part 9): It's like a Christmas tree!

After figuring out how to make an accordion and getting the 0.7 release out the door, I needed to do some necessary refactoring of how we're currently modelling application state in redux to make my intended marquee feature of the next release possible.

After the refactoring, I saw some noticeable sluggishness in the viewer across all templates. This is where my choice to build this viewer on top of React has been validated (once again for the umpteenth time!), as it comes with top-notch developer tool support. So I flicked over to the React Developer Tools, activated update tracing and lo and behold ...

The viewer lit up like a Christmas tree, without any user interaction! What this was showing was that some React component was constantly updating and re-rendering itself and the sluggishness I was experiencing was in part due to that.

So it was clear that the refactoring caused some component to constantly be updating. But what?

That's where I turned to a new feature in the latest 15.4 release of React, which is by simply appending react_perf to the query string

It activates performance profiling in React itself. I then switched to the Timeline tab in the Chrome devtools and recorded 5 seconds of this "idle" activity

And we see there is activity that corresponds to this constant updating/re-rendering. If we drill down to one of these hot spots.

We finally identify the culprit:

  • It is the map viewer component
  • Something is causing the busy count to constantly go up and down. We store busy count as redux state so that the Navigator (aka. Zoom slider) component can listen on to determine whether to show/hide the "busy" animating indicator at the top of the component
The incrementBusyWorker and decrementBusyWorker functions are registered handlers for the imageloadstart and imageloadend events provided by OpenLayers MapGuide image sources. The only way these events would constantly fire from OpenLayers would be if something was constantly refreshing the map.

Which meant the possible suspect location is most likely in the component's componentWillReceiveProps lifecycle hook function which we use to interact with OpenLayers in response to component prop changes.

I stuck a breakpoint here to see if it was being hit while "idle", and it did!

While paused in this breakpoint, I evaluated the above two expressions to see what was being compared

And there was the problem! My visibility difference check was insufficient as it only did a shallow object reference comparison. The possible cause for this behaviour? My refactoring and over-zealous use of the new object spread operator in TypeScript 2.1 for shallow state cloning in my various redux reducers exposed and brought this problem into the light.

The fix was two parts. Firstly, to replace these (obviously incorrect) checks with helper functions that not only check the references (a null/undefined to set reference transition or vice versa is still a legit difference), but the actual object properties themselves.

Secondly, the layerGroupVisibility prop was a nested object prop that was being created in our component's connect function. This is actually a bad thing, which upon further reading makes perfect sense as returning new objects made in connect() will break the shallow object comparison that redux will do against a previous connect-invocation to short-circuit unnecessary component re-rendering. The fix here is to flatten this particular prop by replacing the layerGroupVisibility prop with the 4 individual array props and update all prop usages/references in the component code accordingly.

With this change in place, turning on React update tracing again shows that it no longer flashes around like a Christmas tree.

So what were the lessons learned here?
  • Component props and state, and how you structure and compare them are important when it comes to performance. The endless articles out there on the art of properly implementing shouldComponentUpdate underscores how important this fact is.
  • On the same vein, Redux has its own set of best practices for performance. This is good reference.
  • React's developer tooling is top-notch, and allows for performance issues like this one to be easily identified and debuggable.
Now to make sure this major refactoring didn't regress any other stuff.

No comments: