lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [lucenenet] jeme commented on pull request #347: Move call to debugging flag out of loops calling FixedBitSet.Get/Set
Date Sat, 26 Sep 2020 08:57:45 GMT

jeme commented on pull request #347:
URL: https://github.com/apache/lucenenet/pull/347#issuecomment-699464048


   > Is your recommendation to use `Lazy` also based on the incorrect assumption that `Debugging.AssertsEnabled`
reads the value from the dictionary every time it is read, or is there actually some benefit
to using `Lazy` in this instance?
   
   It was because I miss read the change as it was changed a property, so I read it as a `=>`
instead of a `=` - maybe a "brain autocorrect" because I would avoud public fields in all
cases, so I was expected it to be a Property before and now it became a cached Property. Seeing
as it was a Field before, The change has little value i guess... I would change it to a property
in that case though. So:
   
   ```
   public static bool AssertsEnabled { get; set; }= SystemProperties.GetPropertyAsBoolean("assert",
false);
   ```
   
   > ### Memory Consumption
   > What is not clear to me is why adding this feature that is disabled in production
produces a 23% bigger memory footprint while indexing than when the feature is compiled out
using `Debug.Assert()`.
   > 
   > In most cases adding this feature is a wash because we are now able to turn off test
features in production, but the amount of additional RAM is concerning. I suspect this has
something to do with using `Func<string>` as the second parameter of `Debugging.Assert()`
- something that was done to prevent strings from being unnecessarily built in cases where
the condition parameter is `true`.
   > 
   > There are at least 3 things that could be tried to reduce or eliminate this extra
hit:
   > 
   > 1. Use string interpolation rather than `+` to build the message string of `Debugging.Assert(bool,
Func<string>)`.
   > 2. Change all calls to use the `Debugging.Assert(bool, string)` overload except in
2-3 cases that throw exceptions when the condition is `true`. This will slow down testing
for sure, but may improve the situation in production by eliminating the `Func<string>`.
   > 3. Abandon the DRY principle and move the contents of `Debugging.Assert(bool, Func<string>)`
inline into each method where it is called (thus eliminating the `Func<string>` variable).
   > 
   > Thoughts?
   
   Immediately I would think that there was a certain expense in using a Func for this as
compared to alternatives, but that is really just speculations at this point. But I will see
if I can find some time to go poke at it a bit...
   
   You said you had access to JetBrains resharper though an Apache free use program, does
that include dotMemory? It''s a fairly decent tool for doing memory profiling and it can certainly
tell you which objects you have hanging around. It will also allow you to do a run with the
flag on, safe a snapshot, then do a run without the flag and compare the two snapshots etc...
   
   (It will be most straight forward if you can make your testing into a executeable you can
run from it)


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