Collections benchmarks rewrite #92

adamsitnik · 2018-07-13T19:43:25Z

I was supposed to port collections benchmarks.

After digging more into the existing benchmarks I decided to rewrite them instead. Porting them would take a similar amount of time, whereas they would not benefit from all BDN advantages and it would be hard to compare them. Also, many collections (Immutable ones for example) did not have benchmarks at all.

# Conflicts: # src/benchmarks/Benchmarks.csproj

jorive · 2018-07-13T19:54:03Z

src/benchmarks/Benchmarks.csproj

@@ -6,15 +6,15 @@
    <DebugType>pdbonly</DebugType>
    <DebugSymbols>true</DebugSymbols>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <LangVersion>7.3</LangVersion>
+    <LangVersion>latest</LangVersion>


Good call! 👍 #Closed

adamsitnik · 2018-07-13T19:47:31Z

src/benchmarks/corefx/System.Collections/Add/AddDefaultSize.cs

+namespace System.Collections
+{
+    [BenchmarkCategory(Categories.CoreFX, Categories.Collections, Categories.GenericCollections)]
+    [GenericTypeArguments(typeof(int))] // value type


@jorive @ViktorHofer GenericTypeArguments is the way to tell BDN what T is for generic classes with benchmarks

adamsitnik · 2018-07-13T19:49:09Z

src/benchmarks/corefx/System.Collections/Add/AddDefaultSize.cs

+    {
+        private T[] _uniqueValues;
+
+        [Params(Utils.DefaultCollectionSize)]


@jorive @ViktorHofer Params is the way to parametrize benchmarks per given type, Params can have multiple values like [Params(1, 2, 3)] but we don't have enough time to run all the benchmarks for more test cases so I am using single, same value everywhere

adamsitnik · 2018-07-13T19:50:15Z

src/benchmarks/Benchmarks.csproj

  </PropertyGroup>
  <ItemGroup>
    <Compile Remove="img\**" />
    <EmbeddedResource Remove="img\**" />
    <None Remove="img\**" />
  </ItemGroup>
  <ItemGroup>
-    <PackageReference Include="BenchmarkDotNet" Version="0.10.14.667" />
+    <PackageReference Include="BenchmarkDotNet" Version="0.10.14.683" />


@jorive I needed to update BDN to have glob filtering support #Closed

adamsitnik · 2018-07-17T17:13:58Z

I have just pushed the remaining benchmarks for concurrent collections. The PR is ready for final review.

jorive · 2018-07-17T22:04:46Z

@adamsitnik How are you selecting the scenarios to benchmark for the generic collections? For example, are we covering the basic ICollection<T> interface?

adamsitnik · 2018-07-18T11:04:32Z

@jorive I am using concrete types everywhere, not abstractions.

I could add ICollection<T> , IReadOnlyCollection<T>, IDictionary<T> , IReadOnlyDictionary<T> cases but I would be benchmarking more or less the used collection type + virtual method dispatch overhead.

I think that we should have such benchmarks, but I am not sure if they should be part of the System.Collections benchmarks or CoreClr.Virtualization

@jorive what do you think?

jorive · 2018-07-18T16:14:16Z

@adamsitnik That's a good point, and I think that with categories, we could add them to both :)
I just wanted to understand the cases that we are covering and make sure we have the basics too, for example: insert, remove, find?

jorive · 2018-07-18T17:19:28Z

src/benchmarks/corefx/System.Collections/Create/CtorDefaultSize.cs

+    public class CtorDefaultSize<T>
+    {
+        [Benchmark]
+        public T[] Array() => new T[4]; // array has no default size, List has = 4 so I decided to use 4 here


I am inclined to remove this test as Array does not have default Ctor. #ByDesign

I believe that we can keep it, the value that it provides to me is comparing the cost of array creation vs the cost of creating array wrappers (list, stack, queue). If the latter is much bigger it means we have a problem #Closed

jorive · 2018-07-18T17:29:34Z

src/benchmarks/corefx/System.Collections/Create/CtorGivenSize.cs

+        [Benchmark]
+        public List<T> List() => new List<T>(Size);
+
+#if !NET461 // API added in .NET Core 2.0


I think we should start using #if !NETFRAMEWORK instead. I can see us adding newer Desktop versions to out matrix. Thoughts? #Closed

good idea! #Closed

jorive · 2018-07-18T17:31:16Z

src/benchmarks/corefx/Common/UniqueValuesGenerator.cs

+    {
+        internal static T[] GenerateArray<T>(int count)
+        {
+            var random = new Random(12345); // we always use the same seed to have repeatable results!


// we always use the same seed to have repeatable results! [](start = 44, length = 58)

👍 #Closed

jorive · 2018-07-18T17:32:53Z

src/benchmarks/corefx/Common/UniqueValuesGenerator.cs

+
+        internal static Dictionary<TKey, TValue> GenerateDictionary<TKey, TValue>(int count)
+        {
+            var random = new Random(12345); // we always use the same seed to have repeatable results!


12345 [](start = 36, length = 5)

nit: Can we make the seed a const variable used in both methods? #Closed

jorive · 2018-07-18T18:00:36Z

src/benchmarks/corefx/System.Collections/Add/AddGivenSize.cs

+        public int Count;
+
+        [GlobalSetup]
+        public void Setup() => _uniqueValues = UniqueValuesGenerator.GenerateArray<T>(Count);


nit: Feels like Count here should be replaced with Utils.DefaultCollectionSize, and Count on the benchmarks with _uniqueValues.Length #ByDesign

I am using the value of Param everywhere (not Utils.DefaultCollectionSize) because in the future somebody might like to run this benchmark for more values and then it will require just adding the values to [Param]

[Params(10, 2000, 100000)] public int Size;

BDN will then run the same benchmark for all provided values (10, 2000, 100000) #Closed

jorive · 2018-07-18T18:26:58Z

src/benchmarks/corefx/System.Collections/Contains/ContainsFalse.cs

+        [Benchmark]
+        public bool Array()
+        {
+            bool result = default;


Could you please make this loop dependent? #Closed

Another good point! Done #Closed

jorive · 2018-07-19T18:18:50Z

@adamsitnik We should open an issue about the loop independent variable assignments.

adamsitnik · 2018-07-20T21:37:44Z

We should open an issue about the loop independent variable assignments.

@jorive it's on my TODO list, I will start the discussion right after my holidays. (I don;t want to start a discussion and disappear)

adamsitnik added 19 commits July 11, 2018 20:48

ctor default size all collections benchmarks

13e43b2

ctor given size all collections benchmarks

85f98c1

ctor from collection all collections benchmarks

34fc804

move to a dedicated folder, add categories

af04aa7

iterate for each all collections

8fb685d

iterate for loop all collections

3321633

iterate for each non generic collections

cf7fa2f

iterate for loop non generic collections

578c546

update to BDN which offers glob filtering so I can filter like a pro ;)

cfafed8

Contains for all generic collections

1f81328

ContainsKey for all generic collections

aff1094

add benchmarks

6e68408

try add benchmarks

a954576

Enqueue and Push benchmarks

c9a467b

Indexer Set benchmarks

8317a31

remove benchmarks

fb4a0fe

clear benchmarks

512d24b

refactoring and cleanup + set LangVersion to latest

eb1344f

add missing benchmarks

6ccfc27

adamsitnik requested review from ViktorHofer and jorive July 13, 2018 19:43

Merge remote-tracking branch 'upstream/master' into collectionsRewrite

6bbd707

# Conflicts: # src/benchmarks/Benchmarks.csproj

jorive reviewed Jul 13, 2018

View reviewed changes

adamsitnik added 6 commits July 17, 2018 16:36

fix a typo

41aa067

add missing ConcurrentBag.TryTake benchmark

255f153

AddRemoveFromSameThreads benchmarks

843741c

AddRemoveFromDifferentThreads benchmarks

baf6e76

IsEmpty benchmarks

a99efaf

Count benchmarks

e279cfb

adamsitnik commented Jul 17, 2018

View reviewed changes

support new glob filters

90fa210

jorive reviewed Jul 18, 2018

View reviewed changes

adamsitnik added 5 commits July 19, 2018 12:45

add ICollection, IDictionary and IEnumerable benchmarks

87527d1

use more future proof constant

229c5e6

code review fixes

d734b8e

NETFRAMEWORK is predefined const!

71d8e8e

remove the array benchmark

fb87ea3

jorive approved these changes Jul 19, 2018

View reviewed changes

adamsitnik merged commit 2b3bd56 into dotnet:master Jul 20, 2018

This was referenced Jul 20, 2018

Port System.Collections #47

Closed

Port System.Collections.NonGeneric.Performance.Tests #46

Closed

adamsitnik deleted the collectionsRewrite branch October 17, 2018 15:04

adamsitnik mentioned this pull request Mar 11, 2019

Switch to the dotnet/performance repo dotnet/corefx#35781

Merged

adamsitnik mentioned this pull request Mar 7, 2022

Add some additional micro benchmark coverage for immutable collections. #2268

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collections benchmarks rewrite #92

Collections benchmarks rewrite #92

adamsitnik commented Jul 13, 2018

jorive Jul 13, 2018 •

edited

Loading

adamsitnik Jul 13, 2018

adamsitnik Jul 13, 2018

adamsitnik Jul 13, 2018 •

edited by jorive

Loading

adamsitnik commented Jul 17, 2018

jorive commented Jul 17, 2018

adamsitnik commented Jul 18, 2018

jorive commented Jul 18, 2018 •

edited

Loading

jorive Jul 18, 2018 •

edited

Loading

adamsitnik Jul 19, 2018 •

edited by jorive

Loading

jorive Jul 18, 2018 •

edited

Loading

adamsitnik Jul 19, 2018 •

edited by jorive

Loading

jorive Jul 18, 2018 •

edited

Loading

jorive Jul 18, 2018 •

edited

Loading

jorive Jul 18, 2018 •

edited

Loading

adamsitnik Jul 19, 2018 •

edited by jorive

Loading

jorive Jul 18, 2018 •

edited

Loading

adamsitnik Jul 19, 2018 •

edited by jorive

Loading

jorive commented Jul 19, 2018 •

edited

Loading

adamsitnik commented Jul 20, 2018

Collections benchmarks rewrite #92

Collections benchmarks rewrite #92

Conversation

adamsitnik commented Jul 13, 2018

jorive Jul 13, 2018 • edited Loading

Choose a reason for hiding this comment

adamsitnik Jul 13, 2018

Choose a reason for hiding this comment

adamsitnik Jul 13, 2018

Choose a reason for hiding this comment

adamsitnik Jul 13, 2018 • edited by jorive Loading

Choose a reason for hiding this comment

adamsitnik commented Jul 17, 2018

jorive commented Jul 17, 2018

adamsitnik commented Jul 18, 2018

jorive commented Jul 18, 2018 • edited Loading

jorive Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

adamsitnik Jul 19, 2018 • edited by jorive Loading

Choose a reason for hiding this comment

jorive Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

adamsitnik Jul 19, 2018 • edited by jorive Loading

Choose a reason for hiding this comment

jorive Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

jorive Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

jorive Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

adamsitnik Jul 19, 2018 • edited by jorive Loading

Choose a reason for hiding this comment

jorive Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

adamsitnik Jul 19, 2018 • edited by jorive Loading

Choose a reason for hiding this comment

jorive commented Jul 19, 2018 • edited Loading

adamsitnik commented Jul 20, 2018

jorive Jul 13, 2018 •

edited

Loading

adamsitnik Jul 13, 2018 •

edited by jorive

Loading

jorive commented Jul 18, 2018 •

edited

Loading

jorive Jul 18, 2018 •

edited

Loading

adamsitnik Jul 19, 2018 •

edited by jorive

Loading

jorive Jul 18, 2018 •

edited

Loading

adamsitnik Jul 19, 2018 •

edited by jorive

Loading

jorive Jul 18, 2018 •

edited

Loading

jorive Jul 18, 2018 •

edited

Loading

jorive Jul 18, 2018 •

edited

Loading

adamsitnik Jul 19, 2018 •

edited by jorive

Loading

jorive Jul 18, 2018 •

edited

Loading

adamsitnik Jul 19, 2018 •

edited by jorive

Loading

jorive commented Jul 19, 2018 •

edited

Loading