Skip to content
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

IsSafeToDeferFormatting - Convert.GetTypeCode is faster and better #2595

Merged
merged 1 commit into from
Mar 5, 2018

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Feb 24, 2018

Than GetType().IsPrimitive()

Logger Name Messages Size Args Threads Loggers
NullLogger 50.000.000 64 3 1 1
Test Name Time (ms) Msgs/sec GC2 GC1 GC0 CPU (ms) Alloc (MB)
This PR (32 bit) 16.497 3.030.675 0 0 721 14.359 7.248,2
Master (32 bit) 17.514 2.854.764 0 0 726 16.203 7.248,2
This PR (64 bit) 15.437 3.238.959 0 0 489 14.468 14.114,7
Master (64 bit) 15.923 3.139.958 0 0 481 15.203 14.114,7

@codecov
Copy link

codecov bot commented Feb 24, 2018

Codecov Report

Merging #2595 into master will decrease coverage by <1%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #2595    +/-   ##
=======================================
- Coverage      81%     81%   -<1%     
=======================================
  Files         325     325            
  Lines       23918   23915     -3     
  Branches     3022    3021     -1     
=======================================
- Hits        19408   19394    -14     
- Misses       3693    3708    +15     
+ Partials      817     813     -4

@304NotModified
Copy link
Member

Are we 100% sure it's the same? Don't expect a performance improvement then.

@snakefoot
Copy link
Contributor Author

snakefoot commented Feb 25, 2018

Are we 100% sure it's the same? Don't expect a performance improvement then.

No it is not the same, since it is faster and better (Recognizes DateTime, Enum and Decimal as immutable). Though it will have a side-effect incase some has created custom type that inherits from IConvertible. But only "primitive struct types" should do that.

This the source code of Convert.GetTypeCode:

        public static TypeCode GetTypeCode(object value) {
            if (value == null) return TypeCode.Empty;
            IConvertible temp = value as IConvertible;
            if (temp != null)
            {
                return temp.GetTypeCode();
            }
            return TypeCode.Object;
        }

There is a lot of internal compiler optimization when it comes to GetType() and IsPrimitive(), but at the end it finds an internal typecode, and does a bitwise operation to check if one of the simple types specified here:

https://msdn.microsoft.com/en-us/library/system.type.isprimitive(v=vs.110).aspx

@304NotModified 304NotModified added this to the 4.5 rc7 milestone Mar 5, 2018
@304NotModified 304NotModified merged commit 857edbd into NLog:master Mar 5, 2018
@304NotModified
Copy link
Member

304NotModified commented Mar 5, 2018

thanks!

@snakefoot snakefoot deleted the LogEventInfoFastDeferCheck branch April 4, 2020 17:35
@snakefoot snakefoot modified the milestones: 4.5 rc7, 4.5 Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants