You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2020/09/20 04:39:06 UTC

[GitHub] [incubator-echarts] pissang opened a new pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

pissang opened a new pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314


   This pull request include two improvements to line series.
   
   1. Reduce initial render time(and memory cost) up to 10x in the line series with millions of time series data.
   2. Improve smooth result in some cases. 
   
   ### Details about performance improvement.
   
   ##### 1. Use TypedArray.
   
   The most important thing did to improve the performance is to use TypedArray instead of normal array as much as possible. It will reduce the heap memory and GC cost significantly.
   
   In this PR we use TypedArray to store the points of polyline and polygons. Also no `setItemLayout` anymore.
   
   ##### 2. Bake the transform of dataToPoint to matrix in Cartesian2D
   
   It will simplify the transform to a very fast 6x2 multiply and add operations. But it only works for affine transform when axis type is `time` or `value`. So it's mainly for boosting the time series data.
   
   ##### 3. Remove unnecessary date parsing when it's a timestamp.
   
   It can avoid creating millions of temporary `Date` object when parsing the time series data.
   
   ##### 5. Speed up storage access in List component.
   
   It's a trick did for modern JS engine. I found the access of `storage` is slow when initializing millions of data in https://github.com/apache/incubator-echarts/blob/99ab1d7a2f1413ce38dd55d3808c4e434fe49ba8/src/data/List.ts#L628
   I profiled it and saw `KeyedLoadIC_Megamorphic` in the profiling result, which means it's not a fast property access.
   
   So we simplify the case to the following code.
   ```js
   const obj = {};
   obj.x = 1;
   obj.y = 1;
   function slowObjectAccess() {
       let sum = 0;
       for (let i = 0; i < 1e6; i++) {
           for (let k = 0; k < 2; k++) {
               const val = obj[k === 0 ? 'x' : 'y'];
               sum += val;
           }
       }
       return sum;
   }
   
   function fastObjectAccess() {
       let sum = 0;
       for (let i = 0; i < 1e6; i++) {
           for (let k = 0; k < 2; k++) {
               sum += k === 0 ? obj.x : obj.y;
           }
       }
       return sum;
   }
   ```
   
   And the test result shows `fastObjectAccess`can be 10x faster than `slowObjectAccess`. I can't find out why and how to avoid it because of limited knowledge of JS engine.  So I add an extra `_storageArr` array and get from this array in the hotspot code like `List#_initDataFromProvider`. It works well and the cost of `_initDataFromProvider ` reduced to half.
   
   ### Details about smooth improvement.
   
   ### Result
   
   #### Render time comparison between 4.x and 5.0
   
   <img width="976" alt="infoflow 2020-09-20 12-01-40" src="https://user-images.githubusercontent.com/841551/93694107-73efed00-fb3a-11ea-97aa-6ed35c5bc6d9.png">
   
   #### Heap memory comparison between 4.x and 5.0
   
   <img width="998" alt="infoflow 2020-09-20 12-01-02" src="https://user-images.githubusercontent.com/841551/93694118-8ff38e80-fb3a-11ea-9733-bfa21f074f5c.png">
   
   #### Smooth algorithm comparision
   
   ##### Before
   
   <img width="1211" alt="infoflow 2020-09-18 22-43-12" src="https://user-images.githubusercontent.com/841551/93694129-bb767900-fb3a-11ea-8978-4c899f2759a9.png">
   
   ##### After
   <img width="1198" alt="infoflow 2020-09-18 22-42-06" src="https://user-images.githubusercontent.com/841551/93694130-c0d3c380-fb3a-11ea-94d6-2fe54d65bff1.png">
   
   
   
   
   ### Fixed issues
   
   https://github.com/apache/incubator-echarts/issues/12249
   https://github.com/apache/incubator-echarts/issues/10200
   https://github.com/apache/incubator-echarts/issues/4556
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695758284

@leeoniya Thanks for the benchmark. I'm really impressed by the performance of uPlot!

I add an optimized version that takes a Float64Array as input data. It's a bit more complex to write but has better performance and takes less heap memory.

Optimized version:
https://echarts-bench-from-uplot.glitch.me/index-optimized.html

Original verison:
https://echarts-bench-from-uplot.glitch.me/

