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

improve test-worker.js to exercise all worker types, and syscall.callNow #2217

Merged
merged 2 commits into from
Jan 21, 2021

Conversation

warner
Copy link
Member

@warner warner commented Jan 19, 2021

This fixes and improves test-worker.js to:

  • exercise all worker types, with the same code in each
  • validate the results of the messages, not just the ability to run without crashing
  • exercise syscall.callNow for the workers that can handle it (only local for now, but the upcoming xsnap -based XS worker should be capable too)

@warner warner added the SwingSet package: SwingSet label Jan 19, 2021
@warner warner self-assigned this Jan 19, 2021
@warner warner requested review from FUDCo and dckc January 19, 2021 06:57
@warner
Copy link
Member Author

warner commented Jan 19, 2021

@dckc look at test-worker.js for the spot you should edit to enable the xs-worker test, and mark it as being capable of syscall.callNow

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

Looks good.

Want me to merge it? Should I rebase 1st?

@dckc dckc mentioned this pull request Jan 21, 2021
7 tasks
Also hush some console messages, and test the local worker as well (for
parallelism).

closes #1778
This is the only type of worker which can currently handle syscall.callNow.

The `node-subprocess` case theoretically could (the child subprocess could do a
blocking read of the kernel-to-worker pipe while waiting for the result), but
the code on either side of that pipe does not yet support that mode.

The `nodeWorker` (Node.js threads) case cannot, because threads cannot block,
at least not without heroics involving `Atomic` locks around a
results-bearing `SharedArrayBuffer`.

The old `xs-worker` case could not, for the same shallow implementation
reasons as `node-subprocess`, but the new `xsnap` -based XS worker should be
capable.

closes #1617
@dckc dckc force-pushed the 1778-test-worker-notify branch from 2471c18 to 66aa3d6 Compare January 21, 2021 01:03
@dckc dckc merged commit c3c489e into master Jan 21, 2021
@dckc dckc deleted the 1778-test-worker-notify branch January 21, 2021 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants