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 issue #346: Remove Debugging.AssertsEnabled
Date Wed, 23 Sep 2020 17:10:06 GMT

NightOwl888 commented on issue #346:
URL: https://github.com/apache/lucenenet/issues/346#issuecomment-697715665


   I have spotted and corrected an error in the benchmark.
   
   It looks like the following line:
   
   ```c#
   public static bool AssertsEnabled = SystemProperties.GetPropertyAsBoolean("assert", false);
   ```
   
   was interpreted  as...
   
   ```c#
   public static bool AssertsEnabled => SystemProperties.GetPropertyAsBoolean("assert",
false);
   ```
   
   Subtle, but critical. With that change, the difference is far less than the margin of error.
   
   ```c#
       [SimpleJob(RuntimeMoniker.NetCoreApp31)]
       [MemoryDiagnoser]
       public class DebuggingFlags
       {
           private static Dictionary<string, string> _flags = new Dictionary<string,
string>() { ["assert"] = "false" };
   
           private HashSet<int> _set = new HashSet<int>();
   
           [Params(1000, 10000, 100_000)]
           public int N;
   
           [Benchmark]
           public void Loop()
           {
               for (int i = 0; i < N; i++)
               {
                   Set(i);
               }
           }
   
           [Benchmark]
           public void CachedLoop()
           {
               bool flag = AssertsEnabled;
               for (int i = 0; i < N; i++)
               {
                   SetPassingFlag(i, flag);
               }
           }
   
           internal static bool AssertsEnabled = _flags.TryGetValue("assert", out var flag)
? bool.Parse(flag) : false;
   
           private void Set(int index)
           {
               SetPassingFlag(index, AssertsEnabled);
           }
   
           private void SetPassingFlag(int index, bool flag)
           {
               if (flag) Assert(index > 0 && index < N);
   
               _set.Add(index);
           }
   
           private static void Assert(bool condition)
           {
               if (condition) throw new Exception("Failed assertion");
           }
       }
   ```
   
   ``` 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
     .NET Core 3.1 : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64
RyuJIT
   
   Job=.NET Core 3.1  Runtime=.NET Core 3.1  
   
   ```
   |     Method |      N |        Mean |     Error |    StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated
|
   |----------- |------- |------------:|----------:|----------:|------:|------:|------:|----------:|
   |       **Loop** |   **1000** |    **13.55 μs** |  **0.152 μs** |  **0.127 μs** |  
  **-** |     **-** |     **-** |         **-** |
   | CachedLoop |   1000 |    13.55 μs |  0.217 μs |  0.203 μs |     - |     - |     -
|         - |
   |       **Loop** |  **10000** |   **137.09 μs** |  **2.668 μs** |  **3.911 μs** |  
  **-** |     **-** |     **-** |         **-** |
   | CachedLoop |  10000 |   136.20 μs |  2.403 μs |  2.248 μs |     - |     - |     -
|         - |
   |       **Loop** | **100000** | **1,367.65 μs** | **27.162 μs** | **40.655 μs** |  
  **-** |     **-** |     **-** |         **-** |
   | CachedLoop | 100000 | 1,365.65 μs | 22.746 μs | 27.934 μs |     - |     - |     -
|         - |
   
   I guess another issue is that the dictionary doesn't contain an `"assert"` element by default,
so the `bool.Parse()` should also be taken out of the equation. But since that all happens
the first time the class is accessed by anything, it really makes no difference to the benchmark.
The only difference is that in Lucene.NET, the first call to load the dictionary based on
environment variables is a bit more expensive than just having a static dictionary.
   
   I will try running benchmarks on #347 tomorrow and report the results.


----------------------------------------------------------------
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