6

When optimizing a site, I tried to benchmark the code with Benchmark.Net. But I was surprised to find that some benchmarked code used 40,000 times more memory. After, too much, benchmarking I found that the memory allocation was because of a foreach over a SortedList<int, int>.

using BenchmarkDotNet.Attributes;

namespace NetCollectionsBenchmarks
{
    [MemoryDiagnoser]
    public class CollectionsBenchmarks
    {
        private Dictionary<int, int> DictionaryData = new();
        private SortedList<int, int> SortedListData = new();

        private Dictionary<int, int> DictionaryCheck = new();
        private SortedList<int, int> SortedListCheck = new();

        [GlobalSetup]
        public void Setup()
        {
            for (int x = 0; x < 15; x++)
                this.DictionaryData.Add(x, x);

            this.SortedListData = new SortedList<int, int>(this.DictionaryData);

            this.DictionaryCheck = new Dictionary<int, int>(this.DictionaryData);
            this.SortedListCheck = new SortedList<int, int>(this.DictionaryData);
        }

        [Benchmark(Baseline = true)]
        public long ForLoopDictionaryBenchmark()
        {
            var count = 0L;
            var res = 0L;
            for (int x = 0; x < 1_000_000; x++)
            {
                for (int i = 0; i < 15; i++)
                {
                    if (this.DictionaryCheck.TryGetValue(x, out var value) || value < x)
                        res += value;

                    count++;
                }
            }

            return res;
        }

        [Benchmark]
        public long ForLoopSortedListBenchmark()
        {
            var res = 0L;
            for (int x = 0; x < 1_000_000; x++)
            {
                for (int i = 0; i < 15; i++)
                {
                    if (this.SortedListCheck.TryGetValue(x, out var value) || value < x)
                        res += value;
                }
            }

            return res;
        }

        [Benchmark]
        public long ForeachDictionaryBenchmark()
        {
            var res = 0L;
            for (int x = 0; x < 1_000_000; x++)
            {
                foreach (var needle in this.DictionaryData)
                {
                    if (this.DictionaryCheck.TryGetValue(needle.Key, out var value) || value < needle.Value)
                        res += value;
                }
            }

            return res;
        }

        [Benchmark]
        public long ForeachSortedListBenchmark()
        {
            var res = 0L;
            for (int x = 0; x < 1_000_000; x++)
            {
                foreach (var needle in this.SortedListData)
                {
                    if (this.SortedListCheck.TryGetValue(needle.Key, out var value) || value < needle.Value)
                        res += value;
                }
            }

            return res;
        }

        [Benchmark]
        public long ForeachNoTryGetValueDictionaryBenchmark()
        {
            var res = 0L;
            for (int x = 0; x < 1_000_000; x++)
            {
                foreach (var needle in this.DictionaryData)
                {
                }
            }

            return res;
        }

        [Benchmark]
        public long ForeachNoTryGetValueSortedListBenchmark()
        {
            var res = 0L;
            for (int x = 0; x < 1_000_000; x++)
            {
                foreach (var needle in this.SortedListData)
                {
                }
            }

            return res;
        }
    }
}

The benchmark methods with foreach() over SortedList uses 40,000 times more memory, than the other methods, even when there is no TryGetValue() in the loop.

Why are SortedList so memory expensive when looping it's enumerator?

The benchmarks has been tested in .NET 6.0 and .NET 7.0, with the same result.

