Skip to content

PropertyValueConverter improvements #981

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 6, 2017
Merged

PropertyValueConverter improvements #981

merged 1 commit into from
Jun 6, 2017

Conversation

skomis-mm
Copy link
Contributor

  • changed linq to custom iterators to save on allocations (1 closure object + 1 iterator if I count correctly)
  • avoid allocating DepthLimiter on each create with moving state to thread static field.
  • improved performance on logging dictionaries (get rid of reflection using non-generics)
  • unreachable code detected...NullableScalarConversionPolicy. From the remarks:

When a nullable type is boxed, the common language runtime automatically boxes the underlying value of the Nullable object, not the Nullable object itself.

nits:

  • re-enabled cross-AppDomains test

.NET Core 1.1 (x64, by default)

before:

BenchmarkDotNet=v0.10.6, OS=Windows 10 Redstone 1 (10.0.14393)
Processor=Intel Core i7-4790 CPU 3.60GHz (Haswell), ProcessorCount=8
Frequency=3507500 Hz, Resolution=285.1033 ns, Timer=TSC
dotnet cli version=2.0.0-preview1-005977
  [Host]     : .NET Core 4.6.25211.01, 64bit RyuJIT
  DefaultJob : .NET Core 4.6.25211.01, 64bit RyuJIT


               Method |          Mean |      Error |     StdDev |   Scaled | ScaledSD |  Gen 0 | Allocated |
--------------------- |--------------:|-----------:|-----------:|---------:|---------:|-------:|----------:|
             LogEmpty |      9.427 ns |  0.0465 ns |  0.0435 ns |     1.00 |     0.00 |      - |       0 B |
 LogEmptyWithEnricher |    106.247 ns |  0.3800 ns |  0.3554 ns |    11.27 |     0.06 | 0.0209 |      88 B |
            LogScalar |    424.611 ns |  0.4639 ns |  0.4339 ns |    45.04 |     0.21 | 0.1101 |     464 B |
        LogDictionary | 10,273.550 ns | 50.4577 ns | 47.1981 ns | 1,089.86 |     6.86 | 1.0071 |    4232 B |
          LogSequence |  1,465.570 ns |  3.2861 ns |  3.0738 ns |   155.47 |     0.76 | 0.2422 |    1024 B |
         LogAnonymous |  7,175.774 ns | 16.0528 ns | 15.0158 ns |   761.24 |     3.73 | 0.9613 |    4040 B |

after:

BenchmarkDotNet=v0.10.6, OS=Windows 10 Redstone 1 (10.0.14393)
Processor=Intel Core i7-4790 CPU 3.60GHz (Haswell), ProcessorCount=8
Frequency=3507500 Hz, Resolution=285.1033 ns, Timer=TSC
dotnet cli version=2.0.0-preview1-005977
  [Host]     : .NET Core 4.6.25211.01, 64bit RyuJIT
  DefaultJob : .NET Core 4.6.25211.01, 64bit RyuJIT


               Method |         Mean |      Error |     StdDev | Scaled | ScaledSD |  Gen 0 | Allocated |
--------------------- |-------------:|-----------:|-----------:|-------:|---------:|-------:|----------:|
             LogEmpty |     8.652 ns |  0.0230 ns |  0.0215 ns |   1.00 |     0.00 |      - |       0 B |
 LogEmptyWithEnricher |   104.790 ns |  0.4970 ns |  0.4405 ns |  12.11 |     0.06 | 0.0132 |      56 B |
            LogScalar |   432.424 ns |  0.6263 ns |  0.5858 ns |  49.98 |     0.14 | 0.1030 |     432 B |
        LogDictionary | 3,887.068 ns |  4.4649 ns |  3.7284 ns | 449.26 |     1.16 | 0.5417 |    2296 B |
          LogSequence | 1,428.896 ns |  3.6324 ns |  3.2200 ns | 165.15 |     0.53 | 0.2079 |     880 B |
         LogAnonymous | 6,694.431 ns | 22.4848 ns | 21.0323 ns | 773.73 |     3.00 | 0.8392 |    3528 B |

.NET 4.6(7) (x86, by default)

before:

