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.

Sign up for GitHub

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

Jump to bottom

refactor: webWorker is optionally passed on last argument #1032

Merged
merged 30 commits into from
Jan 10, 2024
Merged

refactor: webWorker is optionally passed on last argument #1032

merged 30 commits into from
Jan 10, 2024

Conversation

thewtex
Copy link
Member

@thewtex thewtex commented Dec 22, 2023

Re #1018

@thewtex thewtex force-pushed the web-worker-option branch 3 times, most recently from b1e90c8 to 5ea2242 Compare January 5, 2024 04:19
@thewtex thewtex requested a review from PaulHax January 5, 2024 04:19
PaulHax
PaulHax reviewed Jan 5, 2024
View reviewed changes
Copy link
Collaborator

@PaulHax PaulHax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API change makes sense to me. In the future, are you thinking of automaticly terminating the worker in runPipeline if no worker option is passed?

@thewtex
refactor: webWorker is optionally passed on last argument
1268a9a 
In runPipeline and bindgen browser typescript functions, the webWorker
is no longer the first argument but passed on the last options argument.

BREAKING_CHANGE: webWorker is optionally passed on last argument
@thewtex
feat: add terminated property to ItkWorker
3cc9721 
A way to identify that a worker has been terminated.
@thewtex
feat: reuse a default web worker on a package on bundle level
e905aac 
This prevents the need to call .terminate() on the work to avoid
resource leaks. .terminate() can still be called, but the worker is
re-used by default for performance.
@thewtex
Copy link
Member Author

thewtex commented Jan 6, 2024

I added a commit so that calling .terminate() is not necessary to avoid resource leaks by default. While .terminate() still can optionally be called to release memory, the default behavior is to re-use the worker, which is a better performance / memory usage tradeoff in most use cases. Creating the worker is not that expensive, but instantiating the wasm module and allocating the memory it uses is expensive.

@thewtex
feat(run-pipeline): copy when SharedArrayBuffer is not available by d…
53e7861 
…efault

To avoid detached buffers, copy ArrayBuffer's for inputs by default.
This is not necessary for outputs, where ownership is transferred to the
caller. For cases where the inputs are not needed further, a `noCopy`
option can be explicitly passed.
@thewtex
feat(bindgen): pass noCopy option to runPipeline
f15794e 
And add API description in README's.
@thewtex
docs: update runPipeline interface documentation
ea39c0d
@thewtex
docs: note webWorker argument change in BREAKING_CHANGES.md
a001061 
webWorker is moved from the first argument to a property on the last
`options` argument. See also commit 1268a9a
@thewtex
chore: bump itk-wasm version to 1.0.0-b.161
db69b8d
@thewtex
fix(bindgen): pass through null for webWorker to runPipeline
1b9326d
@thewtex
chore: bump itk-wasm version to 1.0.0-b.162
fd5e75f
@thewtex
docs: update image-io package description
3a957fe
@thewtex
docs: update image-io README for web browser documentation updates
949b0d4
@thewtex
feat(readImage): pass through the noCopy option
ca89a1c
@thewtex
chore: bump @itk-wasm/image-io package to 1.0.0
6e2cfae 
Includes breaking API changes (see itk-wasm repo BREAKING_CHANGES.md for itk-wasm
1.0.0-b160).
@thewtex
docs: remove webWorker parameter from readImage, writeImage docs
2de3dbd 
Removed in @itk-wasm/image-io 1.0.0 and passed as an option.
@thewtex
chore: bump @itk-wasm/image-io version to 1.0.1
57ff462
@thewtex
feat(readMesh): pass noCopy to io functions
3c45d81
@thewtex
docs: update @itk-wasm/mesh-io README for API updates
cbe9793
@thewtex
chore: update mesh-io version to 1.0.0
81a2826
@thewtex
docs: fix mesh-io package description
cb81590
@thewtex
docs: update @itk-wasm/compare-images for API changes
b4537e2
@thewtex
chore: bump @itk-wasm/compares-images version to 2.0.0
e0406ad
@thewtex thewtex changed the title WIP: refactor: webWorker is optionally passed on last argument refactor: webWorker is optionally passed on last argument Jan 9, 2024
@thewtex thewtex marked this pull request as ready for review January 9, 2024 04:24
@thewtex thewtex merged commit e2d6a5a into InsightSoftwareConsortium:main Jan 10, 2024
105 of 111 checks passed
@thewtex thewtex deleted the web-worker-option branch January 10, 2024 00:09
@thewtex thewtex mentioned this pull request Jan 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PaulHax PaulHax PaulHax left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@thewtex @PaulHax