-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Remove some unsafes by using atomics #2186
Conversation
Web Assembly is going to support threads and atomics (see proposal). I'm not sure if this is a good solution to avoiding |
A shame that the proposal includes only sequential consistency instructions, which makes
|
I don't have a strict opinion on it. If it doesn't degrade the performance, I'm fine with keeping the atomics |
Blocked due to benchmarks not working atm |
@WorldSEnder could you rebase? Benchmarks are fixed but it will end up comparing older yew with master |
4dd7372
to
1a11593
Compare
@voidpumpkin Done. The |
@WorldSEnder benchmark results can be found here: https://github.com/yewstack/yew/runs/4659120701?check_suite_focus=true#step:15:24 I agree that we should make them more visible. I had to dig through the logs to find them |
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.
04_select1k
seems to be scuffed and shows performance decreases even when there are no changes.
03_update10th1k_x16
seems like it was a performance increase.
everything else stayed marginal
Though idk how much we can trust those benchmarks. For this PR it is enough though.
Description
Removes some uses of unsafe operations by substituting them with
atomic
operations. These are, due to the single-threaded nature of the current wasm target, lowered to normal operations on simple datatypes, as far as I know.Uses
Ordering::Relaxed
since none of the operations are used as synchronization primitives.Checklist
cargo make pr-flow