no synchronisation mechanism needed for portfolio workers#840
Merged
quickbeam123 merged 3 commits intomasterfrom Apr 21, 2026
Merged
no synchronisation mechanism needed for portfolio workers#840quickbeam123 merged 3 commits intomasterfrom
quickbeam123 merged 3 commits intomasterfrom
Conversation
mezpusz
reviewed
Apr 20, 2026
| if(!_numWorkers) | ||
| { | ||
| if(!_numWorkers) { | ||
| // TODO this has been around since the ark, is it still worth the weirdness? |
Collaborator
There was a problem hiding this comment.
This is not as weird as it sounds.
It only kicks in if the user explicitly sets --cores 0 which says "give me all" and it gives you less than "all" if you have plenty (i.e. on a server).
Still, open to discussion. At the very least, I can add a comment explaining the semantics...
Contributor
Author
There was a problem hiding this comment.
Ah, OK - and this is desirable on a server, I guess. Comment would be great if you're already looking! Otherwise I can do it.
mezpusz
reviewed
Apr 20, 2026
mezpusz
approved these changes
Apr 20, 2026
Contributor
mezpusz
left a comment
mezpusz
left a comment
There was a problem hiding this comment.
Seems like a nice simplification.
quickbeam123
force-pushed
the
michael-no-portfolio-sync
branch
from
5c6b02c to
3e6e8d1
Compare
MichaelRawson
commented
Apr 21, 2026
|
|
||
| return result; | ||
| // done with the temporary file now | ||
| fs::remove(path); |
Contributor
Author
There was a problem hiding this comment.
@quickbeam123 this was what I had in mind for removing at least the proof that was printed. Works on my machine. ;-) Let me investigate a bit...
MichaelRawson
force-pushed
the
michael-no-portfolio-sync
branch
from
a5f41a7 to
2150406
Compare
MichaelRawson
force-pushed
the
michael-no-portfolio-sync
branch
from
2150406 to
6cc35c5
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
At some point portfolio workers were synchronised with UNIX semaphores to prevent them trampling over a valid proof.
In #540 I changed this mechanism to a more portable version based on files, but we can do better: just don't synchronise at all, have each child process write to their own temporary file, and have the parent process print the proof.
Closes #548.