BenchmarkDotNet=v0.10.6, OS=Windows 10 Redstone 1 (10.0.14393)
Processor=Intel Core i7-4790 CPU 3.60GHz (Haswell), ProcessorCount=8
Frequency=3507500 Hz, Resolution=285.1033 ns, Timer=TSC
  [Host]     : Clr 4.0.30319.42000, 32bit LegacyJIT-v4.7.2053.0
  DefaultJob : Clr 4.0.30319.42000, 32bit LegacyJIT-v4.7.2053.0


               Method |          Mean |      Error |     StdDev |   Scaled | ScaledSD |  Gen 0 | Allocated |
--------------------- |--------------:|-----------:|-----------:|---------:|---------:|-------:|----------:|
             LogEmpty |      9.425 ns |  0.0199 ns |  0.0186 ns |     1.00 |     0.00 |      - |       0 B |
 LogEmptyWithEnricher |     92.270 ns |  0.3120 ns |  0.2919 ns |     9.79 |     0.04 | 0.0113 |      48 B |
            LogScalar |    470.100 ns |  0.6414 ns |  0.5356 ns |    49.88 |     0.11 | 0.0634 |     268 B |
        LogDictionary | 11,379.083 ns | 33.7546 ns | 31.5741 ns | 1,207.32 |     3.97 | 0.5646 |    2392 B |
          LogSequence |  1,565.883 ns |  3.7999 ns |  3.5544 ns |   166.14 |     0.48 | 0.1602 |     680 B |
         LogAnonymous |  6,570.627 ns | 26.4053 ns | 24.6995 ns |   697.15 |     2.86 | 0.5341 |    2256 B |

after:

BenchmarkDotNet=v0.10.6, OS=Windows 10 Redstone 1 (10.0.14393)
Processor=Intel Core i7-4790 CPU 3.60GHz (Haswell), ProcessorCount=8
Frequency=3507500 Hz, Resolution=285.1033 ns, Timer=TSC
  [Host]     : Clr 4.0.30319.42000, 32bit LegacyJIT-v4.7.2053.0
  DefaultJob : Clr 4.0.30319.42000, 32bit LegacyJIT-v4.7.2053.0


               Method |         Mean |      Error |     StdDev | Scaled | ScaledSD |  Gen 0 | Allocated |
--------------------- |-------------:|-----------:|-----------:|-------:|---------:|-------:|----------:|
             LogEmpty |     9.749 ns |  0.0346 ns |  0.0324 ns |   1.00 |     0.00 |      - |       0 B |
 LogEmptyWithEnricher |   103.460 ns |  0.1742 ns |  0.1629 ns |  10.61 |     0.04 | 0.0066 |      28 B |
            LogScalar |   478.723 ns |  0.6996 ns |  0.6201 ns |  49.11 |     0.17 | 0.0591 |     248 B |
        LogDictionary | 3,867.137 ns | 13.5751 ns | 12.6982 ns | 396.67 |     1.79 | 0.3128 |    1324 B |
          LogSequence | 1,309.241 ns |  1.4345 ns |  1.3418 ns | 134.30 |     0.45 | 0.1144 |     484 B |
         LogAnonymous | 6,128.421 ns | 11.3529 ns | 10.6195 ns | 628.62 |     2.28 | 0.4654 |    1960 B |

nblumhardt
nblumhardt reviewed Jun 6, 2017
View reviewed changes
test/Serilog.Tests/Context/LogContextTests.cs
};
var evidence = AppDomain.CurrentDomain.Evidence;
domain = AppDomain.CreateDomain("LogContextTest", evidence, domaininfo);
domain = AppDomain.CreateDomain("LogContextTests", null, AppDomain.CurrentDomain.SetupInformation);
Copy link
Member

Choose a reason for hiding this comment

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

👏

nblumhardt
nblumhardt reviewed Jun 6, 2017
View reviewed changes
test/Serilog.Tests/LoggerConfigurationTests.cs

Assert.Contains("C", xs);
Assert.DoesNotContain(xs, "D");
Assert.DoesNotContain("D", xs);
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch!

@nblumhardt
Copy link
Member

Great that the memory diagnoser is working now; I'll give that a shot on #977 👍

LGTM, merging.

@merbla it looks like the new perf test build on AppVeyor has clobbered the regular CI build... Should we just disable it for now? Cheers!

@nblumhardt nblumhardt merged commit e953401 into serilog:dev Jun 6, 2017
@nblumhardt nblumhardt mentioned this pull request Jun 6, 2017
Merged
@merbla
Copy link
Contributor

merbla commented Jun 6, 2017

@nblumhardt I have disabled that build.

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

@nblumhardt nblumhardt nblumhardt left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@skomis-mm @nblumhardt @merbla