-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Refactored the TLS use of the new runtime #7915
Conversation
old design the TLS held the scheduler struct, and the scheduler struct held the active task. This posed all sorts of weird problems due to how we wanted to use the contents of TLS. The cleaner approach is to leave the active task in TLS and have the task hold the scheduler. To make this work out the scheduler has to run inside a regular task, and then once that is the case the context switching code is massively simplified, as instead of three possible paths there is only one. The logical flow is also easier to follow, as the scheduler struct acts somewhat like a "token" indicating what is active. These changes also necessitated changing a large number of runtime tests, and rewriting most of the runtime testing helpers. Polish level is "low", as I will very soon start on more scheduler changes that will require wiping the polish off. That being said there should be sufficient comments around anything complex to make this entirely respectable as a standalone commit.
I thought it would use the commit message as the description. It appears it did not. |
"Creating a no-op task for every scheduler is not going to be an acceptable solution going forward. Is there a plan to avoid this?" Yes, I think this is ugly too. It should be easy to write a version of sched.bootstrap that does not take a task, I just haven't done it yet. This qualifies as "polish" in the sense that startup of the schedulers in the next iteration will change a fair bit to deal with independent work queues. "In that case, can you remove the other context types from the enum and rename GlobalContext to NewRtContext or something?" Sounds like a good tweak. Will make this one now. |
"Why is SchedHome boxed?" This is referring to it being a ~ pointer right, Option<~SchedHome>? If so, no clue. I just wrote it that way. Maybe it makes sense for it to just be Option. This is option-danced somewhat when tasks are sent between schedulers, so maybe I did it for reasons related to that. |
Continued in #8116 |
Fix shadow_same's positive false for async function's params: Example Code: ```rust #![deny(clippy::shadow_same)] pub async fn foo(_a: i32) { } ``` Output: ``` error: `_a` is shadowed by itself in `_a ``` Hir: ```rust pub async fn foo(_a: i32) -> /*impl Trait*/ #[lang = "from_generator"](move |mut _task_context| { let _a = _a; { let _t = { }; _t } }) ``` Skip checking async function's params. changelog: Fix shadow_same's positive false for async function's params
Fixes shadow_same's false positive for rust-lang#7915 Fix shadow_same's false positive for async function's params(Fixes rust-lang#7915): Example Code: ```rust #![deny(clippy::shadow_same)] pub async fn foo(_a: i32) { } ``` Output: ``` error: `_a` is shadowed by itself in `_a ``` Hir: ```rust pub async fn foo(_a: i32) -> /*impl Trait*/ #[lang = "from_generator"](move |mut _task_context| { let _a = _a; { let _t = { }; _t } }) ``` Skip checking async function's params. changelog: Fix shadow_same's false positive for async function's params
No description provided.