-
Notifications
You must be signed in to change notification settings - Fork 160
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
Perf-improvement: CreateEtag method #1081
Perf-improvement: CreateEtag method #1081
Conversation
src/Microsoft.AspNetCore.OData/Edm/EdmModelAnnotationExtensions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Edm/EdmModelAnnotationExtensions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Edm/EdmModelAnnotationExtensions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Formatter/Serialization/ODataResourceSerializer.cs
Show resolved
Hide resolved
src/Microsoft.AspNetCore.OData/Edm/EdmModelAnnotationExtensions.cs
Outdated
Show resolved
Hide resolved
@ElizabethOkerio In your pull request description you state:
Where is the change for the other cache? |
@gathogojr I decided to leave it out since there is on-going work to improve the VersioningDictionary from the Edm lib. |
926a6bd
to
d30c596
Compare
@ElizabethOkerio could you share the CPU usage for the new changes. |
The CreateEtag method creates an ETag for a given entity. This method is called for every resource and seems to take-up a lot of CPU time even in cases where ETags are not being created.
This is how the CreateEtag method looks like:
We call
return resourceContext.Request.CreateETag(properties, resourceContext.TimeZone)
every time even when the properties are null. This call involves getting the ETag handler from the DI container which seems to take up some CPU time.. Adding a check to only call this method when the concurrency properties are not null seems to improve CPU usage a bit.Comparing the before and after CPU usage, you can see an improvement in CPU usage of about
1.37%