Skip to content
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

Reporting progress from an async Promise function #10

Closed
timcassell opened this issue Jan 20, 2021 · 6 comments · Fixed by #54
Closed

Reporting progress from an async Promise function #10

timcassell opened this issue Jan 20, 2021 · 6 comments · Fixed by #54
Labels
enhancement New feature or request
Milestone

Comments

@timcassell
Copy link
Owner

Currently, it is impossible to report progress from an async Promise function. The conventional way of doing so is to pass an IProgress<T> argument into the function.

My proposal is to allow reporting progress to the async Promise via a Promise.ConfigureAwait method.

public async Promise Func()
{
    await DownloadLargeThing().ConfigureAwait(0f, 0.5f);
    await DownloadSmallThing1().ConfigureAwait(0.5f, 0.75f);
    await DownloadSmallThing2().ConfigureAwait(0.75f, 1f);
}

This allows the programmer to manually normalize the progress that is reported. The ConfigureAwait takes 2 arguments: minProgress and maxProgress. When the configured awaitable is awaited, the progress is immediately set to minProgress and goes up to maxProgress as the promise reports its own progress from 0 to 1. This means the progress can go backwards if the programmer so chooses (this can actually be useful for repeating async actions like error retries).

@timcassell timcassell added proposal enhancement New feature or request and removed proposal labels Jan 20, 2021
@timcassell timcassell added this to the v2.1.0 milestone Jan 22, 2021
@timcassell
Copy link
Owner Author

I'm thinking AwaitWithProgress(float min, float max) would be a better method name. Then the other ConfigureAwait with cancelation token proposal #11 can change to WaitAsync with more options like what TPL is doing dotnet/runtime#48842.

@timcassell
Copy link
Owner Author

Placing this here to remove it from the source until it's implemented...

Unfortunately, we can't do a simple if (typeof(TAwaiter) == typeof(PromiseAwaiter<T>)), because we don't have access to the <T> type. (This would work for PromiseAwaiter without a generic type, though.)

internal interface IPromiseAwaiter
{
    void AwaitOnCompleted(Internal.AsyncPromiseRef asyncPromiseRef);
}

PromiseAwaiters implement this interface. This can be used to subscribe to progress for the async Promise().

public void AwaitOnCompleted<TAwaiter, TStateMachine>(ref TAwaiter awaiter, ref TStateMachine stateMachine)
    where TAwaiter : INotifyCompletion
    where TStateMachine : IAsyncStateMachine
{
    if (typeof(TAwaiter).IsValueType && typeof(IPromiseAwaiter).IsAssignableFrom(typeof(TAwaiter)))
    {
        AwaitOverrider<TAwaiter>.awaitOptimizer.AwaitOnCompleted(ref awaiter, _promise);
    }
    else
    {
        awaiter.OnCompleted(_promise.GetContinuation(ref stateMachine));
    }
}

In the builder, the IsValueType and IsAssignableFrom can theoretically be constant-optimized by the JIT/AOT compiler. IsValueType I believe is already optimized this way in some runtimes/compilers, while IsAssignableFrom I don't think is, but there is an open issue in .Net runtime to add that optimization. In any case, the IsValueType helps it avoid that check when it's not necessary.

internal abstract class AwaitOverrider<T> where T : INotifyCompletion
{
    internal static AwaitOverrider<T> awaitOptimizer;

    private AwaitOverrider() { }

    internal static void Create<TAwaiter>() where TAwaiter : struct, T, ICriticalNotifyCompletion, IPromiseAwaiter
    {
        awaitOptimizer = new Impl<TAwaiter>();
    }

    internal abstract void AwaitOnCompleted(ref T awaiter, Internal.AsyncPromiseRef asyncPromiseRef);

    private sealed class Impl<TAwaiter> : AwaitOverrider<T> where TAwaiter : struct, T, IPromiseAwaiter
    {
        internal override void AwaitOnCompleted(ref T awaiter, Internal.AsyncPromiseRef asyncPromiseRef)
        {
            if (typeof(T) == typeof(TAwaiter))
            {
                Unsafe.As<T, TAwaiter>(ref awaiter).AwaitOnCompleted(asyncPromiseRef);
                return;
            }
            throw new System.InvalidOperationException("type T {" + typeof(T).FullName + "} is not TAwaiter {" + typeof(TAwaiter).FullName + "}");
        }
    }
}

This is the meat of how it works. typeof(T) == typeof(TAwaiter) is already optimized away by the JIT/AOT compiler. This also takes advantage of Unsafe.As. I'm not sure the best way of using that in Unity, if there's a way to link to the Unsafe assembly as a dependency without including the dll for old runtimes. If necessary, we can do a (TAwaiter)(object) awaiter cast (.Net 4.0+ optimizes that operation to not box, though we still end up making a copy of the struct).

