Skip to content

Remove closure allocation in SerilogLoggerProvider. #277

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

Merged
merged 2 commits into from
Jun 4, 2025

Conversation

AndreReise
Copy link
Contributor

@AndreReise AndreReise commented Jun 3, 2025

Addresses #276 issue

BEFORE

Method Mean Error StdDev Ratio RatioSD Gen0 Allocated Alloc Ratio
SerilogOnly 151.4 ns 1.79 ns 1.49 ns 1.00 0.01 0.0381 160 B 1.00
SimpleEvent 255.7 ns 3.38 ns 2.83 ns 1.69 0.02 0.0935 392 B 2.45
Template 343.3 ns 2.84 ns 2.52 ns 2.27 0.03 0.1125 472 B 2.95
StringScope 498.8 ns 3.35 ns 2.97 ns 3.29 0.04 0.2346 984 B 6.15
TemplateScope 699.6 ns 13.36 ns 14.30 ns 4.62 0.10 0.2956 1240 B 7.75
TupleScope 467.8 ns 9.17 ns 8.58 ns 3.09 0.06 0.2217 928 B 5.80
DictionaryScope 513.9 ns 9.06 ns 10.07 ns 3.39 0.07 0.2651 1112 B 6.95
Capturing 654.4 ns 12.32 ns 10.92 ns 4.32 0.08 0.1831 768 B 4.80
CapturingScope 1,069.0 ns 20.69 ns 23.83 ns 7.06 0.17 0.4272 1792 B 11.20
LogInformationScoped 1,112.3 ns 20.53 ns 34.86 ns 7.35 0.24 0.4272 1792 B 11.20
LogInformation_WithEventId 737.1 ns 5.70 ns 4.76 ns 4.87 0.05 0.2422 1016 B 6.35

AFTER

Method Mean Error StdDev Ratio RatioSD Gen0 Allocated Alloc Ratio
SerilogOnly 154.0 ns 2.14 ns 2.00 ns 1.00 0.02 0.0381 160 B 1.00
SimpleEvent 252.2 ns 3.21 ns 3.00 ns 1.64 0.03 0.0858 360 B 2.25
Template 323.3 ns 6.21 ns 7.15 ns 2.10 0.05 0.1049 440 B 2.75
StringScope 492.1 ns 4.44 ns 4.36 ns 3.20 0.05 0.2270 952 B 5.95
TemplateScope 697.9 ns 6.90 ns 5.39 ns 4.53 0.07 0.2880 1208 B 7.55
TupleScope 465.4 ns 7.13 ns 6.32 ns 3.02 0.05 0.2136 896 B 5.60
DictionaryScope 507.4 ns 8.25 ns 7.31 ns 3.30 0.06 0.2575 1080 B 6.75
Capturing 652.8 ns 8.08 ns 6.75 ns 4.24 0.07 0.1755 736 B 4.60
CapturingScope 1,076.7 ns 8.84 ns 7.83 ns 6.99 0.10 0.4196 1760 B 11.00
LogInformationScoped 1,083.3 ns 11.13 ns 8.69 ns 7.04 0.10 0.4196 1760 B 11.00
LogInformation_WithEventId 738.5 ns 11.25 ns 10.52 ns 4.80 0.09 0.2346 984 B 6.15

Copy link
Member

@nblumhardt nblumhardt left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for jumping through all the hoops required for this :-)

Since the logic is now spread between the method and the ScopeCollector helper type, what do you think of making it pull a little more weight, by making the ScopeItems property private, and shifting some more logic into it?

I've sketched a bit of what this might look like, but I didn't give much thought to whether Reverse() could be given a more intent-driven name, or whether the Complete() call might be more like bool TryComplete(out ...), etc.

@AndreReise AndreReise requested a review from nblumhardt June 4, 2025 10:59
@nblumhardt nblumhardt merged commit d8c75ec into serilog:dev Jun 4, 2025
1 check passed
@nblumhardt
Copy link
Member

Thanks @AndreReise 👍

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

Successfully merging this pull request may close these issues.

2 participants