Tested it on my 15inch mac. The initialization time is about 170ms. And the heap memory is about 10~12MB(peak), 5MB(final).

About the smoothing algorithm. I have tried the Catmull-Rom Spline before. But it has similar issues which may look quite weird

I'm not sure if you did any modification on the original Catmull-Rom Spline algorithm.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

echarts-bot[bot] commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695880093


   Congratulations! Your PR has been merged. Thanks for your contribution! 👍


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695758284

@leeoniya Thanks for the benchmark. I'm really impressed by the performance of uPlot

I add an optimized version that takes a Float64Array as data input. It's a bit more complex to write but has better performance and takes less heap memory.

Optimized version:
https://echarts-bench-from-uplot.glitch.me/index-optimized.html

Original verison:
https://echarts-bench-from-uplot.glitch.me/

Tested it on my 15inch mac. The initialization time is about 170ms. And the heap memory is about 10~12MB(peak), 5MB(final).

About the smoothing algorithm. I have tried the Catmull-Rom Spline before. But it has similar issues which looks quite weird

I'm not sure if you did any modification on the spline.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695758284


   @leeoniya Thanks for the benchmark. I'm really impressed by the performance of uPlot 
   
   I add an optimized version that takes a Float64Array as data input. It's a bit more complex to write but has better performance and takes less heap memory.
   
   https://echarts-bench-from-uplot.glitch.me/index-optimized.html
   
   Tested it on my 15inch mac. The initialization time is about 170ms. And the heap memory is about 10~12MB(peak), 5MB(final).
   
   About the smoothing algorithm. I have tried the Catmull-Rom Spline before. But it has similar issues which looks quite weird 
   
   <img width="1041" alt="infoflow 2020-09-20 15-59-58" src="https://user-images.githubusercontent.com/841551/93706604-69455000-fb5a-11ea-8a0b-0d32d290d2ef.png">
   
   I'm not sure if you did any modification on the spline.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695777118


   PS: I did both the performance improvement and smoothing algorithm improvement when refactoring the polyline builder https://github.com/apache/incubator-echarts/blob/76bf8f4e8472187a2668c6282492e5b1c63f57db/src/chart/line/poly.ts. So I didn't try to separate it into two PRs. Sorry about that if it brings any inconvenience.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-698183829


   Hi @leeoniya 
   
   I did a bit more optimization in these two days. Now the time has been reduced to about 140ms. With LTTB downsample enabled, it can be less than 100ms on my 15inch macbook.
   
   https://echarts-bench-from-uplot.glitch.me/index-lttb.html
   
   I also add two test cases with 10x larger data.
   
   https://echarts-bench-from-uplot.glitch.me/index-large.html
   https://echarts-bench-from-uplot.glitch.me/index-lttb-large.html
   
   Just want to thank you and your uplot that motivate me to push the performance of echarts to the limit.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-698183829






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] leeoniya commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

leeoniya commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695745867


   wow, this is super exciting. can't wait to test this improvement on the uPlot benchmark!
   
   https://github.com/leeoniya/uPlot#performance
   
   a 10x improvement would move echarts to second place.
   
   would you mind reviewing my implementation to make sure i'm using the library correctly, and also let me know when this will be available to test in a linkable build?
   
   https://github.com/leeoniya/uPlot/blob/master/bench/ECharts.html
   
   cheers!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695758284

@leeoniya Thanks for the benchmark. I'm really impressed by the performance of uPlot!

I add an optimized version that takes a Float64Array as input data. It's a bit more complex to write but has better performance and takes less heap memory.

Optimized version:
https://echarts-bench-from-uplot.glitch.me/index-optimized.html

Original verison:
https://echarts-bench-from-uplot.glitch.me/

Tested it on my 15inch mac. The initialization time is about 170ms. And the heap memory is about 10~12MB(peak), 5MB(final).

About the smoothing algorithm. I have tried the Catmull-Rom Spline before. But it has similar issues which looks quite weird

I'm not sure if you did any modification on the Catmull-Rom Spline algorithm.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695879944

https://echarts-bench-from-uplot.glitch.me/uplot-smoothing.html

