Question about some pinning code in futures-rs

help
1

I am looking over some code in futures-rs and I wonder why things are written this way:

impl<T> Future for FutureObj<'_, T> {
    type Output = T;

    #[inline]
    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<T> {
        let pinned_field: Pin<&mut LocalFutureObj<'_, T>> = unsafe {
            self.map_unchecked_mut(|x| &mut x.0)
        };
        LocalFutureObj::poll(pinned_field, cx)
    }
}

Had I written this, I would have done:

impl<T> Future for FutureObj<'_, T>
{
	type Output = T;

	fn poll( mut self: Pin<&mut Self>, cx: &mut Context<'_> ) -> Poll<T>
	{
		LocalFutureObj::poll( Pin::new( &mut self.0 ), cx )
	}
}

At first sight, this compiles and works fine. Am I missing something? Does this have perf issues? It does get rid of an unsafe block and a bunch of code.

Also, I wonder for something like this, is it really useful to keep the #[inline] mention? I would think LLVM would figure out what's best?

2

Both versions work because FutureObj<T> is Unpin for any T. The upstream implementation works with or without unpin, so I'm guessing it either didn't used to be unpin, this code predates the current unpin logic, or the author used the usual pinning projection code for consistency.

The two should perform equivalently.

The #[inline] shouldn't be necessary in an ideal world, but you'll find it a lot in library code providing combinators and type wrappers, where it really can make a difference if the complier's inlining heuristics fail. I saved 6% on a binary recently using async by placing a carefully considered #[inline].

3

That unsafe block dates from before LocalTaskObj: Unpin, presumably no-one has noticed it is redundant since then.

1 Like
4

Do you want a PR to remove it? Is simply using Pin::new fine?

5

Yes, if we can reduce the unsafe code used it's definitely worth updating.

6

Ok, I'll do that tomorrow and survey a bit for other candidates.

7

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.