emilsteen
  • 496
  • 4
  • 14
  • 2
    That code is wrong anyway. Why is it doing a `TryGetValue` if it's looping on the `KeyValuePair` already ? – Franck Jan 11 '23 at 15:31
  • 1
    @Franck seems to be a different collection. "Search Collection B for Elements from Collection A". Something like that. – Fildor Jan 11 '23 at 15:34
  • 4
    And this is where it would be really helpful to show a [mcve] *in the question* rather than incomplete code in the question and a link to the full code. It also doesn't help that this conflates two operations: iterating over one collection, and performing a lookup in another one. – Jon Skeet Jan 11 '23 at 15:38
  • 1
    I will start by benchmarking `if (this.DictionaryCheck.TryGetValue(needle.Key, out var value) || value < needle.Value)` versus `if (this.SortedListCheck.TryGetValue(needle.Key, out var value) || value < needle.Value)` on a single value. – Drag and Drop Jan 11 '23 at 15:40
  • 1
    Not a duplicate, but a good read: https://stackoverflow.com/questions/1427147/sortedlist-sorteddictionary-and-dictionary?rq=1 – nilsK Jan 11 '23 at 15:40
  • Regarding your remark. So question must be complete and exist outside of maintenance of a git repro. for example many opensource repro that where part of many answer were deleted by creator when a company brought every C# project – Drag and Drop Jan 11 '23 at 15:42
  • I would guess because it instantiates a new DictionaryEntry for each element : https://referencesource.microsoft.com/#mscorlib/system/collections/sortedlist.cs,802 ... oh wait , that's Framework 4.8 ... nevermind – Fildor Jan 11 '23 at 15:42
  • 1
    Doing anything a million times is an easy way to produce impact. You discovered the difference in the enumerator for [SortedList](https://referencesource.microsoft.com/#mscorlib/system/collections/sortedlist.cs,729) vs [Dictionary](https://referencesource.microsoft.com/#mscorlib/system/collections/generic/dictionary.cs,695). Note how the expensive one is a class and the cheap one is a struct. SortedList has a tricky internal structure. Its enumerator's type takes 48 bytes in 64-bit mode, exactly matching your measurement. Well, 3656 bytes left to explain :) – Hans Passant Jan 11 '23 at 17:32
  • 1
    Note how this is almost always cheap gen#0 memory, so nothing to worry about. – Hans Passant Jan 11 '23 at 17:35
  • @DragandDrop The same amount of memory is allocated even if the lines with TryGetValue() are removed. – emilsteen Jan 11 '23 at 19:25
  • @HansPassant I've been looking at the source here: https://source.dot.net/#System.Collections/System/Collections/Generic/SortedList.cs,de670561692e4a20. And there the Enumerator is a struct in both classes, what I can see. – emilsteen Jan 11 '23 at 19:27
  • @HansPassant Interesting thought about the gen#0 memory. Have to look into that. – emilsteen Jan 11 '23 at 19:28
  • @Fildor That was my first thought too. And it still does that in later frameworks too, but it does it in both Dictionary and SortedList. So that does not explain it. – emilsteen Jan 11 '23 at 19:44
  • @Franck It does TryGetValue() on a different SortedList/Dictionary than the one that it is looping! – emilsteen Jan 19 '23 at 11:30

1 Answers1

5

Since no one seemed to know the answer to this, I continued the investigation to try to find the reason.

As Hans Passant pointed out, the Enumerator in SortedList<> in 4.8 was a class. But I've had already looked at that and the Enumerator in .NET 6 has changed to struct, and has been a struct since at least 2016 according to git. But it is still allocated on the heap.

Since the SortedList<> Enumerator has been a struct since 2016, there could be no way that that code is not included in .NET 6 (or .NET 7). But to be sure I created my own copies of Dictionary<> and SortedList<> from the git repository. Still the same result.

When I was about to change the code in the classes to find what generated the difference, I found the diverging code. It was in GetEnumerator() in the two classes.

GetEnumerator() in SortedList<>:

public IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator()

GetEnumerator() in Dictionary<>:

public Enumerator GetEnumerator()

The return type IEnumerator<KeyValuePair<TKey, TValue>> makes the enumerator to be allocated on the heap, because of interface boxing. Changing the return type to Enumerator removed all the extra allocation reported by Benchmark.NET.

And for the second note from Hans Passant, about it being cheap gen#0 memory. That might be so, but in the benchmarking I have done shows the current implementation of GetEnumerator() takes twice as long time as the one with return type to Enumerator. And the benchmarking I've done is quite close to the production code I'm currently running.

emilsteen
  • 496
  • 4
  • 14
  • After opening an issue in github the reply was that, although my investigation is correct, the change I tested would be a breaking change for existing binaries. – emilsteen Jan 24 '23 at 22:25
  • Interesting. A breaking change should be considered at some point if it provides some real value. If that's too "expensive" an alternate would be useful too. – Kit Jan 24 '23 at 22:39
  • Looks like you found the .NET 1.x compatible enumerator, the generic one [is here](https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs,341). Exact same signature. – Hans Passant Jan 24 '23 at 22:42
  • @Kit Can it be a breaking change if it is changed to .NET 8? – emilsteen Jan 24 '23 at 23:03
  • @HansPassant That's the wrong class. And since my code changes, changes the memory allocation, I'm absolutely sure I've found the correct enumerator! – emilsteen Jan 24 '23 at 23:12
  • Right, the allocation is due to the need to box the struct so it can be assigned to a variable of type `IEnumerable<..>`. – Jeremy Lakeman Jan 25 '23 at 00:37
  • @emilsteen I’m merely saying what you found is worth acting on. How that happens is a whole other non-SO discussion. Might want to add the issue link in your answer. – Kit Jan 25 '23 at 01:26
  • @Kit My question got two comments complaining about me adding a link in the question. So there is no way I'm adding a link... – emilsteen Jan 25 '23 at 08:33
  • @HansPassant And as I already said, my findings has been confirmed by Microsoft. – emilsteen Jan 25 '23 at 10:03
  • @HansPassant Your link is not linking to the GetEnumerator() that is used by foreach(). – emilsteen Feb 25 '23 at 19:44