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

[API Proposal]: Parallel.ForAsync #59019

Closed
0x90d opened this issue Sep 13, 2021 · 7 comments · Fixed by #84804
Closed

[API Proposal]: Parallel.ForAsync #59019

0x90d opened this issue Sep 13, 2021 · 7 comments · Fixed by #84804
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading.Tasks
Milestone

Comments

@0x90d
Copy link

0x90d commented Sep 13, 2021

Background and motivation

#1946 recently Parallel.ForEachAsync has been approved and added to .NET6. However there still no async of Parallel.For. Developers still have to use the same workarounds as for Parallel.ForEach on .NET5 and below to make it run asychronously. An async version of Parallel.For has been mentioned two times in the linked issue, but was left out of discussion so I couldn't find a reason why it hasn't been made async yet. I simply assume that it wasn't taken into consideration because it was not explicit proposed.

As for the motivation I think the same arguments for Parallel.ForEachAsync also applies to Parallel.ForAsync

API Proposal

namespace System.Threading.Tasks
{
    public static class Parallel
    {
        public static Task ForAsync(int fromInclusive, int toExclusive, Func<int, CancellationToken, ValueTask> body);
        public static Task ForAsync(int fromInclusive, int toExclusive, CancellationToken cancellationToken, Func<int, CancellationToken, ValueTask> body);
        public static Task ForAsync(int fromInclusive, int toExclusive, ParallelOptions parallelOptions, Func<int, CancellationToken, ValueTask> body);
        public static Task ForAsync(long fromInclusive, long toExclusive, Func<long, CancellationToken, ValueTask> body);
        public static Task ForAsync(long fromInclusive, long toExclusive, CancellationToken cancellationToken, Func<long, CancellationToken, ValueTask> body);
        public static Task ForAsync(long fromInclusive, long toExclusive, ParallelOptions parallelOptions, Func<long, CancellationToken, ValueTask> body);
    }
}

API Usage

List<DuplicateItem> duplicates = new();
//duplicates.Add(...);
//...

await Parallel.ForAsync(0, duplicates.Count, new ParallelOptions { CancellationToken = cancellationTokenSource.Token}, async (i, token) => {
	await duplicates.GenerateThumbnail();
	
	for (int j = i + 1; j < duplicates.Count; j++) {
		//...
	}
});

Risks

No response

@0x90d 0x90d added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Sep 13, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Threading.Tasks untriaged New issue has not been triaged by the area owner labels Sep 13, 2021
@ghost
Copy link

ghost commented Sep 13, 2021

Tagging subscribers to this area: @dotnet/area-system-threading-tasks
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

#1946 recently Parallel.ForEachAsync has been approved and added to .NET6. However there still no async of Parallel.For. Developers still have to use the same workarounds as for Parallel.ForEach on .NET5 and below to make it run asychronously. An async version of Parallel.For has been mentioned two times in the linked issue, but was left out of discussion so I couldn't find a reason why it hasn't been made async yet. I simply assume that it wasn't taken into consideration because it was not explicit proposed.

As for the motivation I think the same arguments for Parallel.ForEachAsync also applies to Parallel.ForAsync

API Proposal

namespace System.Threading.Tasks
{
    public static class Parallel
    {
        public static Task ForAsync<T>(int fromInclusive, int toExclusive, Func<T, CancellationToken, ValueTask> body);
        public static Task ForAsync<T>(int fromInclusive, int toExclusive, CancellationToken cancellationToken, Func<T, CancellationToken, ValueTask> body);
        public static Task ForAsync<T>(int fromInclusive, int toExclusive, ParallelOptions parallelOptions, Func<T, CancellationToken, ValueTask> body);
        public static Task ForAsync<T>(long fromInclusive, long toExclusive, Func<T, CancellationToken, ValueTask> body);
        public static Task ForAsync<T>(long fromInclusive, long toExclusive, CancellationToken cancellationToken, Func<T, CancellationToken, ValueTask> body);
        public static Task ForAsync<T>(long fromInclusive, long toExclusive, ParallelOptions parallelOptions, Func<T, CancellationToken, ValueTask> body);
    }
}

API Usage

List<DuplicateItem> duplicates = new();
//duplicates.Add(...);
//...

await Parallel.ForAsync(0, duplicates.Count, new ParallelOptions { CancellationToken = cancellationTokenSource.Token}, async (i, token) => {
	await duplicates.GenerateThumbnail();
	
	for (int j = i + 1; j < duplicates.Count; j++) {
		//...
	}
});

