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

Caching is important 🐌 #22261

Merged
merged 1 commit into from
Aug 27, 2020
Merged

Caching is important 🐌 #22261

merged 1 commit into from
Aug 27, 2020

Conversation

smitpatel
Copy link
Member

Perf: Cache change detector in state manager CascadeDelete

Resolves #22215

@smitpatel
Copy link
Member Author

Before:

Method AutoDetectChanges Mean Error StdDev Median Gen 0 Gen 1 Gen 2 Allocated
Remove False 14.86 ms 0.399 ms 1.176 ms 14.25 ms 1000.0000 1000.0000 - 7.67 MB
RemoveRange False 14.20 ms 0.460 ms 1.357 ms 13.58 ms - - - 7.06 MB
Update False 60.01 ms 1.130 ms 1.057 ms 59.47 ms 1000.0000 1000.0000 - 7.51 MB
UpdateRange False 58.87 ms 1.173 ms 1.566 ms 58.78 ms - - - 6.9 MB
Remove True 18.16 ms 0.396 ms 1.167 ms 17.71 ms 1000.0000 1000.0000 - 7.67 MB
RemoveRange True 16.91 ms 0.422 ms 1.245 ms 16.67 ms - - - 7.06 MB
Update True 61.55 ms 1.219 ms 1.404 ms 61.47 ms 1000.0000 - - 7.51 MB
UpdateRange True 59.30 ms 1.161 ms 1.192 ms 58.71 ms - - - 6.9 MB

After:

Method AutoDetectChanges Mean Error StdDev Median Gen 0 Gen 1 Gen 2 Allocated
Remove False 14.13 ms 0.433 ms 1.278 ms 13.65 ms 1000.0000 1000.0000 - 7.67 MB
RemoveRange False 13.82 ms 0.470 ms 1.386 ms 13.29 ms - - - 7.06 MB
Update False 56.76 ms 1.126 ms 1.053 ms 56.20 ms 1000.0000 1000.0000 - 7.51 MB
UpdateRange False 57.83 ms 1.152 ms 1.078 ms 57.22 ms - - - 6.9 MB
Remove True 14.76 ms 0.425 ms 1.252 ms 14.86 ms 1000.0000 - - 7.67 MB
RemoveRange True 13.30 ms 0.469 ms 1.383 ms 12.57 ms - - - 7.06 MB
Update True 58.67 ms 1.114 ms 1.192 ms 58.65 ms 1000.0000 - - 7.51 MB
UpdateRange True 57.74 ms 1.077 ms 1.007 ms 57.39 ms - - - 6.9 MB

Copy link
Member

@AndriySvyryd AndriySvyryd left a comment

Choose a reason for hiding this comment

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

Now to fix Attach perf 😉

@AndriySvyryd
Copy link
Member

Caching Model[CoreAnnotationNames.SkipDetectChangesAnnotation] in other places should also give good results, this should probably be moved to a field on Model instead of an annotation

@smitpatel
Copy link
Member Author

Filed #22262 #22263

@smitpatel
Copy link
Member Author

@Pilchie for RC1 approval.

@Pilchie
Copy link
Member

Pilchie commented Aug 27, 2020

Approved for RC1

@smitpatel smitpatel merged commit a57d2e5 into release/5.0 Aug 27, 2020
@smitpatel smitpatel deleted the smit/cache branch August 27, 2020 17:40
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.

Perf regression in existing data variations
3 participants