dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.3k stars 4.74k forks source link

Improved performance with Task.WhenAll (ImmutableArray<Task> tasks) #29540

Closed Therzok closed 4 years ago

Therzok commented 5 years ago

Should Task.WhenAll have an extension method for ImmutableArray which avoids copies which are done for the array variant?

Clockwork-Muse commented 5 years ago

....I think the larger question would be, "Should we globally add API surface area to avoid copies in the case of ImmutableXXX (or otherwise readonly) datastructures", rather than focusing on Task specifically.

Hmmm.... I wonder if we could/should teach Roslyn to emit ImmutableList<T>/ImmutableArray<T> in the case it can see no modification happens to the collection?

// ImmutableArray<int> instead of int[]
var n = new[] { 0, 1 };
CallingSomeMethod(n);
Therzok commented 5 years ago

@Clockwork-Muse Given how Immutable collections are implemented, a general solution would not work without coordination between corefx and roslyn.

Afaik, Roslyn only optimizes for types in mscorlib, so Immutable collections go out of scope.

Implicit conversions would still allocate an array copy via the immutable collection constructor. I suppose a general purpose fix for this would be way more complex

Clockwork-Muse commented 5 years ago

@Therzok - Oh, I'm aware, it was more just idle musing for that part.

Therzok commented 5 years ago

Just thought about this from a different perspective, which makes the problem really hard to solve in the current world.

Exposing corlib internals to immutable.collections, which would be a layering violation, which most likely makes it not an option.

Then we are left with teaching corlib about ImmutableArray.

Don't know what to say about the other immutable collections. The usage pattern of ImmutableArray in this scenario and possibly many similar others is that we're trying to avoid copying when we can guarantee no copying.

Would ImmutableArray being part of corlib even be considered an option? Is it too much friction/effort for too little gain?

stephentoub commented 5 years ago

Is it too much friction/effort for too little gain?

I think so. But thank you for the suggestion.