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 Fri, 25 Sep 2020 13:28:51 GMT

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


   While it might not be possible to determine which asserts were specifically meant for the
features we need to enable for end users to use all of Lucene.NET's features, there are certain
asserts that we can definitely rule out as having **no benefit** to end users.
   
   In those cases, we can reduce the impact of this feature by going back to compiling out
of the release by using `System.Diagnostics.Debug.Assert()`. It just requires a bit more effort
than find & replace offered. All of the asserts in the example I provided before can be
reverted provided we ramp up our testing to include a nightly build that runs tests in debug
mode as well as release mode.
   
   ```c#
           private DocumentsWriterPerThread InternalTryCheckOutForFlush(ThreadState perThread)
           {
               if (Debugging.AssertsEnabled)
               {
                   // LUCENENET specific - Since we need to mimic the unfair behavior of ReentrantLock,
we need to ensure that all threads that enter here hold the lock.
                   Debugging.Assert(perThread.IsHeldByCurrentThread);
                   Debugging.Assert(Monitor.IsEntered(this));
                   Debugging.Assert(perThread.flushPending);
               }
               try
               {
                   // LUCENENET specific - We removed the call to perThread.TryLock() and
the try-finally below as they are no longer needed.
   
                   // We are pending so all memory is already moved to flushBytes
                   if (perThread.IsInitialized)
                   {
                       if (Debugging.AssertsEnabled) Debugging.Assert(perThread.IsHeldByCurrentThread);
                       DocumentsWriterPerThread dwpt;
                       long bytes = perThread.bytesUsed; // do that before
                       // replace!
                       dwpt = perThreadPool.Reset(perThread, closed);
                       if (Debugging.AssertsEnabled) Debugging.Assert(!flushingWriters.ContainsKey(dwpt),
"DWPT is already flushing");
                       // Record the flushing DWPT to reduce flushBytes in doAfterFlush
                       flushingWriters[dwpt] = bytes;
                       numPending--; // write access synced
                       return dwpt;
                   }
                   return null;
               }
               finally
               {
                   UpdateStallState();
               }
           }
   ```
   
   In these cases, the end user has no control over the conditions that are being checked
- they are internal state of `IndexWriter` (and related classes). However, in cases where
a value is being checked that is being provided by the outside world, we will need the feature
to turn on asserts to ensure `CheckIndex` and the test framework checks what it should for
end users.
   
   The same can probably be said about many more of the asserts. We should start with `IndexWriter`
and its related classes, since the biggest performance impact is there according to the benchmarks.
   
   I am working on getting a nightly build set up so we can move the burden of testing edge
cases and invariants such as these out of the normal workflow. While all of the features in
both the test framework and the Azure Pipelines templates are already implemented for nightly
builds, some of the tests were designed with longer runs than the 1 hour limit of Azure DevOps
in mind, so adjustments to the nightly test limits need to be made to keep it from timing
out.


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