Skip to content

SortedDictionary Copy optimization #45659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 14, 2020

Conversation

johnthcall
Copy link
Contributor

My team uses SortedDictionary to store properties in our logging flow, when we start a child activity we often make a copy of the parent properties. I noticed that SortedDictionary constructor from IDictionary always iterates and uses Add. SortedSet which SortedDictionary uses already has an optimization for DeepCloning when the comparer used is the same. By leveraging DeepClone when copying a SortedDictionary with the same comparer there are huge performance benefits especially for larger SortedDictionaries.

dotnet/performance@master...johnthcall:sorteddictionarycopy

Method Toolchain N Mean Error StdDev Median Ratio RatioSD Gen 0 Gen 1 Allocated
FullString pr 10 434.1 ns 23.25 ns 25.85 ns 426.3 ns 0.31 0.03 0.2173 - 944 B
FullString upstream 10 1,409.2 ns 107.74 ns 119.75 ns 1,410.3 ns 1.00 0.00 0.1950 - 848 B
FullInt pr 10 368.7 ns 15.68 ns 18.06 ns 367.7 ns 0.44 0.04 0.2003 - 864 B
FullInt upstream 10 844.3 ns 68.29 ns 78.64 ns 836.4 ns 1.00 0.00 0.1770 - 768 B
HalfFullString pr 10 270.0 ns 10.76 ns 12.39 ns 269.4 ns 0.46 0.04 0.1465 - 632 B
HalfFullString upstream 10 595.7 ns 37.21 ns 42.85 ns 600.6 ns 1.00 0.00 0.1278 - 552 B
HalfFullInt pr 10 215.7 ns 6.05 ns 6.97 ns 215.5 ns 0.55 0.04 0.1372 - 592 B
HalfFullInt upstream 10 392.5 ns 21.27 ns 24.49 ns 390.6 ns 1.00 0.00 0.1175 - 512 B
FullString pr 200 7,869.1 ns 283.68 ns 326.69 ns 7,920.3 ns 0.14 0.01 2.7078 - 11712 B
FullString upstream 200 58,191.3 ns 2,479.74 ns 2,855.68 ns 57,775.2 ns 1.00 0.00 2.6042 - 11552 B
FullInt pr 200 6,600.6 ns 431.75 ns 497.21 ns 6,706.8 ns 0.21 0.03 2.3420 0.0473 10112 B
FullInt upstream 200 31,265.0 ns 2,894.29 ns 3,096.86 ns 30,616.9 ns 1.00 0.00 2.2563 - 9952 B
HalfFullString pr 200 3,784.7 ns 110.42 ns 127.16 ns 3,807.7 ns 0.15 0.01 1.4015 - 6080 B
HalfFullString upstream 200 25,528.7 ns 1,373.41 ns 1,469.53 ns 25,096.2 ns 1.00 0.00 1.3474 - 5936 B
HalfFullInt pr 200 3,379.2 ns 217.31 ns 250.26 ns 3,312.0 ns 0.25 0.02 1.2217 - 5280 B
HalfFullInt upstream 200 13,296.2 ns 628.27 ns 723.51 ns 13,181.8 ns 1.00 0.00 1.1547 - 5136 B
FullString pr 3000 122,408.5 ns 4,539.68 ns 5,227.90 ns 122,875.8 ns 0.09 0.01 38.3212 4.1058 168640 B
FullString upstream 3000 1,294,468.8 ns 73,230.03 ns 81,394.98 ns 1,268,383.1 ns 1.00 0.00 36.4583 10.4167 168416 B
FullInt pr 3000 101,911.5 ns 4,066.40 ns 4,682.87 ns 101,908.9 ns 0.17 0.01 32.8608 8.3763 144640 B
FullInt upstream 3000 607,575.1 ns 32,861.07 ns 37,842.85 ns 593,137.6 ns 1.00 0.00 32.4074 6.9444 144416 B
HalfFullString pr 3000 61,323.1 ns 2,732.53 ns 3,146.79 ns 60,708.5 ns 0.10 0.00 19.4288 3.7453 84608 B
HalfFullString upstream 3000 584,769.2 ns 13,853.84 ns 14,823.46 ns 585,738.7 ns 1.00 0.00 17.8571 2.2321 84400 B
HalfFullInt pr 3000 48,482.6 ns 1,483.65 ns 1,649.07 ns 48,519.1 ns 0.17 0.01 16.8051 - 72608 B
HalfFullInt upstream 3000 284,763.9 ns 14,896.46 ns 16,557.37 ns 277,911.5 ns 1.00 0.00 15.8898 2.1186 72400 B

@ghost
Copy link

ghost commented Dec 6, 2020

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

Issue Details

My team uses SortedDictionary to store properties in our logging flow, when we start a child activity we often make a copy of the parent properties. I noticed that SortedDictionary constructor from IDictionary always iterates and uses Add. SortedSet which SortedDictionary uses already has an optimization for DeepCloning when the comparer used is the same. By leveraging DeepClone when copying a SortedDictionary with the same comparer there are huge performance benefits especially for larger SortedDictionaries.

dotnet/performance@master...johnthcall:sorteddictionarycopy

Method Toolchain N Mean Error StdDev Median Ratio RatioSD Gen 0 Gen 1 Allocated
FullString pr 10 434.1 ns 23.25 ns 25.85 ns 426.3 ns 0.31 0.03 0.2173 - 944 B
FullString upstream 10 1,409.2 ns 107.74 ns 119.75 ns 1,410.3 ns 1.00 0.00 0.1950 - 848 B
FullInt pr 10 368.7 ns 15.68 ns 18.06 ns 367.7 ns 0.44 0.04 0.2003 - 864 B
FullInt upstream 10 844.3 ns 68.29 ns 78.64 ns 836.4 ns 1.00 0.00 0.1770 - 768 B
HalfFullString pr 10 270.0 ns 10.76 ns 12.39 ns 269.4 ns 0.46 0.04 0.1465 - 632 B
HalfFullString upstream 10 595.7 ns 37.21 ns 42.85 ns 600.6 ns 1.00 0.00 0.1278 - 552 B
HalfFullInt pr 10 215.7 ns 6.05 ns 6.97 ns 215.5 ns 0.55 0.04 0.1372 - 592 B
HalfFullInt upstream 10 392.5 ns 21.27 ns 24.49 ns 390.6 ns 1.00 0.00 0.1175 - 512 B
FullString pr 200 7,869.1 ns 283.68 ns 326.69 ns 7,920.3 ns 0.14 0.01 2.7078 - 11712 B
FullString upstream 200 58,191.3 ns 2,479.74 ns 2,855.68 ns 57,775.2 ns 1.00 0.00 2.6042 - 11552 B
FullInt pr 200 6,600.6 ns 431.75 ns 497.21 ns 6,706.8 ns 0.21 0.03 2.3420 0.0473 10112 B
FullInt upstream 200 31,265.0 ns 2,894.29 ns 3,096.86 ns 30,616.9 ns 1.00 0.00 2.2563 - 9952 B
HalfFullString pr 200 3,784.7 ns 110.42 ns 127.16 ns 3,807.7 ns 0.15 0.01 1.4015 - 6080 B
HalfFullString upstream 200 25,528.7 ns 1,373.41 ns 1,469.53 ns 25,096.2 ns 1.00 0.00 1.3474 - 5936 B
HalfFullInt pr 200 3,379.2 ns 217.31 ns 250.26 ns 3,312.0 ns 0.25 0.02 1.2217 - 5280 B
HalfFullInt upstream 200 13,296.2 ns 628.27 ns 723.51 ns 13,181.8 ns 1.00 0.00 1.1547 - 5136 B
FullString pr 3000 122,408.5 ns 4,539.68 ns 5,227.90 ns 122,875.8 ns 0.09 0.01 38.3212 4.1058 168640 B
FullString upstream 3000 1,294,468.8 ns 73,230.03 ns 81,394.98 ns 1,268,383.1 ns 1.00 0.00 36.4583 10.4167 168416 B
FullInt pr 3000 101,911.5 ns 4,066.40 ns 4,682.87 ns 101,908.9 ns 0.17 0.01 32.8608 8.3763 144640 B
FullInt upstream 3000 607,575.1 ns 32,861.07 ns 37,842.85 ns 593,137.6 ns 1.00 0.00 32.4074 6.9444 144416 B
HalfFullString pr 3000 61,323.1 ns 2,732.53 ns 3,146.79 ns 60,708.5 ns 0.10 0.00 19.4288 3.7453 84608 B
HalfFullString upstream 3000 584,769.2 ns 13,853.84 ns 14,823.46 ns 585,738.7 ns 1.00 0.00 17.8571 2.2321 84400 B
HalfFullInt pr 3000 48,482.6 ns 1,483.65 ns 1,649.07 ns 48,519.1 ns 0.17 0.01 16.8051 - 72608 B
HalfFullInt upstream 3000 284,763.9 ns 14,896.46 ns 16,557.37 ns 277,911.5 ns 1.00 0.00 15.8898 2.1186 72400 B
Author: johnthcall
Assignees: -
Labels:

area-System.Collections

Milestone: -

…c/SortedDictionary.cs

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@johnthcall johnthcall force-pushed the SortedDictionaryCopy branch from 924991c to d1015ed Compare December 6, 2020 20:57
Assert.Equal(sourceSorted, copied);
Assert.Equal(comparer, copied.Comparer);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have tests for when the collection passed in does not have the same comparer as the one passed in? For both IDictionary<K,V> and SortedDictionary<K,V> ? I don't see them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dictionary classes only have constructors with IEqualityComparer not IComparer. In SortedDictionary are we okay using a inline comparer which return negative of what GetKeyIComparer returns?

Copy link
Member

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

@eiriktsarpalis eiriktsarpalis merged commit b01c8e1 into dotnet:master Dec 14, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 13, 2021
@johnthcall johnthcall deleted the SortedDictionaryCopy branch July 29, 2021 17:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants