danchitnis / webgl-plot

A high-Performance real-time 2D plotting library based on native WebGL
https://danchitnis.github.io/webgl-plot-examples/vanilla/
MIT License
527 stars 29 forks source link

Improved performance addLine #52

Closed lootic closed 3 years ago

lootic commented 3 years ago

Hi! First of all awesome library! Made a suggestion for an improve. Please come back with your opinion on it.

What I did was improve the performance of adding lines by moving the compilation of shaders to the initialization of webglplot, this should decrease the lag that can be seen in the demo when showing random line plots with a huge amount of lines.

danchitnis commented 3 years ago

Hello, thanks for the suggestions. I did a quick test, however I didn't see any difference between the two codes regarding the slow start when using large number of lines.

The slow start is most likely due to JS initialising the arrays rather than WebGL itself. I understand your changes in the code, however, you are making the shaders global to the entire WebGL-Plot class. This is is not the intention. Each line (or line type) has its own shader programme. The WebGL commands have little to no overhead as they are just used for compilation. A better approach is to move the shaders to WebglBaseLine. There all the lines types will share the same shaders. In the future, I will be introducing other line types including thick lines.

The OffScreenCanvas should improve the slow start and responsiveness. Examples coming soon.

lootic commented 3 years ago

Hello again!

I added two benchmarks to support my claim. You can find them under docs/benchmarks. These are made using Firefox, so you should be able to import them there. I think you can see there is a significant change in speed. And if you look at the flame chart you can see that the slower one spends a lot of time in the function addLine. Not sure exactly how the time is used, but I had a hunch it was the shaders, so I tried and could see some results. This is running on somewhat weak specs, but speed is kinda why I like your library. Both of these are when I show a thousand lines.

Furthermore I understand your desired direction with the code, I still think you should only compile the shaders once. I can make a change where the compilation of the shaders are moved to the line class using lazy initialisation. This might also give you better seperation in the code. What do you think?

danchitnis commented 3 years ago

I absolutely agree with you that the shaders for each line type should be compiled only once. Of course, at the moment there is only one type which is WebglBaseLine. I am more than happy to review your changes on this.

Regarding the benchmarks, this seems more complicated. I did run the random line example and increased the lines from 1 to 5000. There results in Chrome and Firefox are very different: On my system, Chromes takes 40s, and Firefox 66s. Chrome does the 5000 lines at 16fps while Firefox at 11fps. Chrome does't seem to mind the the multiple calls on addLine(), but Firefox does! I need a bit investigate this further. I am planning on some more benchmarking on various devices. (I will add you to the project page)

danchitnis commented 3 years ago

I created this project page for Benchmarks: https://github.com/danchitnis/webgl-plot/projects/7#column-10569781

lootic commented 3 years ago

Cool! If you want to know my specs I run: Intel Core i5 2500k CPU @ 3.3Ghz(4CPU) AMD Radeon HD 6900 2048MB RAM

I'll come back with a PR once I find some time, maybe that will be this weekend.

danchitnis commented 3 years ago

I made a dedicated small test which to focuses only on webgl-plot's performance. Your change speeds up the creation of the lines nearly by 100 times. You can copy the folder and test it on your own code. I am still investigating why my previous test didn't show the improvements.

https://github.com/danchitnis/webgl-plot/tree/master/benchmark

lootic commented 3 years ago

Nice work on creating a small benchmark. Ill try it out and see what figures I can get from it. Made another suggestion for how you could initialize the the shaders.

I'll close this PR once I know you've seen this message.