Skip to content
This repository was archived by the owner on Dec 5, 2019. It is now read-only.

Conversation

@snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Mar 22, 2019

Fixes performance bug introduced with #200 (Performs reflection on every logevent. Performs reflection on every logger-object-creation).

Updates NLog-test-project dependency to NLog 4.6 (Changed the NLog-test-project to use MDLC + NDLC instead). Handling the breaking change introduced in #181

This PR is not a breaking change.

@snakefoot
Copy link
Contributor Author

@damianh Found that #200 by @jesperll introduced reflection-logic when ever logging an event:

_logEventDelegate = (type, e) => loggerType.GetMethod("Log", new Type[] { typeof(Type), logEventInfoType }).Invoke(logger, new object[] { type, e });

It also performs reflection logic in constructor when ever creating a Logger-object:

internal NLogLogger(object logger)
{
var loggerType = logger.GetType();
_nameDelegate = (NameDelegate)loggerType.GetProperty("Name").GetGetMethod().CreateDelegate(typeof(NameDelegate), logger);

I have now fixed both issues. Removing memory allocation issues and performance hit.

@304NotModified
Copy link
Contributor

Polite bump @damianh

@snakefoot
Copy link
Contributor Author

@damianh Another polite bump. Would be nice to fix the regression introduced with LibLog ver. 5.0.5

@snakefoot
Copy link
Contributor Author

@damianh Yet another ping pong

@snakefoot snakefoot changed the title Update NLog from 4.5.11 to 4.6.0 (MDC to MDLC) Fixes bug where LibLog NLog Provider performs reflections for all logevents Jun 19, 2019
@snakefoot snakefoot changed the title Fixes bug where LibLog NLog Provider performs reflections for all logevents Fixes bug where NLogLogProvider performs reflections for all logevents Jun 19, 2019
@snakefoot snakefoot changed the title Fixes bug where NLogLogProvider performs reflections for all logevents Fixed bug where NLogLogProvider performs reflections for all logevents Jun 20, 2019
@snakefoot
Copy link
Contributor Author

snakefoot commented Jun 30, 2019

@damianh Another polite bump to have this bug-fix merged and released.

@304NotModified
Copy link
Contributor

@damianh please accept this PR. Snakefoot is one of the core contributors of NLog and so you could merge this one with confidence.

If this PR need some changes, please also let us know.

@snakefoot
Copy link
Contributor Author

@damianh How is the calendar looking? Any room available for merging this PR?

@damianh damianh changed the title Fixed bug where NLogLogProvider performs reflections for all logevents Fixed bug where NLogLogProvider performs reflections for all log… Nov 11, 2019
@damianh damianh merged commit 7bdf859 into damianh:master Nov 11, 2019
@damianh
Copy link
Owner

damianh commented Nov 11, 2019

Thanks for PR and patience. Will release this asap.

@snakefoot
Copy link
Contributor Author

Thank you for merging. Looking forward to the final and last release of LibLog.

@snakefoot
Copy link
Contributor Author

Created #271 as a bonus PR to close down a few more issues before version 5.0.7

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants