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

Proposal: "sync" block in async/iterator methods #11308

Closed
svick opened this issue May 14, 2016 · 12 comments
Closed

Proposal: "sync" block in async/iterator methods #11308

svick opened this issue May 14, 2016 · 12 comments

Comments

@svick
Copy link
Contributor

svick commented May 14, 2016

A relatively common request is to do something in a state machine method (i.e. async method or iterator method) before the state machine is created, either for efficiency (common with async) or for correctness (common with iterators).

In C# 7, these cases can be solved fairly well with local functions (the second example uses arbitrary async returns #10902):

public static IEnumerable<T> Where<T>(this IEnumerable<T> source, Func<T, bool> predicate)
{
    if (source == null || predicate == null)
        throw new ArgumentNullException();

    IEnumerable<T> WhereImpl()
    {
        foreach (var item in source)
        {
            if (predicate(item))
                yield return item;
        }
    }

    return WhereImpl();
}

public static ValueTask<int> DelayWithResult(int delay)
{
    if (delay == 0)
        return ValueTask<int>.FromValue(delay);

    async ValueTask<int> DelayImpl()
    {
        await Task.Delay(delay);
        return delay;
    }

    return DelayImpl();
}

My proposal is to make this even nicer, by including a sync block at the start of the method that contains the code that should execute before the state machine is created. Also, if the sync block returns or throws, the result is returned or thrown directly, without wrapping in Task, tasklike, or IEnumerable<T>.

This means the examples above become:

public static IEnumerable<T> Where(this IEnumerable<T> source, Func<T, bool> predicate)
{
    sync
    {
        if (source == null || predicate == null)
            throw new ArgumentNullException();
    }

    foreach (var item in source)
    {
        if (predicate(item))
            yield return item;
    }
}

public static async ValueTask<int> DelayWithResult(int delay)
{
    sync
    {
        if (delay == 0)
            return ValueTask<int>.FromValue(delay);
    }

    await Task.Delay(delay);
    return delay;
}

This is similar to Defer async state machine creation #10449, except that it's explicit. Being explicit has both advantages (the change of semantics regarding what happens to thrown exceptions is allowed) and disadvantages (the optimization doesn't happen automatically).

The keyword sync is not ideal: it's not currently a keyword and could be confusing for iterator methods. Suggestions for alternative syntax are welcome.

@alrz
Copy link
Member

alrz commented May 14, 2016

I think #119 and #8181 are related. Perhaps these should be merged to a single feature.

public int Insert(T item, int index) {
    pre { guard (index >= 0 && index <= Count); }
    post { guard (value >= 0 && value < Count); }
    return InsertCore(item, index);
}

public static IEnumerable<T> Where<T>(this IEnumerable<T> source, Func<T, bool> predicate) {
    pre {
        guard (source != null) else throw new ArgumentNullException(nameof(source));
        guard (predicate != null) else throw new ArgumentNullException(nameof(predicate));
    }
    foreach (var item in source) {
        if (predicate(item))
            yield return item;
    }
}

public static async ValueTask<int> DelayWithResult(int delay) {
    pre { guard (delay > 0) else return ValueTask<int>.FromValue(delay); }
    await Task.Delay(delay);
    return delay;
}
  • Only a single pre and post is allowed in the method body and they must come first.
  • No await or yield return is allowed in these blocks so that the state machine creation is deferred.
  • The else clause is optional in a guard statement just like method contracts.
  • In a post block value refers to the return value and can be disambiguated with @value if there was an identifier named value in the scope.

@temporaryfile
Copy link

Your sync block would give me a structural way to allow async constructors. The language could enforce that all operations that must be done in a constructor, must be be done in a sync block if it's an async constructor.

@alrz
Copy link
Member

alrz commented May 26, 2016

@svick As it turns out, your implementation with local functions DOES NOT defer the state machine creation in future branch. You must not capture any parameter or variable.

@AlgorithmsAreCool
Copy link

AlgorithmsAreCool commented May 26, 2016

This reminds me a little of powershell's begin, process, end blocks for pipeline functions. Thier aim is for code structure, not performance but the idea is similar.

Here is how they do it.

function Paint-Car
{
    param($car)

    begin {
        #setup the prereqs

        $paintColor = Mix-Paint("blue")
    }

    process {
        #The loop body 
        echo "painting $car!"

        ApplyPaint -Paint $paintColor -Car $car
    }

    end {
         #cleanup
        Cleanup-Paint -Paint $paintColor
    }
}

....

List-Cars -Name Ford* | Paint-Car

@svick
Copy link
Contributor Author

svick commented May 28, 2016

@alrz If I understand the generated code correctly, it does defer the state machine creation. But it also allocates a closure (a.k.a. display class) as soon as the method starts, so it does not avoid allocation as I was indeed assuming.

@svick
Copy link
Contributor Author

svick commented May 28, 2016

@playsomethingsaxman I don't understand how would sync block allow async constructors. If you have an async method, but it only contains a sync block, then according to my proposal, it should be the same as a non-async method.

@alrz
Copy link
Member

alrz commented May 28, 2016

@svick You are right, it's a separate class, but there will be an allocation anyways.

@temporaryfile
Copy link

@svick Watch:

public async Constructor()
{
    // All normal field initialization must be done in this block.
    // Easy.
    sync
    {
    }

    // Being able to inline how the class should initialize asynchronously is a MAJOR
    // readability boon.
    await SwitchToThreadPoolAsync();
    BlahBlahBlah();
}

By the way, why did they gut async set?

@svick
Copy link
Contributor Author

svick commented Jun 2, 2016

@playsomethingsaxman But how do you await the constructor? If it's more like async void, then I don't think that's a good idea.

Also, await SwitchToThreadPoolAsync() is generally considered problematic. If you instead used Task.Run(), you wouldn't even need sync in the first place.

@temporaryfile
Copy link

The constructor isn't to be awaited, if you need that then there's the static factory method pattern. It simply continues initialization without having to call a separate dangling async method that also has the danger of being called again by other class members.

I've built epicly scaleable code with constructs like SwitchTo() because intent just flows so much better and faster. Everyone else is just shooting themselves in the foot with dogma. You can't crank serious code out with Task.Run(). And the .NET Framework thread pool is pure crap anyway, no offense. I've seen it crack and shatter embarrassingly with my butt on the line. It doesn't scale to anything, and people get around it by throwing more servers at the problem. I think that's the real reason for .NET Core, to secretly rewrite the crap, I don't think anyone says it out loud, lol

@jnm2
Copy link
Contributor

jnm2 commented Aug 11, 2016

I would love to see async constructors return Task<T>. There's a good discussion in #6788 on async constructors.

@gafter
Copy link
Member

gafter commented Dec 27, 2017

Issue moved to dotnet/csharplang #1217 via ZenHub

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants