Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Could this run in a worker? #100

Open
oskbor opened this issue Jan 11, 2023 · 10 comments
Open

Could this run in a worker? #100

oskbor opened this issue Jan 11, 2023 · 10 comments
Labels
improvement Feature improvement or enhancement

Comments

@oskbor
Copy link

oskbor commented Jan 11, 2023

Greetings!

Do you know if it would be possible to run most of this library in a worker?
Perhaps by using an OffscreenCanvas.
Perhaps the number[]'s that are passed in could be replaced by TypedArrays since they can be backed by ArrayBuffers that are transferable objects

Just curious if this idea has crossed your mind. This library is great and a central part of an application that we are currently building, hence why I'm thinking of ways to squeeze out even more performance.

best regards Oskar

@flekschas
Copy link
Owner

100% I was thinking about moving the library entirely into a worker at some point.

Unfortunately, there are few challenges:

  1. My available free time to work on this
  2. OffscreenCanvas is not yet supported in Safari, the Internet Explorer I suppose... However, dropping support for all iOS devices seems too drastic. Hence, all but the renderer would have to be moved to a web worker while the renderer remains in the main frame.

I think the second point can be addressed by moving all but the renderer into a web worker while keeping the renderer in the main browser thread.

Regarding the first point, I'd really love to workerize this library but I can't tell you when I'd have a chance to get around doing it. I am happy to collaborate if you have spare resources.

Also, I'd love to switch to TypedArrays entire and drop the internal use of standard arrays. However, the spatial index library we're using only works with row-based arrays. I.e., arrays of points instead of TypedArrays of x and y coordinates. We'd have to fix this as well. It's not that hard (I did it for another project) but I never gotten around implementing a proper PR for kdbush. Maybe I'll get started with it.

@flekschas flekschas added the improvement Feature improvement or enhancement label Jan 11, 2023
@oskbor
Copy link
Author

oskbor commented Jan 12, 2023

  1. My available free time to work on this

I can relate 😄 Nice that you have had the same thoughts. At this time I cannot justify doing this, but I would be happy to contribute once we encounter large enough datasets to motivate this work 👍

I think the second point can be addressed by moving all but the renderer into a web worker while keeping the renderer in the main browser thread.

Perhaps the renderer can be built so it runs in a worker when supported and otherwise it falls back to the main thread 🤔

@fspoettel
Copy link
Collaborator

@flekschas would you accept a contribution here? I would be open to do the required work to implement this, but might need some pointers in the process.

We are currently using regl-scatterplot to plot a large number of plots from parquet/arrow tables which we load/parse in a web worker. Even though we have to cast the TypedArrays to Arrays at some point, this already performs quite well (we can plot millions of points across ~30 plots on a page in an acceptable time), but looking at the profiling data, there is some jank that occurs whenever kdbush prepares v. large plots or a GC of intermediate values occurs. I think rendering in a web worker would solve most of this.

With OffscreenCanvas set to ship in Safari 17, it may soon be possible to also leverage that for drawing.

@flekschas
Copy link
Owner

I'm totally open to contributions! However, just be warned that the full workerization might be more involved due to the fact that multiple regl-scatterplot instances (can) share a renderer that holds the actual WebGL context.

I started prototyping a complete worker-based approach some time ago and eventually got the basic setup to work. However, the performance was somehow not great at all. My approach was the following:

Each regl-scatterplot instance creates a main thread plus worker pair. The purpose of the main thread instance is to forward all communication to the worker and provide access to a canvas element via the OffscreenCanvas. In addition, the first regl-scatterplot instance creates another main thread plus worker instance: the renderer. The only reason the renderer needs to know of the main thread is to get access to a hidden canvas element via the OffscreenCanvas. Only the renderer worker creates a WebGL context. Data that is being loaded with a plot instances is processed and transferred to the renderer who holds all WebGL programs. Additionally, the plot instances pass access to their visible canvas elements on to the renderer. Finally, the renderer renders out the pixels using the hidden canvas element and then copies them onto the visible canvas elements.

The setup is somewhat complex but having a single renderer makes it possible to create many plot instances without having to worry about the max number of WebGL contexts. This is important for libraries like jupyter-scatter which use regl-scatterplot.

I'm happy to share more details

@fspoettel
Copy link
Collaborator

fspoettel commented Sep 11, 2023

Thank you for the write-up, appreciate it! Interesting that you did not see performance benefits with your approach.

I started prototyping a complete worker-based approach some time ago and eventually got the basic setup to work.

Is this available on a branch somewhere? Would be helpful to be able to look at the code / run it. I can also share more details about how we currently use regl-scatterplot in our (biomedical) react application if you're interested.

but having a single renderer makes it possible to create many plot instances without having to worry about the max number of WebGL contexts.

We ran into this issue as well and are using regl-scatterplot with a shared webgl context for that reason. I found that beyond the hard context limit, using isolated webgl contexts per plot also comes with considerable performance overhead (mostly memory) when drawing more than a few plots.

@flekschas
Copy link
Owner

It's not yet available on any branch as I was mostly just experimenting with the overall architecture and how to get the communication (between all the workers) working in general. I invited you to the repo. Note, the whole thing is extreme bare bone yet but it should give you an idea. Happy to jump or a call or so to see how we could move this forward but this week is a bit crazy due to some deadlines. Next week should be better.

@fspoettel
Copy link
Collaborator

Thank you! I'm on holiday until end of september, unlikely to start working on this before. I'll shoot you an email to the mail on your website (nice one btw) once I'm back to get the ball rolling.

@flekschas
Copy link
Owner

Sounds good and enjoy vacation! 🥳

@oskbor
Copy link
Author

oskbor commented Dec 7, 2023

Hi guys,
has there been any developments on this?
Glad to see that @fspoettel is also interested in this feature.

@fspoettel
Copy link
Collaborator

Hey @oskbor, yes, there has been. @flekschas and I discussed this topic in a call, I still need to write the followups for that.

We identified that a good place to start would be the kdbush index generation which is one of the main blockers on the main thread. We decided that a good way to start here is doing two things:

  1. move the kdbush index generation to a worker.
  2. and benchmark how much faster it is. (here we want to add some general benchmark harnesses)

I started working on those. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Feature improvement or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants