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

[release/5.0-rc2] Optimize the allocation and speed of ActivitySet/GetCustomProperty #41908

Merged
merged 1 commit into from
Sep 8, 2020

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 5, 2020

Backport of #41840 to release/5.0-rc2

#41840 contains the perf numbers before and after the fix.

/cc @tarekgh

Customer Impact

In 5.0 We have exposed the APIs Get/SetCutomProperty in Activity class. It has been reported by Azure monitoring team (and other teams) this API is allocating too much memory when calling the setter. These APIs are used in the Open Telemetry implementation which deals with many activity objects and set a custom property values to such objects.

The fix here is reduce the allocation and enhance the speed of these APIs.

Testing

Manually tested and ran the whole regression testing on the fix.

Risk

very low as we are not changing any logic of the APIs. We are just switched the implementation to use Dictionary<,> instead of ConcurrentDictionary<,>.

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@tarekgh tarekgh added area-System.Diagnostics.Tracing Servicing-consider Issue for next servicing release review labels Sep 5, 2020
@ghost
Copy link

ghost commented Sep 5, 2020

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

@danmoseley
Copy link
Member

Because the installer tests didn't run at all, I clicked rerun failed jobs.

@danmoseley
Copy link
Member

@tarekgh you can merge when ready..

@tarekgh tarekgh merged commit 96f6793 into release/5.0-rc2 Sep 8, 2020
@tarekgh tarekgh deleted the backport/pr-41840-to-release/5.0-rc2 branch September 8, 2020 19:24
@karelz karelz modified the milestones: 5.0.0 rc2, 5.0.0 Sep 9, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants