Migrate new convenience APIs into extension methods

[karelz]

Highly used generic types (mostly Collections) pay high cost for every API (method) we add to it.

We should revisit the decisions of adding new APIs (esp. convenience APIs) directly on the type and consider moving them into extension methods instead (in future and potentially revisit those we added during 2.0 and did not publish yet).

Context: see @jkotas's comment and guidance here: dotnet/coreclr#10203 (comment)

[karelz]

@terrajobst @stephentoub @weshaggard @danmosemsft @KrzysztofCwalina we should discuss it on Tuesday the latest ...

@jkotas might want to join us if we expect difference in opinions during the discussion ;-)

[karelz]

cc @ianhays @safern @AlexGhiondea @joperezr for Collections and Runtime areas which are potentially impacted the most in CoreFX.

[danmoseley]

Remove -- https://github.com/dotnet/corefx/issues/15622
TryAdd https://github.com/dotnet/corefx/issues/1942 -- this one I think we were OK with the perf and code size

[jkotas]

It is not a high cost for every API. It becomes high cost once the per-API cost gets multiplied by non-trivial number of APIs. Death by thousand paper cuts sort of problem.

Also, this is not a black and white. Some new additions are fine - in particular the ones that make a significant dent in the usability of the type (e.g. new APIS on ArraySegment), but there is balance to strike.

[karelz]

Here's a proposal (for common generic classes):

1. If it is primarily convenience API (usability), make it extension method, unless it would have really bad perf which we believe matters.
   • Examples: Dictionaty<,>.GetValueOrDefault(), Remove, etc.
2. If it is core API for the type, add it into the type - any recent examples?
3. If it is perf-sensitive API which cannot be implemented in extension method efficiently, add it on the type
   • Note: When unsure about perf-sensitivity, make it extension method - we can always move it into the type later when the value is clear.
   • Examples: TryAdd (see https://github.com/dotnet/corefx/issues/17275#issuecomment-287564658)

[stephentoub]

Examples: Dictionaty<,>.GetValueOrDefault(), Remove, TryAdd, etc.

TryAdd was convenience but also to avoid two lookups when one would suffice, in a commonly-used pattern. The latter part can't be done externally to the type.

[benaadams]

Some apis get a benefit from being a member e.g. Dictionary<TKey,TValue>.TryAdd not doing a double check

Others just use the public interface so shouldn't loose much from being an extension
e.g.

• Span<T>.Clear() -> .Clear(this Span<T>) (though struct copy overhead?)
• Span<T>.Fill(T value) -> .Fill(this Span<T>, T value)
• Array.Fill<T>(T[] array, T value) -> .Fill<T>(this T[] array, T value)
• Array.Fill<T>(T[] array, T value, int startIndex, int count) -> .Fill<T>(this T[] array, T value, int startIndex, int count)

Slightly tangential, but related does ValueTuple need to implement 7 interfaces? (especially the ones that end up boxing)

[karelz]

Thanks @stephentoub @benaadams for pointing out that TryAdd belongs to category [3] and not [1]. I didn't go deeper on the examples - I probably should have. My proposal is now updated to avoid further confusion.

[benaadams]

Another reason for using extensions on generic types is it allows for specialisation of the methods based on type (e.g. Span.IndexOf(this Span<T>, ...) that also has Span.IndexOf(this Span<byte>, ...))

[terrajobst]

We discussed this. Conclusion:

• Let's create a list of all the convenience APIs like the Remove with an out, GetValueOrDefault and decided what to do with it.
• We don't want to "hack" the APIs to optimize the code gen, but practically speaking we need to keep those constraints in mind.
• In this instance, we already have an extension home (CollectionExtensions) and we can design most of the convenience target the interfaces in which case extension methods are the most logical design point.

[karelz]

@ianhays can you please list the APIs we added in 2.0 on any collection? @weshaggard can help if needed.
Thanks!

[danmoseley]

Is a principle established for API's that provide perf benefits, like TryAdd? I assume those can continue to go on the real types so long as they don't excessively bloat the code nor have a perf impact on existing API.