-
Notifications
You must be signed in to change notification settings - Fork 824
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
Fix Wait/Notify opcode, the waiters hashmap is now on the Memory itself #3723
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing to fix, and some recommendations.
I like moving this to the memory.
Note: this can be a separate PR, but we also need some external way to know when a thread goes to sleep (wait) and is woken up (notify).
Moving this to the memory is actually great for that, because now we should be able to add it to the trait, so the behaviour of wait/notify can be customized.
You see any problems with that?
I'm fine with that :) |
Hey guys - this is not asynchronous thus it will block execution and cause quite some performance bottlenecks on the WASI implementation with |
@john-sharratt we will add functionality to extend the wait/notify behaviour in the It will be difficult while also being able to snapshot the stack. Need to talk/think about this, but that can be done as a followup, not in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The Wait/Notify opcodes needs a list a waiters to works.
This list (an hashmap in fact) needs to be on a common object. It makes more sense to have the hashmap being part of the Memory object.
The
LinearMemory
memory trait has been expanded accordingly, with sensible default, that will behave as if the memory was not shared.The actuall hashmap in only implemented for
VMSharedMemory