Internal logger calls impact when it is disabled

[luigiberrettini]

Provide a way to disable internal logger calls and message creation when the internal logger / that level of internal logging is disabled.

[304NotModified]

Is this NLog's internal logger? Are there performance issues?

[luigiberrettini]

I would like to try adding extension methods accepting a lambda so that string concatenation and possibly other statements only get executed when the logger is enabled.

[304NotModified]

Recent NLog releases have InternalLogger.Log with parameters, e.g InternalLogger.Debug("hello {0}", foo).

But the most efficient is to first check InternalLogger.IsDebugEnabled.

[JonPaz]

I agree with @luigiberrettini, having a wrapper that handles the level enabled detection and only invokes the supplied action as necessary would be great. Perhaps this could get rolled back into NLog itself?

[304NotModified]

Perhaps this could get rolled back into NLog itself?

sounds good to me. That was also the reason to react here ;)

[JonPaz]

Thanks @304NotModified,

I'll look at raising a ticket on the main NLog project as well (if one doesn't already exist) and if I get time do a branch for Targets.Syslog.

Out of interest (and I haven't yet looked at the NLog backlog), do you know of any plans to roll this type of functionality into the NLog:ILogger interface etc? SLF4J has this type of interface and it is rather useful ;-)

Cheers,

Jon.

[304NotModified]

, do you know of any plans to roll this type of functionality into the NLog:ILogger interface etc?

not for NLog 4 as that will be a breaking change.

[luigiberrettini]

If @JonPaz is going to create a PR for NLog I will then upgrade the NLog dependency in this target.
Why adding methods with lambda parameters to the ILogger interface is a breaking change @304NotModified?

[304NotModified]

Why adding methods with lambda parameters to the ILogger interface is a breaking change @304NotModified?

because customers could have written their own logger class which implements ILogger

But to be clear, do you like to add this to ILogger (Logger class) or the InternalLogger. The first is already supporting lamda's with the delegate LogMessageGenerator, see http://nlog-project.org/documentation/v4.4.0/html/M_NLog_ILogger_Debug.htm

[luigiberrettini]

It is needed for the InternalLogger but I honestly don't know if the InternalLogger implements ILogger or is a standalone class with its own behavior.
Anyway we will sort it out looking at the code thanks!

[304NotModified]

It is needed for the InternalLogger but I honestly don't know if the InternalLogger implements ILogger

It doesn't as ILogger contains a lot of methods and InternalLogger is a static class

[304NotModified]

anyway PR accepted, unit tests are mandatory ;)

[luigiberrettini]

@JonPaz IMHO you only have to overload the Log and Write functions with wrappers that accept a lambda and in their body call it and pass the resulting string to the currently implemented versions of those methods.

[JonPaz]

@304NotModified - I wouldn't miss those out!

[304NotModified]

created a PR already (NLog/NLog#1919), as I will try to release 4.4.2 soon

[JonPaz]

Great stuff @304NotModified , you're faster than I am ;-)

[JonPaz]

@luigiberrettini - Just a quick message to say that we're now using this target in our production system and it is working nicely, thanks for all of your hard work!!! I'll have a look at getting it running with .Net core at some point in the next few weeks.

[snakefoot]

@luigiberrettini I guess the lambda allocation is faster than the string-format, but an even faster alternative is:

InternalLogger.Debug("Successfully sent message '{0}'", logEventMsgSet);

Then there is no allocations at all.

[304NotModified]

@snakefoot it allocates a string? ;)

[snakefoot]

InternalLogger.Debug("Successfully sent message '{0}'", logEventMsgSet);

it allocates a string? ;)

@304NotModified Yes :) but it is a one time interned string-allocation, so it is a one time penalty, while the lambda-capture-local-context-allocation is for every log-event.

[304NotModified]

You're right @snakefoot,

But I have my doubts if that has any impact in performance in practice (the lambda)

[snakefoot]

But I have my doubts if that has any impact in performance in practice (the lambda)

The lambda allocations has a cost, and will put a strain on the GC0-collections, but yes they are cheaper than performing a string.Format(). I was just referring to the PR-title "Internal logger calls should not impact when it is not enabled". And my suggestion would lower the impact even further, so it just becomes a stack-allocation.

[304NotModified]

And my suggestion would lower the impact even further, so it just becomes a stack-allocation.

Yes that's true!

[luigiberrettini]

If that formatting signature is already available I will give it a try, thanks @snakefoot!