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

[Architecture] Support of Web Workers #2195

Open
Desplandis opened this issue Sep 25, 2023 · 4 comments
Open

[Architecture] Support of Web Workers #2195

Desplandis opened this issue Sep 25, 2023 · 4 comments
Assignees
Labels
architecture 🏬 proposal 👍 Proposal of a new feature

Comments

@Desplandis
Copy link
Contributor

Desplandis commented Sep 25, 2023

This issue is a technical discussion opened to all contributors and users of the library. Feel free to upvote (with 👍 ), comment, propose a solution and provide use-cases related to this issue.

Context

By default, the browser uses a single thread to run all the JavaScript in your page [...]. This means that long-running JavaScript functions can block the thread, leading to an unresponsive page and a bad user experience. [Source MDN]

The Web Workers API provides a set of utilities to run a scripts in background threads. The worker thread can perform tasks without interfering with the user interface. Communication between a spawned worker and the main thread is done via bidirectional message passing.

In iTowns, we only have little CPU time between the rendering of two frames (~16.67ms). Any CPU-bound may block the main thread and by extension cause delays in event processing or painting. As a consequence, a user may find that iTowns feels sluggish in terms of framerate.

One example of such slowness in iTowns is the visualization of large point-clouds. By profiling the enwine_simple_loader example, we find that during event/painting delays (janks), 80% of CPU time is used by the LAZ decompression procedure.

This issue aims to discuss a correct way to integrate Web Workers in iTowns and identify parts of the library that may takes advantage of Web Workers (see Identified use-cases below).

The multiple Problems

First and foremost, Web Workers are a pain to integrate seamlessly for users let alone for library authors. A non exhaustive list of such problems (feel free to extend):

  • Web worker scripts are self-contained (and should be bundled with all their dependencies) and follows the pre-ES6 semantics. There is support for ESM modules but this is only recently (June 2023) supported by all major engines.
  • No dependency sharing between a Web worker and its parent context leading to a duplication of common dependencies.
  • Message-passing through postMessage serialize data and can easily be used inefficiently (See Is postMessage slow).
  • No Promise-based API (see Comlink for an easier way to work with workers).
  • No "standard" way to manage a pool of worker (spawning, scheduling, etc...).
  • Browsers requires that Workers scripts to be of same-origin as the parent context creating them (which excludes the use of CDNs).

All those problems are exacerbated as a library as workers should work out-of-the-box with all bundlers. However...

  • There is no standard way to bundle Web Workers.
  • All libraries implements their own Worker pool.

The not-so-satisfying Solutions

The new URL(...) way

A recommended way to bundling external resources is to use the following syntax:

new Worker(new URL('./worker.js', import.meta.url));

where ./worker.js is a relative path known statically at bundle-time.

The new URL(...) constructor takes a relative URL as the first argument and resolves it against an absolute URL provided as the second argument. In our case, the second argument is import.meta.url which gives the URL of the current JavaScript module, so the first argument can be any path relative to it.

Bundlers could then recognize this syntax and automatically bundle web workers. This is supported by all major bundlers: parcel, rollup, vite, webpack.

Note that the import.meta.url is only a valid syntax in the context of EcmaScript ESM modules. However since we are currently transpiling and distributing iTowns as CommonJS (CJS) modules, this is not a valid syntax...

I experimented with disabling transpilation to CJS modules and adding a "type": "module" to our package.json but it breaks unit testing since mocha does not seems to support mocking ESM modules.

The inlining way

Another way would be to inline the worker code as a string during the transpilation.

  • Original code:
new Worker(new URL('./worker.js', import.meta.url));
  • Transpiled code:
  const workerString = "worker.js bundle code";

  const blob = new Blob([workerString]);
  const url = URL.createObjectURL(blob);
  const worker = new Worker(url);

This would however necessitates to implement a pass in babel to pre-bundle the worker using a bundler (either webpack or rollup). Moreover, inlining worker may lead to Content Security Policy issues.

Identified use-cases

TLDR

We should not expect users of the library to encapsulate CPU-bound procedures (e.g. point cloud parsing) as the ESM standard does not provide an "easy" way to spawn a thread. However the lack of a standard way to bundle Web Workers leaves us with basically two choices with their own set of problems...

@jailln
Copy link
Contributor

jailln commented Oct 3, 2023

Thanks for this thorough analysis ! 🙂

Regarding the packaging, which is indeed the first the point to address, my opinion is that we should implement both solutions:

  • the new URL(...) way because it allows to address ESM modules users and it seems to be the way to go with web workers. (we can also check what are the options for mocha but if there are too complicated we can use the bundle created with the inlining way)
  • the inlining way because we still have users of the commonjs module (and maybe also for unit testing). From what I understand from the article you linked, the csp issue can be solved with the correct server configuration so documenting this extra server configuration needed for users of the commonjs module should be enough (it can be seen as the cost of not using ESM modules 🙂 )

WDYT?

@Makaronelle
Copy link

I would add that both ESM and CJS can be kept under the same umbrella but exposed the end-user through differents exports in the package.json.

If there is a way to keep both solution and make that work through named exports and still keep compatibility for CJS users, it would be a win-win situation.

@Desplandis
Copy link
Contributor Author

We finally decided to completely dropped support for CJS distribution (see proposal #2256). We'll push in a coming PR distribution as ES-only module (I already have a fully working branch). This could ease the use of workers for users with modern packers as well as fixing duplicates dependencies issues. Users of the itowns bundle will have their workers inlined by webpack (with worker-loader).

BTW, I experimented with worker inlining with webpack + babel, however those scripts are a bit hacky and I don't want to add more junk into iTowns.

Since I'm not here on the coming week, I expect to push two PRs on those subjects (ESM distrib + Workers) the week of the 26th of February.

@Makaronelle Could I ping you to test some of those branches?

@jailln
Copy link
Contributor

jailln commented Dec 20, 2024

@Desplandis I think this has been implemented now :) Should we close it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture 🏬 proposal 👍 Proposal of a new feature
Projects
None yet
Development

No branches or pull requests

4 participants