Prevent allocation of Enumerator in CaptureMessageProperties

[agehrke]

ILogger extensions defined in Microsoft.Extensions.Logging.LoggerExtensions and LoggerMessage.Define() uses a state object that implements IReadOnlyList<KeyValuePair<string, object>>. By checking for this and using a for loop we can prevent allocation of an Enumerator.

[codecov]

Codecov Report

Merging #226 into master will decrease coverage by 0.81%.
The diff coverage is 14.28%.

@@            Coverage Diff             @@
##           master     #226      +/-   ##
==========================================
- Coverage   72.15%   71.33%   -0.82%     
==========================================
  Files           6        6              
  Lines         316      321       +5     
  Branches       75       78       +3     
==========================================
+ Hits          228      229       +1     
- Misses         59       63       +4     
  Partials       29       29

Impacted Files	Coverage Δ
src/NLog.Extensions.Logging/Logging/NLogLogger.cs	72.98% <14.28%> (-2.16%)	⬇️
...g.Extensions.Logging/Logging/NLogLoggerProvider.cs	94.28% <0%> (+2.85%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d0d78f...379e24d. Read the comment docs.

[snakefoot]

The code you are trying to optimize is only exercised when the initial parsing fails:

NLog.Extensions.Logging/src/NLog.Extensions.Logging/Logging/NLogLogger.cs

Line 40 in 3d0d78f

var messageParameters = NLogMessageParameterList.TryParse(_options.CaptureMessageTemplates ? state as IReadOnlyList<KeyValuePair<string, object>> : null);

The initial parsing already attempts casting to IReadOnlyList. If something is broken in the initial parsing so it allocated enumerator, then please tell.

[agehrke]

Yes, but isn't that only executed if I enable CaptureMessageTemplate? If I don't want NLog to parse my message template, but still wants to log the state to event properties, I enable CaptureMessageProperties.

[snakefoot]

CaptureMessageTemplate causes NLog to filter away the special event-property injected by Microsoft-Extension-Logging {OriginalFormat} and also injects the event-properties into NLog using a memory-optimized-collection-with-stable-ordering (instead of allocating a Dictionary).

Any reason why you want this special event-property, and allocating a full Dictionary ?

[agehrke]

Thanks for taking the time to look into the PR. I'm closing it and will have a look at CaptureMessageTemplate.

[snakefoot]

@agehrke I have reconsidered your proposal. And after seeing the effect of NLog parsing BeginScope, then I think it is a good idea to be able to skip all the parsing overhead.

Have created #232 where CaptureMessageTemplate = false and IncludeScopes = false will improve the logging performance (While keeping CaptureMessageProperties = true)

[agehrke]

Cool, thanks for letting me know. My use case is exactly what you mention in PR :)