lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [lucenenet] NightOwl888 commented on pull request #345: Reduce casting
Date Wed, 23 Sep 2020 03:59:35 GMT

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:
users@infra.apache.org



Mime
View raw message