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

Simplify worker handling #268

Merged
merged 3 commits into from
Jan 14, 2022
Merged

Simplify worker handling #268

merged 3 commits into from
Jan 14, 2022

Conversation

ahocevar
Copy link
Contributor

@ahocevar ahocevar commented Jan 8, 2022

This pull request simplifies worker handling and updates the documentation accordingly. It includes breaking changes (see below).

Workers out of the box

The build and the modules now already contain a worker that is inlined into a new ./dist-module/worker/decoder.js module, so users do not need to worry about how to build the worker. The ./pool.js module uses that worker as a dynamic import, unless the Pool instance is created with a custom createWorker() function as 2nd argument. This makes it easy to use custom workers that add additional decoders.

The inlining of the worker is done with the new scripts/serialize-workers.js script, using rollup and a technique that is borrowed form OpenLayers.

Removal of the threads.js dependency

Worker pools are simple enough to not require a heavy dependency like threads.js. This pull request removes that dependency. This change reduces the resulting build sizes and gets rid of bundler warnings about eval().

Fixes

  • The default pool size calculation has been fixed. It now defaults to 2 threads when the pool size cannot be determined.
  • The npm run dev now works again, thanks to the inlined worker.
  • The Pool test has been resurrected.

Documentation

The API docs have been updated to show how to use a custom decoder worker. I removed the section about the worker overhead for copying data between worker and main thread, because that is no longer necessary thanks to the use of Transferables (this was also the case with threads.js already).

Breaking changes

  • The Pool API has changed. The 2nd argument is no longer a threads/Worker instance, but a function that returns a Web Worker.
  • The package contents have changed. What was previously in the src/ folder is now in the dist-module/ folder. This change was necessary to allow for a good build story of the new ./dist-worker/worker/decoder.js module. All development life cycle scripts have been updated so developers don't need to worry about this change during development.

@constantinius
Copy link
Member

@ahocevar Thanks for the changes!

@constantinius constantinius merged commit a934701 into geotiffjs:master Jan 14, 2022
@ahocevar ahocevar deleted the worker branch January 14, 2022 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants