refactor(daemon): Coordinate host provideX implementations
#2139
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rekmarks
commented
Mar 13, 2024
12 tasks
rekmarks
force-pushed
the
rekmarks-synchronize-provideWorker
branch
from
5f17afc to
8441d8d
Compare
rekmarks
force-pushed
the
rekmarks-synchronize-makeHost
branch
2 times, most recently
from
133ebdd to
d91fa5b
Compare
rekmarks
force-pushed
the
rekmarks-synchronize-provideWorker
branch
from
8441d8d to
f8dca9c
Compare
rekmarks
force-pushed
the
rekmarks-synchronize-provideWorker
branch
2 times, most recently
from
9a3ce97 to
6d026a6
Compare
rekmarks
force-pushed
the
rekmarks-synchronize-provideWorker
branch
3 times, most recently
from
838a78f to
595e36b
Compare
rekmarks
changed the title
provideX implementations
and etymologically, “order with”. |
kriskowal
approved these changes
Mar 16, 2024
| tasks.push, | ||
| ); | ||
| const workerFormulaIdentifier = tryGetWorkerFormulaIdentifier(workerName); | ||
| prepareWorkerIncarnation(workerName, workerFormulaIdentifier, tasks.push); |
There was a problem hiding this comment.
Apropos of naming, and entirely irrelevant to this PR…
Ain’t it weird that JavaScript inherits the deque methods from Perl:
| front | back | |
|---|---|---|
| put | unshift | push |
| get | shift | pop |
Whereas for a single ended queue, we still reach for French?
| front | back | |
|---|---|---|
| put | enqueue | |
| get | dequeue |
AsyncQueue inherits get and put from priors.
I don’t think enqueue is wrong for serial jobs, but I’m not sure it’s the right word either.
Since you’re here, let me waste your time with this idea that’s been riding rent free in my hindbrain for at least ten years.
| front | back | |
|---|---|---|
| put | pish | posh |
| get | pip | pop |
| peek | tip | top |
| await E(host).provideWorker('foo'); | ||
| await t.throwsAsync(() => E(host).evaluate('foo', '20', [], [], 'bar'), { | ||
| message: | ||
| 'Cannot deliver "evaluate" to target; typeof target is "undefined"', |
There was a problem hiding this comment.
This is almost certainly a symptom of there being no entry in the worker weak map, or the internal facet for this formula being undefined anyway. I regret the internal/external separation and look forward to eliminating it.
rekmarks
force-pushed
the
rekmarks-synchronize-provideWorker
branch
from
595e36b to
328c305
Compare
coordinate, verb, "to combine in harmonious relation or action." Refactors the host's `provideX()` methods such that they all: 1. Attempt to get the existing formula id for the provided name, if any. 2. If there is no existing formula id, incarnate and return a new value. All type checks during step 1 have been removed. In other words, if you attempt to provide something under an existing name that resolves to a different value type, you're on your own. We may revisit this decision in the future.
rekmarks
force-pushed
the
rekmarks-synchronize-provideWorker
branch
from
328c305 to
7483103
Compare
rekmarks
added a commit
that referenced
this pull request
Mar 18, 2024
Fixes the provision of new (i.e. `NEW`) unnamed workers after #2139 broke it. Adds a regression test for this case.
rekmarks
added a commit
that referenced
this pull request
Mar 18, 2024
Fixes the provision of new (i.e. `NEW`) unnamed workers after #2139 broke it. Adds a regression test for this case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Progresses: #2086
Synchronizes the host's
provideWorker(). In addition, coordinates the implementations of the host'sprovideXmethods such that they all:All type checks during step 1 have been removed. In other words, if you attempt to provide something under an existing name that resolves to a different value type, you're on your own. We may revisit this decision in the future.