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

Use affine tasks instead of non-affine #106

Closed
Swoorup opened this issue Nov 16, 2020 · 3 comments · Fixed by #107
Closed

Use affine tasks instead of non-affine #106

Swoorup opened this issue Nov 16, 2020 · 3 comments · Fixed by #107

Comments

@Swoorup
Copy link
Contributor

Swoorup commented Nov 16, 2020

Shouldn't the default behaviour be to use ply affine tasks instead of non-affine.

Reason being that current scheduler shouldn't be ignored. This allows having top-level control over degree of parallelism in which all child tasks would respect as well.

It's easy to override and use default scheduler even if using affine tasks than the other way around

@TheAngryByrd
Copy link
Collaborator

No according to Microsoft general purpose libraries should be using ConfigureAwait(false) https://devblogs.microsoft.com/dotnet/configureawait-faq/

@Swoorup
Copy link
Contributor Author

Swoorup commented Nov 17, 2020

Not sure I agree, that it should be the default. There is a minor performance setback without using ConfigureAwait(false) which I agree, but it also means losing control of parallelism if subsequent child tasks are launched in parallel say in Asp.Net context.

It's possible to install the default/custom synchronization context when using the affine tasks, but not possible if using the non-affine version.

In any case, I think it should be a conscious decision to choose the default.
https://dev.to/noseratio/why-i-no-longer-use-configureawait-false-3pne

For myself since I am using the current library in asp.net project, the problem I am facing with the current setting is inability to control parallelism for subsequent CPU bounded tasks launched as it freely uses all the thread-pool resources, which causes timeouts, as there aren't any free slots in the thread-pool.

Even if we use the affine version, You can get the perf improvement by simply something analogous to the following, but not possible the other way around.

await TaskScheduler.Default.SwitchTo();
await RunOneWorkflowAsync();
await RunAnotherWorkflowAsync();

More information here: See also https://blog.stephencleary.com/2017/03/aspnetcore-synchronization-context.html

@TheAngryByrd
Copy link
Collaborator

Ok I read through those and they seem reasonable. I don't have power currently so could you send a PR with this fix?

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

Successfully merging a pull request may close this issue.

2 participants