The spread operator is significantly slower than using the special constructor when converting data to ObservableCollection

[HotCakeX]

Take this benchmark for example

public class ObservableCollectionBenchmark<T>
{
    private List<T> Data;

    [GlobalSetup]
    public void Setup()
    {
        Data = Enumerable.Range(0, 10000).Select(i => (T)Convert.ChangeType(i, typeof(T))).ToList();
    }

    [Benchmark]
    public void CollectionExpression()
    {
        ObservableCollection<T> Result = [];
        Result = [.. Data];
    }

    [Benchmark]
    public void ConstructorInitialization()
    {
        ObservableCollection<T> Result = [];
        Result = new ObservableCollection<T>(Data);
    }

    public static void Run()
    {
        var summary = BenchmarkRunner.Run<ObservableCollectionBenchmark<int>>();
    }
}

class Program
{
    static void Main()
    {
        ObservableCollectionBenchmark<int>.Run();
    }
}

When I run it, i get the following result

As you can see, using [.. ] is a lot slower than using this: https://learn.microsoft.com/en-us/dotnet/api/system.collections.objectmodel.observablecollection-1.-ctor

Processor = Intel 14th gen Core i7

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

[KalleOlaviNiemitalo]

Result = [.. Data]; compiles to a loop that enumerates Data and calls Collection<T>.Add(T) for each item, and each call goes to code in ObservableCollection<T> that constructs multiple instances of EventArgs classes. SharpLab

If #18087 is fixed by adding a Collection<T>.AddRange(IEnumerable<T>) method, then the spread operator can use that instead, which should be faster. That was already added in dotnet/corefx#35772 but soon reverted in dotnet/corefx#38115.

[KalleOlaviNiemitalo]

@HotCakeX, does Visual Studio or dotnet-format suggest a code fix that changes new ObservableCollection<T>(Data) to [.. Data]? If so, that seems a bug because the collection expression is slower and the analyzer could guess this from the fact that an AddRange method is not available.

[stephentoub]

cc: @CyrusNajmabadi

[HotCakeX]

@HotCakeX, does Visual Studio or dotnet-format suggest a code fix that changes new ObservableCollection<T>(Data) to [.. Data]? If so, that seems a bug because the collection expression is slower and the analyzer could guess this from the fact that an AddRange method is not available.

Yes, i had to turn these off

dotnet_diagnostic.IDE0028.severity = silent
dotnet_diagnostic.IDE0306.severity = silent

[jnyrup]

dotnet/roslyn#75981 reports similar problems when constructing a HashSet<T> from another collection using the spread operator.

[CyrusNajmabadi]

We fixed and will no longer report that here.

[jeffhandley]

We fixed and will no longer report that here.

@CyrusNajmabadi I couldn't find where this was fixed, and I see dotnet/roslyn#75981 is still open. Can you help clarify the status of this please?

[CyrusNajmabadi]

@jeffhandley we do not offer for types where we think the semantics will change. We do it when semantics will be the same, but where compiler may not implement efficient code.

The point of collection exprs is to express what you want, and have the compiler implement that efficiently. There are some places where the compiler doesn't do that yet. But that's an issue on the compiler. Grill the language and ide perspective, we move users to the idiomatic form which we think people should be writing.

Unfortunately, with the rifs, I don't know when compiler will get around to optimizing these cases properly. But I hope it still happens.

[jeffhandley]

Thanks, @CyrusNajmabadi. I will close this issue, with dotnet/roslyn#75981 representing the open issue for that optimization. I just wanted to make sure I wasn't overlooking a change already made and alluded to by the previous "We fixed" comment. Thanks for the quick clarification.