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

System.Threading.Tasks.Parallel.Tests fails on Android #50566

Closed
1 task
mdh1418 opened this issue Apr 1, 2021 · 3 comments · Fixed by #73486
Closed
1 task

System.Threading.Tasks.Parallel.Tests fails on Android #50566

mdh1418 opened this issue Apr 1, 2021 · 3 comments · Fixed by #73486

Comments

@mdh1418
Copy link
Member

mdh1418 commented Apr 1, 2021

One test from System.Threading.Tasks.Parallel.Tests fails on Android

  • System.Threading.Tasks.Tests.ParallelForEachAsyncTests.Dop_WorkersCreatedRespectingLimitAndTaskScheduler_Sync(dop: -1)
System.Threading.Tasks.Parallel.Tests.dll   Failed: 1

Test collection for System.Threading.Tasks.Parallel.Tests.dll
System.Threading.Tasks.Tests.ParallelForEachAsyncTests.Dop_WorkersCreatedRespectingLimitAndTaskScheduler_Sync(dop: -1)
    Assert.InRange() Failure\r\nRange:  (0 - 1)\r\nActual: 2

Based on CI runs from #50095

@ghost
Copy link

ghost commented Apr 1, 2021

Tagging subscribers to this area: @carlossanlop
See info in area-owners.md if you want to be subscribed.

Issue Details

One test from System.Threading.Tasks.Parallel.Tests fails on Android

  • System.Threading.Tasks.Tests.ParallelForEachAsyncTests.Dop_WorkersCreatedRespectingLimitAndTaskScheduler_Sync(dop: -1)

Based on CI runs from #50095

Author: mdh1418
Assignees: -
Labels:

area-System.Threading.Tasks, os-android

Milestone: 6.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 1, 2021
@SamMonoRT SamMonoRT removed the untriaged New issue has not been triaged by the area owner label Jun 23, 2021
@adamsitnik adamsitnik modified the milestones: 6.0.0, 7.0.0 Jul 28, 2021
steveisok pushed a commit to steveisok/runtime that referenced this issue Aug 5, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 5, 2022
@mdh1418
Copy link
Member Author

mdh1418 commented Aug 6, 2022

It looks like

If it is -1, there is no limit on the number of concurrently running operations (with the exception of the ForEachAsync method, where -1 means ProcessorCount.

from https://docs.microsoft.com/en-us/dotnet/api/system.threading.tasks.paralleloptions.maxdegreeofparallelism?view=net-6.0#system-threading-tasks-paralleloptions-maxdegreeofparallelism is not being honored, as in the ProcessorCount is 1, but the Parallel.ForEachAsync is using 2 schedulers.

@mdh1418
Copy link
Member Author

mdh1418 commented Aug 6, 2022

Looking through the code path, it seems like the test with MaxDegreeOfParallelism = -1 (unlimited) with MaxSchedulerLimit = 2 passed through Parallel.ForEachAsync will end up having EffectiveMaxConcurrencyLevel = 2

internal int EffectiveMaxConcurrencyLevel
{
get
{
int rval = MaxDegreeOfParallelism;
int schedulerMax = EffectiveTaskScheduler.MaximumConcurrencyLevel;
if ((schedulerMax > 0) && (schedulerMax != int.MaxValue))
{
rval = (rval == -1) ? schedulerMax : Math.Min(schedulerMax, rval);
}
return rval;
}
}
} // class ParallelOptions
and does not hit this code path which should have set it to the expected Environment.ProcessorCount
internal static int DefaultDegreeOfParallelism = Math.Min(Environment.ProcessorCount, MAX_SUPPORTED_DOP);

But I'm not sure why this fails only on Android. Is its Environment.ProcessorCount not supposed to be 1?

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants