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

docs: An example that implements broadcast communication #656

Merged
merged 9 commits into from
Sep 25, 2024

Conversation

testwhygh
Copy link
Contributor

Copy link
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

This is amazing, thanks!
Could you rather add it to the docs/examples section?

Copy link
Collaborator

@ronag ronag left a comment

Choose a reason for hiding this comment

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

You should also make sure to set the flag that makes sure piscina doesn't use blocking Atomics.wait

@metcoder95 metcoder95 changed the title An example that implements broadcast communication docs: An example that implements broadcast communication Sep 15, 2024
@metcoder95 metcoder95 added the backport v4 Backport work label related with v4.x label Sep 15, 2024
@testwhygh
Copy link
Contributor Author

This is amazing, thanks! Could you rather add it to the docs/examples section?

Of course, I will also add it to docs/docs/examples

@testwhygh
Copy link
Contributor Author

You should also make sure to set the flag that makes sure piscina doesn't use blocking Atomics.wait

Yes, setting the flag useAtomics to false avoids using Atomics.wait, but why should I do this? Is it for better concurrency performance?

@metcoder95
Copy link
Member

You should also make sure to set the flag that makes sure piscina doesn't use blocking Atomics.wait

Yes, setting the flag useAtomics to false avoids using Atomics.wait, but why should I do this? Is it for better concurrency performance?

Is to avoid the thread to be blocked when idle (we use Atomic.wait to put the thread on hold until further tasks are enqueued).

@testwhygh
Copy link
Contributor Author

You should also make sure to set the flag that makes sure piscina doesn't use blocking Atomics.wait

Yes, setting the flag useAtomics to false avoids using Atomics.wait, but why should I do this? Is it for better concurrency performance?

Is to avoid the thread to be blocked when idle (we use Atomic.wait to put the thread on hold until further tasks are enqueued).

Thanks, I have modified it.

metcoder95
metcoder95 previously approved these changes Sep 18, 2024
Copy link
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

lgtm

@testwhygh testwhygh closed this Sep 23, 2024
@testwhygh testwhygh reopened this Sep 23, 2024
@testwhygh
Copy link
Contributor Author

lgtm
Hello, should I do anything else to get this PR completed?

const Piscina = require('piscina');
const piscina = new Piscina({
filename: resolve(__dirname, 'worker.js'),
useAtomics: false
Copy link
Member

Choose a reason for hiding this comment

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

Let's just add a comment of why useAtomics is disabled, and it should be ready to go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, it is necessary

@metcoder95 metcoder95 merged commit d57084a into piscinajs:current Sep 25, 2024
10 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 25, 2024
Co-authored-by: Carlos Fuentes <me@metcoder.dev>
(cherry picked from commit d57084a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport v4 Backport work label related with v4.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants