-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Scheduling and Task Safety Improvements #753
Conversation
I haven't reviewed everything thoroughly, but I agree in principle with what's going on here. I'm not sure about the atomic reference counting for tasks - having those have special reference counting needs kinda smells funny. There's a good bit of asserts and logging that are lost in these patches. Running this locally I see regressions in multithreaded runs of the make check, invalid reads that didn't exist before. They all seem to originate in task::worker, which is really terribly unsafe, so I'm in the process of removing it from the codebase. I'll see if I get better results then. |
OK, I checked in a commit that removed task::worker and things seem much more reliable now |
Well, single-threaded anyway. With 'RUST_THREADS=32 make check-stage1-cfail' I get errors like: ==2124== Thread 5: and free: ptr 0xcd64fb4 is not in allocation_list |
Thanks, brson! Atomic reference counting is there because tasks are reference by multiple threads. No other data (well... almost) is shared between tasks, which is why the task structure itself is special. This is a temporary measure anyway, eventually we'll just have handles. |
Tasks are spawned on a random thread. Currently they stay there, but we should add task migration and load balancing in the future. This should drammatically improve our task performance benchmarks.
that absolutely will not succeed with a large default stack. This should be removed once we have stack grown working. Also updated word-count to succeed under the new test framework.
…s appears to give us much better parallel performance. Also, commented out one more unsafe log and updated rust_kernel.cpp to compile under g++
…towards atomic reference counting of tasks.
After further experimentation I don't think the valgrind problems I reported are caused by this patch set. |
Integrated. |
typo fix: add missing `by`
* add testcase for enzyme loads behind phis * handle load trough phis * adjust test for older llvms * adjust test for older llvms * adjust test for even older llvms
* Move compiletest out of `x.py` * Fixups * Add `pwd` to `--build-base` and split cmd into several lines * Use correct mode for `rmc-docs` suite
Schedulers now each have their own lock, which reduces the contention on the big scheduler lock. This means we regularly get much better performance out of tasks.
Scheduler threads also wait better. Rather than using
sync::sleep
, they uselock.wait()
, which lets them be woken up quicker when tasks become unblocked. Again, a performance improvement.To make this not crash all the time, it was necessary to use atomic reference counts for task. Most of this will be undone later though, as we move to a handle-based system.