Make preview selections faster by only dispatching on changes#1481
Conversation
| /** | ||
| * Perform a simple shallow object equality check. | ||
| */ | ||
| export function objectShallowEquals(a: Object, b: Object): boolean { |
There was a problem hiding this comment.
I felt that it was simpler to write this function than to include an additional dependency the project. I think we previously had some lodash functions, but now I'm not seeing any, so I didn't want to add additional bytes to the bundle for something so small.
Codecov Report
@@ Coverage Diff @@
## master #1481 +/- ##
==========================================
+ Coverage 80.79% 80.81% +0.02%
==========================================
Files 157 157
Lines 10792 10807 +15
Branches 2627 2631 +4
==========================================
+ Hits 8719 8734 +15
Misses 1874 1874
Partials 199 199
Continue to review full report at Codecov.
|
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
Looks good to me. But since we are now doing more calculations for every preview selection change, I'm not sure if that really makes it faster. Do you have an example of mentioned performance problem?
I also have an optional suggestion for the objectShallowEquals but not super sure about it. It's your call :)
| /** | ||
| * Perform a simple shallow object equality check. | ||
| */ | ||
| export function objectShallowEquals(a: Object, b: Object): boolean { |
I didn't keep the profiles I was recording, but it was with an earlier version of the JS tracer chart. It would make my browser barely function when I was zooming out, but the viewport wasn't actually changing. As for the extra calculations, these objects have only a few properties, so in practice the equality check should be almost free. Any dispatch to the Redux store has a non-trivial cost associated with it. Unfortunately, objects of the same shape, still trigger re-renders, since they fail the equality test. So this should reduce the number of dispatches that we perform overall, with little cost to the normal functioning. |
gregtatum
force-pushed
the
limit-preview-selection-dispatches
branch
from
c4c6b79 to
1c73bb2
Compare
2 participants
I was running into performance issues on the JS Tracer chart when dispatching many equivalent preview selections. This happens especially when scrolling out to the maximum zoom level. I did not include a test specifically for this behavior change, but we have existing coverage over the action, so to me it didn't seem worth it. I will double check the coverage information once it comes back from CI.