Skip to content
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

improve EntitySet.Add, Contains performance #126

Merged
merged 1 commit into from
May 28, 2018
Merged

improve EntitySet.Add, Contains performance #126

merged 1 commit into from
May 28, 2018

Conversation

uozuAho
Copy link
Contributor

@uozuAho uozuAho commented May 16, 2018

Much simpler change than #105 , and achieves similar results, except for Remove & Detach.

  • add internal HashSet for fast Contains()
  • use _list.RemoveAt when index is known

Note that 2 extra tests started failing for me on this branch and on master: EntitySet & EntityCollection ICVF_MemoryLeakTest. Not sure if my environment's changed, I'll have a bit more of a look at this later.

I ran benchmarks for master and this branch. Below are the percentage differences between this branch and master. I'm not sure why the error/stddev is so high, my PC may have been doing updates...

EDIT: the errors and stddev figures are also percentage differences from master, so don't make too much sense when viewed here (eg. negative stddev :P)

EntitySetBenchmarks

Method NumEntities Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
None 10 -9.70 -24.76 -24.76 0.00 #VALUE! #VALUE! 0.05
Add 10 -21.73 -62.42 -62.42 0.00 #VALUE! #VALUE! 0.56
Attach 10 -1.79 -23.34 -23.35 0.00 #VALUE! #VALUE! 0.46
AddAndModify 10 -8.31 10.69 10.69 1.03 #VALUE! #VALUE! 0.36
AddAndRemove 10 -14.07 -85.39 -85.39 1.56 #VALUE! #VALUE! 0.56
AddAndDetach 10 -15.72 944.88 944.62 1.28 #VALUE! #VALUE! 0.46
AttachAndModify 10 -11.59 -16.24 -16.24 0.00 #VALUE! #VALUE! 0.32
AttachAndRemove 10 -12.44 18.27 18.28 1.23 #VALUE! #VALUE! 0.44
AttachAndDetach 10 2.14 -33.05 -33.05 0.00 #VALUE! #VALUE! 0.46
None 500 -8.83 -55.31 -55.31 0.00 -11.11 -33.33 0.00
Add 500 -8.89 -73.03 -73.03 0.00 0.00 0.00 0.57
Attach 500 -6.41 23.64 23.64 -3.13 0.00 0.00 0.48
AddAndModify 500 -10.26 71.53 71.53 0.00 40.00 100.00 0.36
AddAndRemove 500 -14.96 -14.40 -14.40 0.00 -9.09 50.00 0.55
AddAndDetach 500 -14.70 -94.15 -94.15 -3.13 0.00 #VALUE! 0.46
AttachAndModify 500 -1.42 420.88 420.88 0.00 20.00 0.00 0.33
AttachAndRemove 500 -8.56 -31.30 -31.30 0.00 -14.29 0.00 0.44
AttachAndDetach 500 -2.69 1,793.98 1,793.98 0.00 0.00 #VALUE! 0.47
None 5000 -5.94 77.70 77.70 0.00 0.00 0.00 0.00
Add 5000 -42.44 -77.10 -77.10 0.74 6.90 8.33 0.55
Attach 5000 -2.95 -98.03 -98.03 1.25 3.23 0.00 0.45
AddAndModify 5000 -34.75 520.21 520.21 0.00 -1.61 0.00 0.35
AddAndRemove 5000 -45.08 -62.66 -62.66 0.00 0.00 0.00 0.52
AddAndDetach 5000 -40.97 325.89 325.89 0.00 13.64 5.26 0.43
AttachAndModify 5000 -9.04 349.73 349.73 -2.56 -8.93 -13.64 0.30
AttachAndRemove 5000 -8.74 -19.17 -19.17 -0.59 0.00 -6.67 0.41
AttachAndDetach 5000 -10.99 -1.76 -1.76 1.23 10.00 7.69 0.44

Load Benchmarks

Method Job Jit Platform NumEntities Mean Error StdDev Median Gen 0 Gen 1 Gen 2 Allocated
NoOp LegacyJitX86 LegacyJit X86 10 -13.59 -60.52 -0.51 -9.27 0.00 #VALUE! #VALUE! 0.00
LoadEntities LegacyJitX86 LegacyJit X86 10 -11.69 -12.59 -33.37 -11.49 1.25 #VALUE! #VALUE! 1.09
LoadAndMergeEntities LegacyJitX86 LegacyJit X86 10 -8.24 -6.62 10.58 -3.94 1.08 #VALUE! #VALUE! 0.96
LoadAndRefreshEntities LegacyJitX86 LegacyJit X86 10 -17.93 -12.81 109.25 -18.05 1.08 #VALUE! #VALUE! 0.96
NoOp RyuJitX64 RyuJit X64 10 1.92 330.32 405.92 1.16 0.00 #VALUE! #VALUE! 0.00
LoadEntities RyuJitX64 RyuJit X64 10 5.31 121.77 404.75 6.00 0.00 #VALUE! #VALUE! -0.09
LoadAndMergeEntities RyuJitX64 RyuJit X64 10 -1.86 -2.94 -24.90 -2.38 0.60 #VALUE! #VALUE! 0.74
LoadAndRefreshEntities RyuJitX64 RyuJit X64 10 -1.37 -4.02 -25.44 -3.33 0.60 #VALUE! #VALUE! 0.74
NoOp LegacyJitX86 LegacyJit X86 100 -16.57 -17.33 -33.40 -14.32 0.00 #VALUE! #VALUE! 0.00
LoadEntities LegacyJitX86 LegacyJit X86 100 -8.40 -6.45 27.27 -10.23 1.06 #VALUE! #VALUE! 1.03
LoadAndMergeEntities LegacyJitX86 LegacyJit X86 100 -15.09 -16.82 2.33 -16.43 0.94 #VALUE! #VALUE! 0.91
LoadAndRefreshEntities LegacyJitX86 LegacyJit X86 100 -16.20 -15.56 -12.73 -18.69 0.94 #VALUE! #VALUE! 0.90
NoOp RyuJitX64 RyuJit X64 100 0.67 -15.72 -64.42 0.59 0.00 #VALUE! #VALUE! 0.00
LoadEntities RyuJitX64 RyuJit X64 100 -3.14 -2.51 -3.50 -5.23 0.61 -50.00 #VALUE! 0.74
LoadAndMergeEntities RyuJitX64 RyuJit X64 100 -1.26 -0.99 1.76 -1.43 1.15 0.00 #VALUE! 0.69
LoadAndRefreshEntities RyuJitX64 RyuJit X64 100 -0.92 -1.22 15.44 -1.89 1.15 0.00 #VALUE! 0.69
NoOp LegacyJitX86 LegacyJit X86 1000 -18.82 -20.63 -55.71 -20.87 -7.41 0.00 #VALUE! 0.00
LoadEntities LegacyJitX86 LegacyJit X86 1000 -13.43 -14.31 -42.03 -12.81 9.09 20.00 #VALUE! 1.02
LoadAndMergeEntities LegacyJitX86 LegacyJit X86 1000 -19.94 -87.62 -95.33 -21.01 16.67 100.00 #VALUE! 0.91
LoadAndRefreshEntities LegacyJitX86 LegacyJit X86 1000 -20.92 70.97 -30.95 -23.41 16.67 100.00 #VALUE! 0.91
NoOp RyuJitX64 RyuJit X64 1000 3.57 3.03 9.65 5.42 0.00 0.00 0.00 0.00
LoadEntities RyuJitX64 RyuJit X64 1000 2.10 -29.82 41.06 3.71 -14.71 -25.00 50.00 0.73
LoadAndMergeEntities RyuJitX64 RyuJit X64 1000 -2.00 -7.90 -64.96 -0.31 -13.16 -27.27 -33.33 0.65
LoadAndRefreshEntities RyuJitX64 RyuJit X64 1000 0.87 0.86 10.86 0.67 -13.16 -27.27 -33.33 0.65

@Daniel-Svensson
Copy link
Member

I've ran the benchmarks against this branch and master and so far it looks good.
My standard errors looks a lot better, maybe you are not running you computer in "performance energy mode" or it throttles due to termal limitations?

I'll be away most of the coming week but will try to get some time to look at the code, but it might take an additional week. So far it does look promising with such few changes required 👍

@Daniel-Svensson
Copy link
Member

I've reviewed the code and everything looks fine. I just want to add a comment to the protected list property that it should never be used to modify the list (since the set will come out of sync and some other functionality won't work).

Either you add the comment and I'll merge it directly or I will try to merge it and make a release next weekend

@Daniel-Svensson
Copy link
Member

You can disc´regard from my last comment, I had forgotten that EntitySet is sealed so it is not that important to comment the protected property

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants