Improve performance canvas heatmap #574

aashish24 · 2016-05-11T15:26:17Z

No description provided.

The update delay can be specified.

…r than using jQUery's css function. This reduces a bit of jitter when panning a rotated heat map.

…y coordinates.

This will only change if the data changes or the projection changes, which will cause it to be recomputed then. Otherwise, this reduces update time by 10% of so.

Switching the core drawImage loop sped up large datasets by 40% or so.

Precalculating position must always be done, too.

aashish24 · 2016-05-11T15:28:33Z

Fixes #574 with many minor tweaks and one major tweak to get the max performance for canvas heatmap. Few other changes (binning, gaussian circle etc.) will be upcoming in separate PRs.

codecov-io · 2016-05-11T15:31:37Z

Current coverage is 72.78%

Merging #574 into master will increase coverage by +1.02%

@@             master       #574   diff @@
==========================================
  Files            82         82          
  Lines          7074       7128    +54   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           5076       5188   +112   
+ Misses         1998       1940    -58   
  Partials          0          0

2 files (not in diff) in src/d3 were modified. more
- Misses -30
- Hits +30
4 files (not in diff) in src were modified. more
- Misses -18
- Hits +18
File ...rc/heatmapFeature.js was modified. more
- Misses -9
- Partials 0
- Hits +9

Powered by Codecov. Last updated by aedf747...1e168f8

manthey · 2016-05-11T16:22:21Z

During the development of this code, I did some timing tests on my desktop:

Rendering time for the heatmap in Chrome:

# of points	Before	This PR
1,000	73 ms	27 ms
10,000	425 ms	68 ms
100,000	4373 ms	577 ms
1,000,000	44 s	5.7 s

Firefox is slower.

Of this, around 75 or 80% of the time is spent in drawImage.

This excludes the benefits of debouncing the rendering.

manthey · 2016-05-11T16:23:25Z

@aashish24 Do you have any machines that exercise the big-endian code?

aashish24 · 2016-05-11T16:55:42Z

@aashish24 Do you have any machines that exercise the big-endian code?

No I don't. The code piece I got inspiration from had done something but I don't know how thorough it was.

manthey · 2016-05-11T16:57:10Z

src/canvas/heatmapFeature.js

