NightOwl888 commented on pull request #345:
URL: https://github.com/apache/lucenenet/pull/345#issuecomment-697119925
@jeme
Thanks Jens. Looks like a `null` check has a significant impact if many
cases are `null`. However, the cases I was worried about look more like:
```c#
object something = cache.Value;
if (something is Value1Type)
{
Value1Type value1 = (Value1Type)something; // <- Allocation happens
here (conditionally)
// do something with value1 and return
return value1.Get();
}
else if (something is Value2Type)
{
Value2Type value2 = (Value2Type)something; // <- Allocation happens
here (conditionally)
// do something with value2 and return
return value2.Get();
}
return new ARealValue(something);
```
If a large number of cases are `Value2Type` (or even neither `Value1Type` or
`Value2Type`) what I am wondering is do we waste memory allocation on
`Value1Type` if it uses pattern matching (`if (something is Value1Type
value1)`)?
```c#
object something = cache.Value;
if (something is Value1Type value1) // <- Allocation happens here (always)
{
// do something with value1 and return
return value1.Get();
}
else if (something is Value2Type value2) // <- Allocation happens here
(always when first if condition fails)
{
// do something with value2 and return
return value2.Get();
}
// If we are here, we are throwing out 2 unnecessary memory allocations
return new ARealValue(something);
```
It seems like we are declaring `Value1Type value1 = default` and then
throwing its value out when the `if` condition fails, but perhaps the IL is
compensating and converting it back to the above behind the scenes...? I
suspect not because the `value1` variable can be used outside of the `if` block
even if the condition fails.
This would be especially important in `Equals()` methods where we want to
shunt to `false` as quickly as possible with as few allocations as possible.
It looks like you have covered that case in your benchmarks above, but the
important metric to consider is the memory allocated rather than the execution
time, as memory allocations will cause GC to happen more frequently, which may
negate any gain in raw performance. `null` reference type memory allocations
[are either 4 or 8
bytes](https://stackoverflow.com/questions/3801878/how-much-memory-does-null-pointer-use)
of extra RAM, so they will contribute to the total amount of memory pressure
even if they are never assigned a real value.
I have run a benchmark, and even after cranking it up to 1 billion
iterations, it seems that either this is not a significant thing to worry about
or BenchmarkDotNet does not consider `null` reference type allocations when it
measures the results.
``` ini
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.18363.1016
(1909/November2018Update/19H2)
Intel Core i7-8850H CPU 2.60GHz (Coffee Lake), 1 CPU, 12 logical and 6
physical cores
.NET Core SDK=3.1.301
[Host] : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX
4.700.20.27001), X64 RyuJIT
DefaultJob : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX
4.700.20.27001), X64 RyuJIT
```
| Method | Mean | Error | StdDev |
Gen 0 | Gen 1 | Gen 2 | Allocated |
|--------------------------------------
|---------:|----------:|----------:|------:|------:|------:|----------:|
| PatternMatching_Class1Set_Class2Set | 5.935 ms | 0.1155 ms | 0.1581 ms |
- | - | - | - |
| PatternMatching_Class1Null_Class2Set | 6.005 ms | 0.1184 ms | 0.1945 ms |
- | - | - | - |
| PatternMatching_Class1Null_Class2Null | 5.903 ms | 0.1120 ms | 0.1333 ms |
- | - | - | - |
| NonMatching_Class1Set_Class2Set | 5.915 ms | 0.1148 ms | 0.1410 ms |
- | - | - | - |
| NonMatching_Class1Null_Class2Set | 5.874 ms | 0.1168 ms | 0.1637 ms |
- | - | - | - |
| NonMatching_Class1Null_Class2Null | 5.897 ms | 0.1133 ms | 0.1391 ms |
- | - | - | - |
```c#
public class Class1 { }
public class Class2 { }
[MemoryDiagnoser]
public class AllocationBenchmarks
{
public static readonly Class1 class1Null = null;
public static readonly Class1 class1Set = new Class1();
public static readonly Class2 class2Null = null;
public static readonly Class2 class2Set = new Class2();
public const int Iterations = 1000000000;
[Benchmark]
public void PatternMatching_Class1Set_Class2Set()
{
for (int i = 0; i < Iterations; i++)
PatternMatching(class1Set, class2Set);
}
[Benchmark]
public void PatternMatching_Class1Null_Class2Set()
{
for (int i = 0; i < Iterations; i++)
PatternMatching(class1Null, class2Set);
}
[Benchmark]
public void PatternMatching_Class1Null_Class2Null()
{
for (int i = 0; i < Iterations; i++)
PatternMatching(class1Null, class2Null);
}
[Benchmark]
public void NonMatching_Class1Set_Class2Set()
{
for (int i = 0; i < Iterations; i++)
NonMatching(class1Set, class2Set);
}
[Benchmark]
public void NonMatching_Class1Null_Class2Set()
{
for (int i = 0; i < Iterations; i++)
NonMatching(class1Null, class2Set);
}
[Benchmark]
public void NonMatching_Class1Null_Class2Null()
{
for (int i = 0; i < Iterations; i++)
PatternMatching(class1Null, class2Null);
}
public static void PatternMatching(Class1 class1, Class2 class2)
{
if (class1 is Class1 c1)
{
}
else if (class2 is Class2 c2)
{
}
}
public static void NonMatching(Class1 class1, Class2 class2)
{
if (class1 is Class1)
{
Class1 c1 = (Class1)class1;
}
else if (class2 is Class2)
{
Class2 c2 = (Class2)class2;
}
}
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
