Reduce event source logger allocations #870

pakrym · 2018-12-26T21:02:57Z

Use EventData
No-op more
Split event id and event name so we don't ToString it all the time

davidfowl · 2018-12-26T22:35:32Z

src/Logging/Logging.EventSource/src/EventSourceLogger.cs

-            var arguments = new List<KeyValuePair<string, string>>();
-            var asKeyValues = state as IEnumerable<KeyValuePair<string, object>>;
-            if (asKeyValues != null)
+            if (state is IEnumerable<KeyValuePair<string, object>> keyValuePairs)
            {


Is there no way to write the object directly as EventData before turning it into a list of string, string?

I suspect there is but it's not easy. I didn't touch methods that take IEnumerable for the first pass.

Ok we need to do that though. I’m ok looking into that next. I think it’s worrhwhile to do.

davidfowl · 2018-12-26T22:59:09Z

Do a before and after for allocations? I’d like to see what this saves. Maybe we should also add a micro benchmark for it.

pakrym · 2018-12-27T01:20:36Z

Before:

            Method | HasSubscribers |  Json |        Mean |        Error |       StdDev |        Op/s |  Gen 0 | Allocated |
------------------ |--------------- |------ |------------:|-------------:|-------------:|------------:|-------:|----------:|
 EventSourceLogger |          False | False |    667.5 ns |    12.749 ns |    12.521 ns | 1,498,047.7 | 0.0029 |     752 B |
 EventSourceLogger |          False |  True |    646.7 ns |     3.370 ns |     2.631 ns | 1,546,216.1 | 0.0029 |     752 B |
 EventSourceLogger |           True | False | 74,842.2 ns | 1,475.193 ns | 2,697.473 ns |    13,361.5 |      - |    3464 B |
 EventSourceLogger |           True |  True |  3,512.6 ns |    69.683 ns |    71.560 ns |   284,687.0 | 0.0191 |    4384 B |

After:


            Method | HasSubscribers |  Json |        Mean |        Error |       StdDev |        Op/s |  Gen 0 | Allocated |
------------------ |--------------- |------ |------------:|-------------:|-------------:|------------:|-------:|----------:|
 EventSourceLogger |          False | False |    199.4 ns |     4.010 ns |     8.886 ns | 5,014,173.1 |      - |      24 B |
 EventSourceLogger |          False |  True |    195.9 ns |     4.114 ns |     5.900 ns | 5,105,384.5 |      - |      24 B |
 EventSourceLogger |           True | False | 71,330.0 ns | 1,405.246 ns | 2,707.432 ns |    14,019.3 |      - |    2640 B |
 EventSourceLogger |           True |  True |  4,879.7 ns |    80.149 ns |    74.971 ns |   204,928.6 | 0.0153 |    3976 B |

src/Logging/Logging.EventSource/src/LoggingEventSource.cs

davidfowl

Great improvement in memory (esp when nobody is listening).

cc @vancem We should work on the EventSource allocations here to make sure it's as cheap as it can be (even when there are listeners).

Commit migrated from dotnet/extensions@091ee13

* Reorganize source code In preparation for merging many other projects into this repo, this establishes a new source code organization which groups projects together based on subject matter. Commit migrated from dotnet/extensions@7ce647c * Merge branch 'release/2.1' into release/2.2 Commit migrated from dotnet/extensions@18fcffb * Merge branch 'release/2.2' Commit migrated from dotnet/extensions@34204b6 * Reorganize source code in preparation to move into aspnet/Extensions Prior to reorganization, this source code was found in https://github.com/aspnet/DependencyInjection/tree/7a283947c231b6585c8ac95e653950b660f3da96 Commit migrated from dotnet/extensions@689c4a8 * Reorganize source code in preparation to move into aspnet/Extensions Prior to reorganization, this source code was found in https://github.com/aspnet/DependencyInjection/tree/12eef5e86e965b9611221c72c169002e6f3038ee Commit migrated from dotnet/extensions@04e957b * Reorganize source code in preparation to move into aspnet/Extensions Prior to reorganization, this source code was found in https://github.com/aspnet/DependencyInjection/tree/d77b090567a1e6ad9a5bb5fd05f4bdcf281d4185 Commit migrated from dotnet/extensions@9a4c61b * Reorganize source code in preparation to move into aspnet/Extensions Prior to reorganization, this source code was found in https://github.com/aspnet/Logging/tree/f7d8e4e0537eaab54dcf28c2b148b82688a3d62d Commit migrated from dotnet/extensions@f3cd14a * Reorganize source code in preparation to move into aspnet/Extensions Prior to reorganization, this source code was found in https://github.com/aspnet/HttpClientFactory/tree/cf7cf83ee869ed50a41852f5e880d073386b7fe7 Commit migrated from dotnet/extensions@c38f9c1 * Use the SharedSourceRoot variable in project files Commit migrated from dotnet/extensions@888bcba * Allow caching of IEnumerable services (dotnet/extensions#575) Commit migrated from dotnet/extensions@c89a5b5 * Reduce event source logger allocations (dotnet/extensions#870) Commit migrated from dotnet/extensions@091ee13 * Use Arcade (dotnet/extensions#586) Use arcade Commit migrated from dotnet/extensions@f045899 * Cleanup conversion to Arcade (dotnet/extensions#1014) * Remove obsolete targets, properties, and scripts * Replace IsProductComponent with IsShipping * Undo bad merge to version.props * Update documentation, and put workarounds into a common file * Replace usages of RepositoryRoot with RepoRoot * Remove API baselines * Remove unnecessary restore feeds and split workarounds into two files * Enable PR checks on all branches, and disable autocancel Commit migrated from dotnet/extensions@f41cfde * Shrink StringValues (dotnet/extensions#1283) Commit migrated from dotnet/extensions@2f8e1fb * Disable transitive project references in test projects (dotnet/extensions#1834) Commit migrated from dotnet/extensions@e7d5bea * Fix HTTP client benchmarks Commit migrated from dotnet/extensions@8f0f7fa * Add typed client creation benchmark Commit migrated from dotnet/extensions@20ce03a * Support netcoreapp3.1 TFM (dotnet/extensions#2336) * Support netcoreapp3.1 TFM * Unpin SDK for source build * Update to preview1 branding Commit migrated from dotnet/extensions@32cc816 * Normalize all file headers to the expected Apache 2.0 license Commit migrated from dotnet/extensions@cec6e75 * Switch file headers to the MIT license Commit migrated from dotnet/extensions@321a30c * Moves Microsoft.Extensions.* perf tests over * Using the Open key to fully sign assembly - Needed to later get InternalsVisibleTo on DI * Apply suggestions from code review Make batch commit using suggestions in PR feedback Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com> * Fix compile + other feedback * Apply suggestions from code review Co-authored-by: Nate McMaster <nate.mcmaster@microsoft.com> Co-authored-by: Pavel Krymets <pavel@krymets.com> Co-authored-by: Ryan Brandenburg <rybrande@microsoft.com> Co-authored-by: Nate McMaster <natemcmaster@users.noreply.github.com> Co-authored-by: Ben Adams <thundercat@illyriad.co.uk> Co-authored-by: Ryan Nowak <rynowak@microsoft.com> Co-authored-by: John Luo <johluo@microsoft.com> Co-authored-by: Sam Harwell <Sam.Harwell@microsoft.com> Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>

Reduce event source logger allocations

1e77f95

pakrym requested a review from davidfowl December 26, 2018 21:02

Test

eef5595

davidfowl reviewed Dec 26, 2018

View reviewed changes

pakrym added 3 commits December 26, 2018 15:32

Perf bench

69262d2

Test itself

b2a6554

more

cdfa3e4

davidfowl reviewed Dec 27, 2018

View reviewed changes

src/Logging/Logging.EventSource/src/LoggingEventSource.cs Outdated Show resolved Hide resolved

davidfowl approved these changes Dec 27, 2018

View reviewed changes

Feedback

61723f0

pakrym merged commit 091ee13 into master Dec 27, 2018

natemcmaster deleted the pakrym/reduce-event-source-logger-alloc branch December 31, 2018 17:35

maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Feb 28, 2020

Reduce event source logger allocations (dotnet/extensions#870)

ad69bae

Commit migrated from dotnet/extensions@091ee13

maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 2, 2020

Reduce event source logger allocations (dotnet/extensions#870)

e853a81

Commit migrated from dotnet/extensions@091ee13

maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 11, 2020

Reduce event source logger allocations (dotnet/extensions#870)

ab0fd98

Commit migrated from dotnet/extensions@091ee13

wiktork mentioned this pull request Mar 11, 2020

EventSourceLogger does not propagate state data during ActivityStart dotnet/runtime#33486

Closed

maryamariyan pushed a commit to maryamariyan/runtime that referenced this pull request Mar 27, 2020

Reduce event source logger allocations (dotnet/extensions#870)

a0bed62

Commit migrated from dotnet/extensions@091ee13

maryamariyan pushed a commit to maryamariyan/performance that referenced this pull request Apr 3, 2020

Reduce event source logger allocations (dotnet/extensions#870)

c7451bd

Commit migrated from dotnet/extensions@091ee13

ghost locked as resolved and limited conversation to collaborators May 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Reduce event source logger allocations #870

Reduce event source logger allocations #870

Uh oh!

pakrym commented Dec 26, 2018 •

edited

Loading

Uh oh!

davidfowl Dec 26, 2018

Uh oh!

pakrym Dec 26, 2018

Uh oh!

davidfowl Dec 26, 2018

Uh oh!

davidfowl commented Dec 26, 2018

Uh oh!

pakrym commented Dec 27, 2018

Uh oh!

Uh oh!

davidfowl left a comment

Uh oh!

Uh oh!

Reduce event source logger allocations #870

Reduce event source logger allocations #870

Uh oh!

Conversation

pakrym commented Dec 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidfowl Dec 26, 2018

Choose a reason for hiding this comment

Uh oh!

pakrym Dec 26, 2018

Choose a reason for hiding this comment

Uh oh!

davidfowl Dec 26, 2018

Choose a reason for hiding this comment

Uh oh!

davidfowl commented Dec 26, 2018

Uh oh!

pakrym commented Dec 27, 2018

Uh oh!

Uh oh!

davidfowl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pakrym commented Dec 26, 2018 •

edited

Loading