+    if (m_typedBuffer[4] === 0x0a &&
+        m_typedBuffer[5] === 0x0b &&
+        m_typedBuffer[6] === 0x0c &&
+        m_typedBuffer[7] === 0x0d) {


When I try this out in the Chrome console, it doesn't work. m_typedBuffer references an ArrayBuffer, which doesn't give me index reference access to the array. I think this comparison needs to be on the m_typedClampedBuffer, not on m_typedBuffer. What is here works, but only because we've only tried in on little-endian machines.

I see. let me have a look at it.

@manthey yes, it should be on clamped buffer, thanks for report this. I pushed a fix.

Earlier looking at the wrong buffer on which indexing was undefined

manthey · 2016-05-11T19:25:52Z

@aashish24 In thinking about this further, won't this mess up the gradient on a big-endian machine because it will switch the color channels? Specifically, in _colorize, the data from gradient is going to be in the same order as the data we want in the canvas putImageData, but we are reversing it.

The original (non-typed) behavior was only slower than the current code because it called m_this.style('opacity') for every pixel. I just tested this with that original code without multiplying the by opacity style, and the timing is statistically identical whether we use typed arrays or the simple copy that was done originally.

aashish24 · 2016-05-12T00:50:28Z

@aashish24 In thinking about this further, won't this mess up the gradient on a big-endian machine because it will switch the color channels? Specifically, in _colorize, the data from gradient is going to be in the same order as the data we want in the canvas putImageData, but we are reversing it.

I don't think so? Since gradient index look will give us the right value but we need to write that value that we get from gradient to the right offset.

Too bad that jsperf is down or else we could have tested it with some simple code (performance). The original author claimed that in the general it performed faster.

aashish24 · 2016-05-12T01:46:44Z

it was reported that firefox folks has seen quite a bit of performance improvement (but I cannot confirm now since jsperf is under renovation): https://jsperf.com/canvas-pixel-manipulation/6. I am wondering are there any big endian machine left? https://news.ycombinator.com/item?id=9451284

This is slightly faster because it operates on 32 bit values instead of 8 bit values. For 100,000 points in a controlled test, the average render time went from 517.7 ms to 499.5 ms (a 3.5% improvement). Additionally, there is only one code path, rather than two, which simplifies testing.

manthey · 2016-05-12T13:25:14Z

@aashish24 I made a simplification that is slightly faster and no longer needs separate code paths for different endians.

aashish24 · 2016-05-12T14:15:58Z

src/canvas/heatmapFeature.js

-      m_typedBuffer = new ArrayBuffer(imageData.data.length);
+  this._colorize = function (pixels, gradient) {
+    var grad = new Uint32Array(gradient.buffer);
+    var i, j, k, pixlen = pixels.length;


can we remove the other var (to be consistent). Also I was told that fewer vars declarations is better.

I'll make it so there is just one statement.

I'd be shocked if there is a measurable performance difference between single and multiple var statements.

The one var rule comes from jslint. I think the justification for it is preventing C programmers from misunderstanding how scoping rules work in javascript. My preference is just to keep all of the variable declarations at the top of the function, but I don't think enforcing a one var rule is worthwhile.

Roger that. I think most of our code is one var and declarations are on top of the function. As far as we are consistent, I am good.

Every layer has an opacity property. Unless there is a real need to have multiple features in the same layer with different per-feature opacities, I think we should stick with the current model.

aashish24 · 2016-05-12T14:32:03Z

@aashish24 I made a simplification that is slightly faster and no longer needs separate code paths for different endians.

less is better. I will have a look. How did you capture the performance?

manthey · 2016-05-12T14:36:42Z

I am using my modified example where I can load an arbitrary number of points. I loaded 100,000 points. I then used chrome's console to render the same frames in a predictable way: $.each([40,30,20,10,0], function (idx, angle) {window.setTimeout(function(){heatmap.layer().map().rotation(angle * Math.PI / 180);}, 5000 + 1000 * idx); }); where I've exposed the heatmap feature as a global variable. After starting that in the console, I used Chrome's Timeline function, and saw the time consumed for each frame (~500 ms per frame).

Note that Chrome is slower if the computer is busy doing something else, so make sure this is the only cpu-intensive process for consistent results.

Fix some of the documentation.

Fix an issue where a transform of a zero length array would throw an error.

manthey · 2016-05-12T18:48:47Z

@aashish24 This now has complete test coverage. At this point, I think you should do review and decide if it is ready for merge.

aashish24 · 2016-05-13T16:18:54Z

LGTM 👍

aashish24 and others added 9 commits May 11, 2016 11:26

Using typed array to increase performance

e9592b2

Using event data for transformation

19557f8

Fix intermediate transforms and update delays on the heatmap.

fadaa91

The update delay can be specified.

A minor speed-up can be obtained by directly setting transforms rathe…

037c106

…r than using jQUery's css function. This reduces a bit of jitter when panning a rotated heat map.

Rename origin to gcsOrigin to make it more obvious it isn't in displa…

d0cb87c

…y coordinates.

Reduce number of function calls for a minor speed improvement.

5e23638

Precompute gcs positions for heatmaps.

47e461a

This will only change if the data changes or the projection changes, which will cause it to be recomputed then. Otherwise, this reduces update time by 10% of so.

Loops can be faster than functions.

f2c6a68

Switching the core drawImage loop sped up large datasets by 40% or so.

Fix some issues setting max and min intensity.

f52faa5

Precalculating position must always be done, too.

aashish24 force-pushed the improve_performance_canvas_heatmap branch from ea28230 to f52faa5 Compare May 11, 2016 15:26

manthey reviewed May 11, 2016
View reviewed changes

Fixed Endian check

b4801bd

Earlier looking at the wrong buffer on which indexing was undefined

Do not use typed array on big-endian systems

8f25c0a

aashish24 force-pushed the improve_performance_canvas_heatmap branch from bd4e7cb to 8f25c0a Compare May 12, 2016 02:20

aashish24 reviewed May 12, 2016
View reviewed changes

manthey added 2 commits May 12, 2016 10:16

Remove opacity as a heatmap feature property.

a1db91b

Every layer has an opacity property. Unless there is a real need to have multiple features in the same layer with different per-feature opacities, I think we should stick with the current model.

Switch to a single var statement.

3b8fa85

Complete coverage of src/heatmapFeature.js.

ee6500e

Fix some of the documentation.

Add tests for _animatePan.

62b0e77

Fix an issue where a transform of a zero length array would throw an error.

Merge branch 'master' into improve_performance_canvas_heatmap

10da56c

aashish24 merged commit b3e15f8 into master May 13, 2016

aashish24 deleted the improve_performance_canvas_heatmap branch May 13, 2016 16:19

manthey mentioned this pull request May 18, 2016

Performance improvements to canvas heatmap implementation #559

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve performance canvas heatmap #574

Improve performance canvas heatmap #574

aashish24 commented May 11, 2016

aashish24 commented May 11, 2016

codecov-io commented May 11, 2016 •

edited

Loading

manthey commented May 11, 2016

manthey commented May 11, 2016

aashish24 commented May 11, 2016 •

edited

Loading

manthey May 11, 2016

aashish24 May 11, 2016

aashish24 May 11, 2016

manthey commented May 11, 2016

aashish24 commented May 12, 2016

aashish24 commented May 12, 2016 •

edited

Loading

manthey commented May 12, 2016

aashish24 May 12, 2016

manthey May 12, 2016

jbeezley May 12, 2016

aashish24 May 12, 2016

aashish24 commented May 12, 2016

manthey commented May 12, 2016

manthey commented May 12, 2016

aashish24 commented May 13, 2016

Improve performance canvas heatmap #574

Improve performance canvas heatmap #574

Conversation

aashish24 commented May 11, 2016

aashish24 commented May 11, 2016

codecov-io commented May 11, 2016 • edited Loading

Current coverage is 72.78%

manthey commented May 11, 2016

manthey commented May 11, 2016

aashish24 commented May 11, 2016 • edited Loading

manthey May 11, 2016

Choose a reason for hiding this comment

aashish24 May 11, 2016

Choose a reason for hiding this comment

aashish24 May 11, 2016

Choose a reason for hiding this comment

manthey commented May 11, 2016

aashish24 commented May 12, 2016

aashish24 commented May 12, 2016 • edited Loading

manthey commented May 12, 2016

aashish24 May 12, 2016

Choose a reason for hiding this comment

manthey May 12, 2016

Choose a reason for hiding this comment

jbeezley May 12, 2016

Choose a reason for hiding this comment

aashish24 May 12, 2016

Choose a reason for hiding this comment

aashish24 commented May 12, 2016

manthey commented May 12, 2016

manthey commented May 12, 2016

aashish24 commented May 13, 2016

codecov-io commented May 11, 2016 •

edited

Loading

aashish24 commented May 11, 2016 •

edited

Loading

aashish24 commented May 12, 2016 •

edited

Loading