lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [lucenenet] theolivenbaum commented on issue #346: Remove Debugging.AssertsEnabled
Date Wed, 23 Sep 2020 18:15:50 GMT

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


   True, I must have totally misread that line. I had the impression it was
   always calling the System Properties method. Then the cost that was showing
   in the profiler must have been only due to the field access outside of the
   inner method, not from the actual getter, so we can remove the change to
   the Debugging file
   
   On Wed, Sep 23, 2020, 7:10 PM Shad Storhaug <notifications@github.com>
   wrote:
   
   > I have spotted and corrected an error in the benchmark.
   >
   > It looks like the following line:
   >
   > public static bool AssertsEnabled = SystemProperties.GetPropertyAsBoolean("assert",
false);
   >
   > was interpreted as...
   >
   > public static bool AssertsEnabled => SystemProperties.GetPropertyAsBoolean("assert",
false);
   >
   > Subtle, but critical. With that change, the difference is far less than
   > the margin of error.
   >
   >     [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");
   >
   >         }
   >
   >     }
   >
   > 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
   > <https://github.com/apache/lucenenet/pull/347> tomorrow and report the
   > results.
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/lucenenet/issues/346#issuecomment-697715665>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ACDCOAYMDNZM2ZXOE6GEFS3SHITXHANCNFSM4RV2BDBA>
   > .
   >
   


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