Mark PLINQ as enabled in WASM and make browser compat changes #58227
Conversation
|
Tagging subscribers to this area: @tarekgh, @dotnet/area-system-linq-parallel Issue DetailsThis PR enables PLINQ for wasm and makes adjustments to avoid hitting threading code while in the browser, and ideally will fix #43752
|
|
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsThis PR enables PLINQ for wasm and makes adjustments to avoid hitting threading code while in the browser, and ideally will fix #43752
|
|
you'll need to remove the exclusion from src/libraries/tests.proj to get them to run in ci |
| for (int i = 0; i < locks.Length; i++) | ||
| { | ||
| lock (locks[i]) | ||
| if (!OperatingSystem.IsBrowser()) { |
There was a problem hiding this comment.
Is this needed to actually fix something, or is it here just to silence the analyzer?
I'm not excited at the idea of sprinkling these conditions through the bowels of the PLINQ code.
There was a problem hiding this comment.
It's protecting the synchronization API from being called, since it won't work in the browser, and satisfying the analyzer
There was a problem hiding this comment.
Why are any locks being created in the first place?
The PLINQ code base is insanely complicated. I'm worried that we're sprinkling around these IsBrowser checks, both complicating the code in general and also potentially missing something of relevance to the browser.
There was a problem hiding this comment.
If you would prefer that I reimplement all the relevant classes, I can do that. There's a lot of threading baked directly into the PLINQ primitives.
There was a problem hiding this comment.
@stephentoub The code now uses a SinglePartitionMode helper property that acts as a [SupportedOSPlatformGuard("browser")]. It results in the same amount of logic branch sprinkling, but it's now more expressive.
Does this address your concerns?
kg
added
the
NO-MERGE
src/libraries/System.Linq.Parallel/src/System/Linq/ParallelEnumerable.cs
Outdated
Show resolved
Hide resolved
jeffhandley
removed
the
NO-MERGE
…merable.cs Co-authored-by: Jeff Handley <jeffhandley@users.noreply.github.com>
marek-safar
changed the title
ghost
locked as resolved and limited conversation to collaborators
This PR enables PLINQ for wasm and makes adjustments to avoid hitting threading code while in the browser, and ideally will fix #43752