static PromiseAwaiter()
{
    AwaitOverrider<PromiseAwaiter<T>>.Create<PromiseAwaiter<T>>();
}

And this is how the awaiters actually subscribe themselves to the AwaitOverrider while still working in AOT/IL2CPP environments without reflection.

@timcassell
Copy link
Owner Author

timcassell commented Nov 14, 2021

I actually think I figured out a cleaner way to handle this without Unsafe.As (though the type checks are still necessary in the builder):

internal abstract class AwaitOverrider<T> where T : INotifyCompletion
{
    internal static AwaitOverrider<T> awaitOptimizer;

    internal abstract void AwaitOnCompleted(ref T awaiter, Internal.AsyncPromiseRef asyncPromiseRef);
}

internal sealed class AwaitOverriderImpl<TAwaiter> : AwaitOverrider<TAwaiter> where TAwaiter : struct, ICriticalNotifyCompletion, IPromiseAwaiter
{
    internal static void Create()
    {
        awaitOptimizer = new AwaitOverriderImpl<TAwaiter>();
    }

    internal override void AwaitOnCompleted(ref TAwaiter awaiter, Internal.AsyncPromiseRef asyncPromiseRef)
    {
        awaiter.AwaitOnCompleted(asyncPromiseRef);
    }
}
static PromiseAwaiter()
{
    AwaitOverriderImpl<PromiseAwaiter<T>>.Create();
}

@timcassell
Copy link
Owner Author

timcassell commented Nov 14, 2021

It is unfortunate that we have to resort to a virtual function call here and allocate an object to enable the virtual call, when the call itself should be directly callable. If the compiler would allow method overloads in the AsyncMethodBuilder, we could add an overload very easily without all this extra ceremony:

public void AwaitOnCompleted<T, TStateMachine>(ref PromiseAwaiter<T> awaiter, ref TStateMachine stateMachine)
    where TStateMachine : IAsyncStateMachine
{
    awaiter.AwaitOnCompleted(_promise);
}

But that's not possible (the compiler-generated state machine bakes in the default method call).

We could use a delegate instead of a class with a virtual function, but that still has to allocate the delegate and actually costs more than a virtual call.

C# 9 added function pointers, which we can take advantage of to remove the allocation and extra overhead of virtual function/delegates, so it just calls the function directly via its pointer:

internal abstract unsafe class AwaitOverrider<T> where T : INotifyCompletion
{
    internal static delegate*<ref T, Internal.AsyncPromiseRef, void> action;
}

internal sealed unsafe class AwaitOverriderImpl<TAwaiter> : AwaitOverrider<TAwaiter> where TAwaiter : struct, ICriticalNotifyCompletion, IPromiseAwaiter
{
    internal static void Create()
    {
        action = &AwaitOnCompleted;
    }

    private static void AwaitOnCompleted(ref TAwaiter awaiter, Internal.AsyncPromiseRef asyncPromiseRef)
    {
        awaiter.AwaitOnCompleted(asyncPromiseRef);
    }
}

Unfortunately, the call cannot be in-lined, as it's determined at runtime, but I think that's the best we can do until/unless the compiler ever allows overloads in the builder.

@timcassell
Copy link
Owner Author

According to Stephen Toub, the JIT will optimize away ((IPromiseAwaiter)awaiter).AwaitOnCompleted(_promise) to not box, so we can simplify the builder method:

public void AwaitOnCompleted<TAwaiter, TStateMachine>(ref TAwaiter awaiter, ref TStateMachine stateMachine)
    where TAwaiter : INotifyCompletion
    where TStateMachine : IAsyncStateMachine
{
    if (typeof(TAwaiter).IsValueType && typeof(IPromiseAwaiter).IsAssignableFrom(typeof(TAwaiter)))
    {
        ((IPromiseAwaiter)awaiter).AwaitOnCompleted(_promise);
    }
    else
    {
        awaiter.OnCompleted(_promise.GetContinuation(ref stateMachine));
    }
}

But, we can't rely on that optimization in Unity with Mono or IL2CPP, so we still have to use the AwaitOverrider method to avoid boxing. We can #ifdef to use AwaitOverrider for Unity and this method for other runtimes.

@timcassell
Copy link
Owner Author

timcassell commented Jan 26, 2022

I'm thinking of pulling this into v2.0 to be able to create a nuget package with progress enabled (#51). It wouldn't make much sense to have progress enabled by default without async Promise progress support. I will need to do some initial work to see if this can be done like ((IPromiseAwaiter)awaiter).AwaitOnCompleted(_promise) without boxing in Unity which would make this much easier.

[Edit] I just tested in latest Unity, and it still doesn't optimize away the box there (it still allocates). Looks like I'll have to do this the hard way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant