Runtime issue 14477 more immutable benchmarks #2306
Conversation
| for (int i = 0; i < uniqueValues.Length; i++) | ||
| collection.TryAdd(uniqueValues[i], uniqueValues[i]); | ||
| return collection; | ||
| } |
There was a problem hiding this comment.
I didn't end up adding TryAdd benchmarks for the immutable collections here because they just use the TryAdd extension which does not work for them.
| @@ -21,6 +21,7 @@ public class CopyTo<T> | |||
| private T[] _array; | |||
| private List<T> _list; | |||
| private ImmutableArray<T> _immutablearray; | |||
| private ImmutableList<T> _immutableList; | |||
There was a problem hiding this comment.
All collection shave the CopyTo method (e. g. dictionaries, sets, etc) but since the original file only went with lists and arrays I also took that approach. Happy to add the rest of the collections if desired
| @@ -25,22 +25,5 @@ public void InitializeContainsValue() | |||
|
|
|||
| [Benchmark] | |||
| public object Clone() => new Dictionary<int, int>(_dict); | |||
There was a problem hiding this comment.
The original recommendation was to remove this method and make a Clone benchmark for all collections.
There is already a CtorFromCollection benchmark which largely covers this case and includes immutable collections.
What that benchmark doesn't test is the optimization immutable collections can have where CreateRange on another instance of the same type of immutable collection can just return the input. However, that doesn't seem worth benchmarking as it can easily be unit-tested.
| { | ||
| var immutableList = _immutableList; | ||
| for (int i = 0; i < immutableList.Count; i++) | ||
| immutableList = immutableList.SetItem(i, default); |
There was a problem hiding this comment.
SetItem is not technically an indexer set, but in other benchmarks (e. g. the Ctor benchmarks) there was precedent for including the immutable equivalent of a mutable method. So, I felt like it made sense to include this here rather than creating a separate SetItem benchmark. Let me know if you disagree.
| _immutableList = Immutable.ImmutableList.CreateRange(GenerateValues()); | ||
|
|
||
| [Benchmark] | ||
| public ImmutableList<T> ImmutableList() => _immutableList.Sort(); |
There was a problem hiding this comment.
Most of this benchmark assumes mutable sort, so it's setup involves populating N lists/arrays upfront and then sorting each of them on subsequent iterations. That doesn't make sense for immutable collections, but will the fixed invocation count throw off the measurement at all here?
|
@adamsitnik any concerns with these? |
|
@adamsitnik anything else to do here? |
See dotnet/runtime#14477 (comment) and #2268 (comment) for context.