Consider preferring AddRange over CopyTo for collection-expressions of List type

[RikkiGibson]

Spun-off from #71195

See the following simple test case, which has IL size 66 for the Span/CopyTo strategy and 18 for the AddRange strategy.

Perf of these two strategies appears to be similar per #70656 (comment).

Note that using AddRange and SetCount/AsSpan together may be problematic. I think whenever you add to the list you need to call AsSpan again for example. And, uses of SetCount would need to be adjusted.

Since we already prefer AsSpan for collection expressions created from lists without spreads, it's possible that CopyTo is still overall better for uniformity/sanity here.

        [InlineData(new int[0])]
        [InlineData(new[] { (int)WellKnownMember.System_Runtime_InteropServices_CollectionsMarshal__AsSpan_T })]
        [InlineData(new[] { (int)WellKnownMember.System_Runtime_InteropServices_CollectionsMarshal__SetCount_T })]
        [InlineData(new[] { (int)WellKnownMember.System_Runtime_InteropServices_CollectionsMarshal__AsSpan_T, (int)WellKnownMember.System_Runtime_InteropServices_CollectionsMarshal__SetCount_T })]
        [Theory]
        public void ListConstruction_MissingMembers_CollectionsMarshal(int[] missingMembers)
        {
            string source = """
                using System.Collections.Generic;
                class Program
                {
                    static void Main()
                    {
                        var x = F([1, 2, 3]);
                        x.Report();
                    }
                    static List<T> F<T>(T[] items) => [..items];
                }
                """;

            var comp = CreateCompilation(
                new[] { source, s_collectionExtensions },
                targetFramework: TargetFramework.Net80,
                options: TestOptions.ReleaseExe);

            foreach (int missingMember in missingMembers)
            {
                comp.MakeMemberMissing((WellKnownMember)missingMember);
            }

            var verifier = CompileAndVerify(
                comp,
                verify: Verification.FailsPEVerify,
                expectedOutput: IncludeExpectedOutput("[1, 2, 3], "));

            if (missingMembers.Length == 0)
            {
                verifier.VerifyIL("Program.F<T>(T[])", """
                    {
                      // Code size       66 (0x42)
                      .maxstack  5
                      .locals init (T[] V_0,
                                    System.Span<T> V_1,
                                    int V_2,
                                    System.Span<T> V_3)
                      IL_0000:  ldarg.0
                      IL_0001:  stloc.0
                      IL_0002:  newobj     "System.Collections.Generic.List<T>..ctor()"
                      IL_0007:  dup
                      IL_0008:  ldloc.0
                      IL_0009:  ldlen
                      IL_000a:  conv.i4
                      IL_000b:  call       "void System.Runtime.InteropServices.CollectionsMarshal.SetCount<T>(System.Collections.Generic.List<T>, int)"
                      IL_0010:  dup
                      IL_0011:  call       "System.Span<T> System.Runtime.InteropServices.CollectionsMarshal.AsSpan<T>(System.Collections.Generic.List<T>)"
                      IL_0016:  stloc.1
                      IL_0017:  ldc.i4.0
                      IL_0018:  stloc.2
                      IL_0019:  ldloca.s   V_3
                      IL_001b:  ldloc.0
                      IL_001c:  call       "System.Span<T>..ctor(T[])"
                      IL_0021:  ldloca.s   V_3
                      IL_0023:  ldloca.s   V_1
                      IL_0025:  ldloc.2
                      IL_0026:  ldloca.s   V_3
                      IL_0028:  call       "int System.Span<T>.Length.get"
                      IL_002d:  call       "System.Span<T> System.Span<T>.Slice(int, int)"
                      IL_0032:  call       "void System.Span<T>.CopyTo(System.Span<T>)"
                      IL_0037:  ldloc.2
                      IL_0038:  ldloca.s   V_3
                      IL_003a:  call       "int System.Span<T>.Length.get"
                      IL_003f:  add
                      IL_0040:  stloc.2
                      IL_0041:  ret
                    }
                    """);
            }
            else
            {
                // TODO: this codegen is much smaller than the Span/Slice/CopyTo way.
                // should we prefer this across the board for List?
                // microbenchmark?
                verifier.VerifyIL("Program.F<T>(T[])", """
                    {
                      // Code size       18 (0x12)
                      .maxstack  3
                      .locals init (T[] V_0)
                      IL_0000:  ldarg.0
                      IL_0001:  stloc.0
                      IL_0002:  ldloc.0
                      IL_0003:  ldlen
                      IL_0004:  conv.i4
                      IL_0005:  newobj     "System.Collections.Generic.List<T>..ctor(int)"
                      IL_000a:  dup
                      IL_000b:  ldloc.0
                      IL_000c:  callvirt   "void System.Collections.Generic.List<T>.AddRange(System.Collections.Generic.IEnumerable<T>)"
                      IL_0011:  ret
                    }
                    """);
            }
        }

[CyrusNajmabadi]

i think AddRange is fine in a select set of static cases. Repeating here so it doesn't get lost:

1. statically, the type is an interface.
2. statically, the type is either List<T> or some Ilist<T> impl. We know the impl of AddRange optimizes those.
3. the type doesn't have a specialized value-type GetEnumerator

Basically, if the type has a specialized value-type GetEnumerator and is not one of the above, then calling through the helper will box the enumerator, causing garbage.

That seems unfortunate, esp. given that the type stated explicitly it wanted to avoid that.

[RikkiGibson]

When you say "the type" you are referring to the type of the spread operand right?

[RikkiGibson]

It was mentioned offline that we could also improve the IL size (amortized) by emitting helpers for the span case.

• The "new list, set count, get span" flow is a bit weighty. Maybe a compiler-generated helper CreateList<T>(size, out span).
• spreadAsSpan.CopyTo(dest.Slice(...)) could also be shoved into some helper and could shave IL size down. Also, we could consider calling the Slice which takes only a start index, and use an automatic length. Could be another place to microbenchmark.

[MichalPetryka]

Worth noting that for small constant sizes, the JIT will unroll CopyTo and I guess it depends on if and how AddRange is inlined if it'll do that there too.

[RikkiGibson]

Closing out as speculative.