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:
[email protected]
