GET /api/users/[userId] #39
sx property of
PickersDay doesn't update properly when controlled by state in
renderDay function mui/mui-x#6017
Last week I mentioned that I would be working on new infrastructure for implementing Necode's REST APIs. I did work on that, but some issues with how the new API would be typed led me to drop it for now. The work is on the
This week pretty much all of my development attention was on the classroom management page, and particularly on the lesson pane on the right side of it.
Previously, as part of an "optimization," changes made to a lesson would not immediately be sent to the server. Only after a certain amount of time of inactivity, leaving the page, or switching to a different lesson, would the local cached version be sent up to the server for storage in the database. While batching operations like this may not be a terrible idea in theory, there's no reason to do it unless an actual performance metric suggests it would be helpful, and the way I was doing it (sending over the entire lesson and letting the server reconcile the differences) caused a lot of problems. Particularly, data loss, data integrity loss, and client-server desyncs.
The new approach is much more atomic. A request is now made after every change,1 and the API now supports much more fine-grained tweaking of lesson/activity parameters. In conjunction with some new database schema-enforced data integrity constraints, building lessons now feels rock-solid.
With some of the changes, especially the change to reordering activities, things were starting to get really sluggish.
My suspicion was that this had to do with repeated calls to
getClientBoundingBox() in order to determine where to show the blue preview line.
getClientBoundingBox(), along with several other functions and properties, are notorious when it comes to performance because they trigger a reflow, something which can add a huge performance penalty when done frequently (e.g. on every hover event). However, as always, profiling is important.
What I would only catch onto much later after significant reading graphs and commenting out code and reading more graphs is that relatively little of the time spent is actually being spent on rendering. However, if reflow were the culprit, we would expect rendering to be dominating the graph.
Eventually, after seeing that most of the time was spent in React internals, I decided to install the React Developer Tools Chrome extension and take a look at what the specialized React profiler was saying.
I'll spare you the disappointingly large amount of time that I stared at this while still being convinced that reflow was the real issue, and just explain what this is showing.
(Functional) React components only re-render when:
setState function triggering
In the flame graph, you can see that the root of the re-rendering was caused at
PageContext from a hook update, presumably triggered by
react-dnd (the drag-and-drop library I'm using). That's fine and expected. What is not fine and expected is that a ton of stuff is being rendered that doesn't change at all. Here's an illustration of what should have been re-rendered and what shouldn't have been:
So then, the question is: why was so much stuff re-rendering if it didn't change at all?
Fortunately the React dev tools provide us with that information. Some of it re-rendered simply because it wasn't split out into separate components (i.e. the
children prop changed), and that was expected. Less expected was elements like
StaticDatePicker, which have no children and which should've had all the same properties, re-rendering.
The issue essentially boiled down to two things:
React provides hooks to resolve both of these issues--
useMemo--but the ergonomics of memoizing every single
sx prop (a common culprit when exploring the profile) would be horrible, and would probably cause some performance issues itself.2
However, functions and ordinary objects aren't the only things that can be memoized; component graphs can be memoized too. Along with extracting a component sub-graph to its own component (which sometimes works well but not always), this provides another way to make component graphs only re-render when an interesting property changes. After applying both of those techniques in a few places, dragging performance is no longer an issue:
There's still one lingering task for the lesson overhaul before I can close out the epic, which is the ability to move lessons to other dates. The API code for this is already written (assuming it works), but there is some frontend work to do. Hopefully that will get done in the next couple days.
After that I see a few possible areas to tackle:
And so on. I think option 2 is probably the best course of action for now, not because third-party activities are so important, but because lazy loading activities will require some major refactoring (which is better to get out of the way early) and should hopefully result in some significant performance improvements from cold start (and often require downloading less data). We'll see though, maybe something else will come up which takes priority.
Some changes, such as typing into a text field, are still cached locally and only sent out on blur. This is because unlike with creating and deleting and reordering activities, the cost of having a network round trip after every keystroke actually does have a meaningful performance impact. ↩
Many instances of the
sx props refer to (value-ly) constant objects, so those could be pulled outside of the component. This ergonomics of that suck too, but it's a bit less bad. ↩