Micro-optimization of Remote::poll in compio-executor.

[abh1nav10]

The implementation of the poll method for Remote checks whether the current waker needs to be updated using will_wake and calls State::start_setting_waker only if it is required. If yes, it then checks whether a waker was inserted in that window and drops it unconditionally. We could insert another check using will_wake on that waker as if it returns true, we could avoid two atomic operations as in one for the increment of the ref-count while cloning the waker and another when the waker gets dropped. Further, even if the setting of waker in the window is highly unlikely, the branch predictor will end up being right most of the times.

I tried to change it from

                   if state.is_cancelled() {
                        self.header().state.finish_setting_waker::<false>();

                        return ControlFlow::Break(Poll::Ready(None));
                    }

                    let waker = unsafe { &mut *ptr };

                    if state.has_waker() {
                        unsafe { waker.assume_init_drop() };
                    }

                    if state.is_completed() && state.has_result() {
                        self.header().state.finish_setting_waker::<false>();

                        return ControlFlow::Continue(state);
                    }

                    waker.write(cx.waker().clone());

to the following snippet

                    if state.is_cancelled() {
                            self.header().state.finish_setting_waker::<false>();
                            return ControlFlow::Break(Poll::Ready(None));
                    }

                    let waker = unsafe { &mut *ptr };

                    let has_waker = if state.has_waker() {a
                        Some(unsafe { waker.assume_init_read() })
                    } else {
                        None
                    };

                    if state.is_completed() && state.has_result() {
                        self.header().state.finish_setting_waker::<false>();

                        return ControlFlow::Continue(state);
                    }

                    if let Some(wkr) = has_waker
                        && cx.waker().will_wake(&wkr)
                    {
                        waker.write(wkr);
                    } else {
                        waker.write(cx.waker().clone());
                    }

If this is worth adding, would be happy to make a PR :)

[Berrysoft]

Have you benchmarked the performace?

cc: @George-Miao

[abh1nav10]

I have not yet benchmarked it. I shall get back with the results.

[abh1nav10]

I was trying to recreate a scenario which reflects what the change is trying to optimize for. The change that i made requires the Waker to have changed which would lead the will_wake check before we enter into the critical section by calling start_setting_waker to return false. We then enter the critical section and then in order for this code change to actually do something meaningful, it requires the Waker to be set back to refer to the task context that we are currently in. The first part does happen if lets say the JoinHandle is polled from two different executors on different threads. However, I am trying to figure out whether the second part can ever be true because when lets say we await the JoinHandle on the executor on another thread that propagates down the task context from the highest enclosing async block or spawn a different task that polls the JoinHandle in the executor on the another thread, the second will_wake to turn out to be true requires the Waker for that task context to already be in there I think is unreachable. I think that the change would essentially turn out to be dead code.

Happy to be corrected if I am missing something.

[George-Miao]

Well, that's not really possible to happen since there will be only one JoinHandle, which is not cloneable. There's only one writer exist in the first place. SETTING_WAKER flag is only used by executor to avoid waking the waker when it's finished or cancelled.

[abh1nav10]

Right, I was under the impression that it could be polled concurrently as JoinHandle is Sync but completely missed the point that polling requires a Pin<&mut Self> which we cant get via a reference to JoinHandle. Thanks!!

[George-Miao]

But still, thanks a lot for all the effort trying to optimize the executor!