I tried it and seems a bit different from my Catmull-Rom spline implementation. Haven't dig into it and not sure if it's because I draw the spline with interpolated points and yours are bezier curves.

But it looks similar to our original bezier curve implementation without limiting to min, max extremes. The problem is the curve will exceed the extremes. For example under the 0 baseline or exceed the max value(if the max value has some meaning like 100 percent.

![image](https://user-images.githubusercontent.com/841551/93729676-1cb05200-fbf8-11ea-869b-f46e90dda42a.png)

If we limiting the curve between global min, max extremes.
https://echarts-bench-from-uplot.glitch.me/uplot-smoothing-limit-extrema.html

It becomes almost the same as the result before change in this PR.

![image](https://user-images.githubusercontent.com/841551/93729893-e9ba8e00-fbf8-11ea-8723-4e41fd0dab09.png)

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695762009


   Did some modifications to make these two versions look more reasonable :)
   
   <img width="1928" alt="infoflow 2020-09-20 16-47-34" src="https://user-images.githubusercontent.com/841551/93707425-099e7300-fb61-11ea-89ae-d925d65c302e.png">
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695777118






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang merged pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang merged pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] leeoniya edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

leeoniya edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695785582


   > I'm not sure if you did any modification on the original Catmull-Rom Spline algorithm.
   
   i simply adjusted this implementation for the uPlot demo from this gist: https://github.com/leeoniya/uPlot/blob/master/demos/line-smoothing.html#L49
   
   i don't think the version i'm using would behave as your posted image given the very few and sparse datapoints which are visible. can you try the same dataset in the uPlot demo and see how that looks? (sorry i'm on holiday and on phone, so cannot test it for a while).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] leeoniya edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

leeoniya edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695785582


   > I'm not sure if you did any modification on the original Catmull-Rom Spline algorithm.
   
   i simply adjusted this implementation for the uPlot demo from this gist: https://github.com/leeoniya/uPlot/blob/master/demos/line-smoothing.html#L49
   
   i don't think the version i'm using would behave as your posted image given the very few and sparse datapoints which are visible. can you try your dataset in the uPlot demo and see how that looks? (sorry i'm on holiday and on phone, so cannot test it for a while).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-698183829


   Hi @leeoniya 
   
   I did a bit more optimization in these two days. Now the time has been reduced to about 140ms. With LTTB downsample enabled, it can be less than 100ms on my 15inch MacBook and the display result is almost the same.
   
   https://echarts-bench-from-uplot.glitch.me/index-lttb.html
   
   I also add two test cases with 10x larger data.
   
   https://echarts-bench-from-uplot.glitch.me/index-large.html
   https://echarts-bench-from-uplot.glitch.me/index-lttb-large.html
   
   Just want to thank you and your uplot that motivate me to push the performance of echarts to the limit.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695879944

https://echarts-bench-from-uplot.glitch.me/uplot-smoothing.html

But it looks similar to our original bezier curve implementation without limiting min, max value. The problem is the curve will exceed the extremes. For example under the 0 baseline or exceed the max value(if the max value has some meaning like 100 percent.

![image](https://user-images.githubusercontent.com/841551/93729676-1cb05200-fbf8-11ea-869b-f46e90dda42a.png)

If we limiting the curve between global min, max extremes.
https://echarts-bench-from-uplot.glitch.me/uplot-smoothing-limit-extrema.html

It's becomes almost the same as the result in this PR before optimization.

![image](https://user-images.githubusercontent.com/841551/93729893-e9ba8e00-fbf8-11ea-8723-4e41fd0dab09.png)

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695879944

https://echarts-bench-from-uplot.glitch.me/uplot-smoothing.html

I tried it and seems a bit different with my Catmull-Rom spline implementation. Haven't dig into it and not sure if it's because I draw the spline with interpolated points and yours is bezier curve.

But it looks similar to our original bezier curve implementation without limiting min, max value. The problem is the curve will exceed the extremas. For example under the 0 baseline or exceed the max value(if the max value has some meaning like 100 percent.

![image](https://user-images.githubusercontent.com/841551/93729676-1cb05200-fbf8-11ea-869b-f46e90dda42a.png)

If we limiting the curve between global min, max extremas.
https://echarts-bench-from-uplot.glitch.me/uplot-smoothing-limit-extrema.html

It's almost like the result in this PR before optimization.

![image](https://user-images.githubusercontent.com/841551/93729893-e9ba8e00-fbf8-11ea-8723-4e41fd0dab09.png)

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang merged pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang merged pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

echarts-bot[bot] commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695880093


   Congratulations! Your PR has been merged. Thanks for your contribution! 👍


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695911751

> i've tried feeding typed arrays to uPlot in the past and surprisingly did not measure a difference in performance. i'll take a closer look in a few days at your improvement to the uPlot bench, but i was surprised that typed arrays made a significant difference for you. maybe i should look into it again.

Perhaps it's because echarts use a different data structure that has worse performance than uplot when using a normal array.

I noticed the data structure used in uplot is
```js
[
[........], // x dimension
[........], // y dimension
[........] // z dimension
]
```

It's much flatter than the structure echarts used.

```js
[
[x, y, z],
[x, y, z],
[x, y, z]
]
```

Which may need less heap memory and have less GC pressure.

Using TypedArray in echarts is mainly for getting away long GC time because TypedArray don't take the heap memory. I tested read/write of TypedArray, it has no big difference than a normal array.

> out of curiosity, does echarts handle data gaps by default or is this an additional setting? in uPlot the null-testing per-datapoint adds ~20% overhead to the bench in both, dataset min/max finding as well as path drawing. i decided it was an okay trade-off instead of bloating the code and maintaining slightly different dedicated loops for gap-less data.

echarts handles null data by default. Haven't tested how much time can be saved if we remove the null testing. But I guess it should not the bottleneck for echarts currently.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695762009


   Did some modifications to make these two versions look better(won't affect the performance.
   
   <img width="1928" alt="infoflow 2020-09-20 16-47-34" src="https://user-images.githubusercontent.com/841551/93707425-099e7300-fb61-11ea-89ae-d925d65c302e.png">
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695777118


   PS: I did both the performance improvement and smoothing algorithm improvement when refactoring the polyline builder. So I didn't try to separate it into two PRs. Sorry about that if it brings any inconvenience.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695879944

https://echarts-bench-from-uplot.glitch.me/uplot-smoothing.html

![image](https://user-images.githubusercontent.com/841551/93729676-1cb05200-fbf8-11ea-869b-f46e90dda42a.png)

If we limiting the curve between global min, max extremes.
https://echarts-bench-from-uplot.glitch.me/uplot-smoothing-limit-extrema.html

It's almost like the result in this PR before optimization.

![image](https://user-images.githubusercontent.com/841551/93729893-e9ba8e00-fbf8-11ea-8723-4e41fd0dab09.png)

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] leeoniya commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

leeoniya commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695785582






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] leeoniya commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

leeoniya commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695746133


   also, your smoothing does not represent the data well. take a look at using the Catmull-Rom Spline (Centripetal) algo:
   
   https://leeoniya.github.io/uPlot/demos/line-smoothing.html


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695758284

@leeoniya Thanks for the benchmark. I'm really impressed by the performance of uPlot!

I add an optimized version that takes a Float64Array as input data. It's a bit more complex to write but has better performance and takes less heap memory.

Optimized version:
https://echarts-bench-from-uplot.glitch.me/index-optimized.html

Original verison:
https://echarts-bench-from-uplot.glitch.me/

Tested it on my 15inch mac. The initialization time is about 170ms. And the heap memory is about 10~12MB(peak), 5MB(final).

About the smoothing algorithm. I have tried the Catmull-Rom Spline before. But it has similar issues which looks quite weird

I'm not sure if you did any modification on the spline.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695911751

Perhaps it's because echarts use a different data structure that has worse performance than uplot when using a normal array.

I noticed the data structure used in uplot is
```js
[
[........], // x dimension
[........], // y dimension
[........] // z dimension
]
```

It's much flatter than the structure echarts used.

```js
[
[x, y, z],
[x, y, z],
[x, y, z]
]
```

Which may need less heap memory and have less GC pressure.

Using Float64Array in echarts is mainly for getting away from long GC time because TypedArray don't take the heap memory. I tested read/write of Float64Array, it has no big difference than a normal array.

Also in echarts we provide a faster way to process the Float64Array data. Meanwhile, when processing the normal array we need to check the structure of each item in the array because the structure we supported is quite flexible. It may be convenient for the developers, but not good for the performance:(

echarts handles null data by default. Haven't tested how much time can be saved if we remove the null testing. But I guess it should not the bottleneck for echarts currently.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695911751

Perhaps it's because echarts use a different data structure that has worse performance than uplot when using a normal array.

I noticed the data structure used in uplot is
```js
[
[........], // x dimension
[........], // y dimension
[........] // z dimension
]
```

It's much flatter than the structure echarts used.

```js
[
[x, y, z],
[x, y, z],
[x, y, z]
]
```

Which may need less heap memory and have less GC pressure.

Using Float64Array in echarts is mainly for getting away long GC time because TypedArray don't take the heap memory. I tested read/write of Float64Array, it has no big difference than a normal array.

Also in echarts we provide a faster way to process the Float64Array data. When processing the normal array we need to check the structure of each item in the array because of the flexible structure we provided. It may be convenient for the developers, but not good for the performance:(

echarts handles null data by default. Haven't tested how much time can be saved if we remove the null testing. But I guess it should not the bottleneck for echarts currently.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695777118


   PS: I did both the performance imrpvoment and smoothing algorithm improvement when refactoring the polyline builder. So I didn't try to separate it into two PRs. Sorry about that if it brings any inconvenience.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-698183829


   Hi @leeoniya 
   
   I did a bit more optimization in these two days. Now the time has been reduced to about 140ms. With LTTB downsample enabled. It can be less than 100ms on my 15inch macbook.
   
   https://echarts-bench-from-uplot.glitch.me/index-lttb.html
   
   I also add two test cases with 10x larger data.
   
   https://echarts-bench-from-uplot.glitch.me/index-large.html
   https://echarts-bench-from-uplot.glitch.me/index-lttb-large.html
   
   Just want to thank you and your uplot that motivate me pushing the performance of echarts to the limit.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695879944

https://echarts-bench-from-uplot.glitch.me/uplot-smoothing.html

I tried it and the result seems a bit different from my Catmull-Rom spline implementation. Haven't dig into it and not sure if it's because I drew the spline with interpolated points and in the uplot are bezier curves.

![image](https://user-images.githubusercontent.com/841551/93729676-1cb05200-fbf8-11ea-869b-f46e90dda42a.png)

If we limiting the curve between global min, max extremes.
https://echarts-bench-from-uplot.glitch.me/uplot-smoothing-limit-extrema.html

It becomes almost the same as the result before change in this PR.

![image](https://user-images.githubusercontent.com/841551/93729893-e9ba8e00-fbf8-11ea-8723-4e41fd0dab09.png)

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

echarts-bot[bot] commented on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695714878


   Thanks for your contribution!
   The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/incubator-echarts/wiki/How-to-make-a-pull-request).
   
   The pull request is marked to be `PR: author is committer` because you are a committer of this project.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695911751

Perhaps it's because echarts use a different data structure that has worse performance than uplot when using a normal array.

I noticed the data structure used in uplot is
```js
[
[........], // x dimension
[........], // y dimension
[........] // z dimension
]
```

It's much flatter than the structure echarts used.

```js
[
[x, y, z],
[x, y, z],
[x, y, z]
]
```

Which may need less heap memory and have less GC pressure.

Using Float64Array in echarts is mainly for getting away long GC time because TypedArray don't take the heap memory. I tested read/write of Float64Array, it has no big difference than a normal array.

Also in echarts we provide a faster way to process the Float64Array data. When processing the normal array we need to check the structure of each item in the array because the structure we supported are quite flexible. It may be convenient for the developers, but not good for the performance:(

echarts handles null data by default. Haven't tested how much time can be saved if we remove the null testing. But I guess it should not the bottleneck for echarts currently.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org

[GitHub] [incubator-echarts] pissang edited a comment on pull request #13314: [5.0] [Performance] Improve large line performance. Improve line smoothing.

Posted by GitBox <gi...@apache.org>.

pissang edited a comment on pull request #13314:
URL: https://github.com/apache/incubator-echarts/pull/13314#issuecomment-695777118






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org