Risks

No response

Author: 0x90d
Assignees: -
Labels:

api-suggestion, area-System.Threading.Tasks, untriaged

Milestone: -

@jeffhandley jeffhandley added this to the Future milestone Oct 4, 2021
@jeffhandley jeffhandley added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Oct 4, 2021
@WeihanLi
Copy link
Contributor

Any plan for this?

@stephentoub
Copy link
Member

Any plan for this?

Not currently. In the meantime, you can of course just use ForEachAsync with a range:

Parallel.ForEachAsync(Enumerable.Range(0, count), async (i, cancellationToken) =>
{
    ...
});

@buyaa-n buyaa-n removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Mar 22, 2022
@stephentoub
Copy link
Member

As for the motivation I think the same arguments for Parallel.ForEachAsync also applies to Parallel.ForAsync

Sort of. With Parallel.ForEachAsync, the primary motivation was being able to consume IAsyncEnumerable<T>, and once we're doing that, overloads that also support async bodies also make sense (and then once you have async bodies, you also want to consider IEnumerable<T>... it's a slippery slope). With Parallel.For, IAsyncEnumerable<T> isn't a factor; the source is always synchronous since it's just a constant range. There's still the possibility of an async body, but that's the minor aspect of the APIs. There are also multiple ways to achieve the same basic functionality, e.g. the aforementioned layering on ForEachAsync, using Partitioner.Create to split up the range into individual iterators and spawning a task to process each partition, etc.

If we decide it's worthwhile to add Parallel.ForAsync, the proposed API set looks good to me. I've marked it as ready for review to have a broader discussion about whether we want ForAsync at all.

@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jan 21, 2023
@stephentoub stephentoub modified the milestones: Future, 8.0.0 Apr 1, 2023
@bartonjs
Copy link
Member

bartonjs commented Apr 13, 2023

Video

Looks good as proposed

namespace System.Threading.Tasks
{
    public static class Parallel
    {
        public static Task ForAsync(int fromInclusive, int toExclusive, Func<int, CancellationToken, ValueTask> body);
        public static Task ForAsync(int fromInclusive, int toExclusive, CancellationToken cancellationToken, Func<int, CancellationToken, ValueTask> body);
        public static Task ForAsync(int fromInclusive, int toExclusive, ParallelOptions parallelOptions, Func<int, CancellationToken, ValueTask> body);
        public static Task ForAsync(long fromInclusive, long toExclusive, Func<long, CancellationToken, ValueTask> body);
        public static Task ForAsync(long fromInclusive, long toExclusive, CancellationToken cancellationToken, Func<long, CancellationToken, ValueTask> body);
        public static Task ForAsync(long fromInclusive, long toExclusive, ParallelOptions parallelOptions, Func<long, CancellationToken, ValueTask> body);
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 13, 2023
@stephentoub stephentoub self-assigned this Apr 13, 2023
@stephentoub
Copy link
Member

stephentoub commented Apr 13, 2023

Temporarily moving back to api-ready-for-review to discuss collapsing the int/long overloads to be just based on T : notnull, IBinaryInteger<T>

@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-approved API was approved in API review, it can be implemented labels Apr 13, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 13, 2023
@stephentoub stephentoub added the blocking Marks issues that we want to fast track in order to unblock other important work label Apr 16, 2023
@bartonjs
Copy link
Member

bartonjs commented Apr 20, 2023

Video

We brought this back to discuss collapsing the int/long variants into a single generic representation, with a result of we do feel like one generic version is the better approach.

The discussion says that IBinaryInteger is the correct restriction, so we're going with that.

namespace System.Threading.Tasks
{
    public static partial class Parallel
    {
        public static Task ForAsync<T>(T fromInclusive, T toExclusive, Func<T, CancellationToken, ValueTask> body) where T : notnull, IBinaryInteger<T>;
        public static Task ForAsync<T>(T fromInclusive, T toExclusive, CancellationToken cancellationToken, Func<T, CancellationToken, ValueTask> body) where T : notnull, IBinaryInteger<T>;
        public static Task ForAsync<T>(T fromInclusive, T toExclusive, ParallelOptions parallelOptions, Func<T, CancellationToken, ValueTask> body) where T : notnull, IBinaryInteger<T>;
    }
}

@bartonjs bartonjs removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 20, 2023
@bartonjs bartonjs added the api-approved API was approved in API review, it can be implemented label Apr 20, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 25, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading.Tasks
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants