SWEEP: Fixed Debugging.Assert() calls that format strings with parame…

…ters so the parameters are not resolved until a condition fails. There are still some calls that do light math and pick items from arrays, but this performance hit in the tests is something we can live with for better production performance. Closes apache#375, closes apache#373, closes apache#372.
NightOwl888 · Nov 3, 2020 · 81ad561 · 81ad561
1 parent 5104be2
commit 81ad561
Show file tree

Hide file tree

Showing 71 changed files with 208 additions and 189 deletions.
diff --git a/src/Lucene.Net.Analysis.Common/Analysis/CharFilter/BaseCharFilter.cs b/src/Lucene.Net.Analysis.Common/Analysis/CharFilter/BaseCharFilter.cs
@@ -114,7 +114,7 @@ protected virtual void AddOffCorrectMap(int off, int cumulativeDiff)
             }
 
             int offset = offsets[(size == 0) ? 0 : size - 1];
-            if (Debugging.AssertsEnabled) Debugging.Assert(size == 0 || off >= offset, "Offset #{0}({1}) is less than the last recorded offset {2}\n{3}\n{4}", size, off, offset, Arrays.ToString(offsets), Arrays.ToString(diffs));
+            if (Debugging.AssertsEnabled) Debugging.Assert(size == 0 || off >= offset, "Offset #{0}({1}) is less than the last recorded offset {2}\n{3}\n{4}", size, off, offset, offsets, diffs);
 
             if (size == 0 || off != offsets[size - 1])
             {

diff --git a/src/Lucene.Net.Analysis.Common/Analysis/CharFilter/HTMLStripCharFilter.cs b/src/Lucene.Net.Analysis.Common/Analysis/CharFilter/HTMLStripCharFilter.cs
@@ -31636,9 +31636,9 @@ string hexCharRef
                                 {
                                     codePoint = int.Parse(hexCharRef, NumberStyles.HexNumber, CultureInfo.InvariantCulture);
                                 }
-                                catch (Exception /*e*/)
+                                catch (Exception e)
                                 {
-                                    if (Debugging.AssertsEnabled) Debugging.Assert(false, "Exception parsing hex code point '{0}'");
+                                    if (Debugging.AssertsEnabled) Debugging.Assert(false, "Exception parsing hex code point '{0}'", e);
                                 }
                                 if (codePoint <= 0x10FFFF)
                                 {

diff --git a/src/Lucene.Net.Analysis.Common/Analysis/Util/RollingCharBuffer.cs b/src/Lucene.Net.Analysis.Common/Analysis/Util/RollingCharBuffer.cs
@@ -107,11 +107,14 @@ public int Get(int pos)
             }
             else
             {
-                // Cannot read from future (except by 1):
-                if (Debugging.AssertsEnabled) Debugging.Assert(pos < nextPos);
+                if (Debugging.AssertsEnabled)
+                {
+                    // Cannot read from future (except by 1):
+                    Debugging.Assert(pos < nextPos);
 
-                // Cannot read from already freed past:
-                if (Debugging.AssertsEnabled) Debugging.Assert(nextPos - pos <= count, "nextPos={0} pos={1} count={2}", nextPos, pos, count);
+                    // Cannot read from already freed past:
+                    Debugging.Assert(nextPos - pos <= count, "nextPos={0} pos={1} count={2}", nextPos, pos, count);
+                }
 
                 return buffer[GetIndex(pos)];
             }

diff --git a/src/Lucene.Net.Codecs/SimpleText/SimpleTextDocValuesReader.cs b/src/Lucene.Net.Codecs/SimpleText/SimpleTextDocValuesReader.cs
@@ -1,6 +1,7 @@
 using J2N.Text;
 using Lucene.Net.Diagnostics;
 using Lucene.Net.Support;
+using Lucene.Net.Util;
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
@@ -76,21 +77,24 @@ internal SimpleTextDocValuesReader(SegmentReadState state, string ext)
                 {
                     break;
                 }
-                if (Debugging.AssertsEnabled) Debugging.Assert(StartsWith(SimpleTextDocValuesWriter.FIELD), scratch.Utf8ToString());
+                // LUCENENET specific - use wrapper BytesRefFormatter struct to defer building the string unless string.Format() is called
+                if (Debugging.AssertsEnabled) Debugging.Assert(StartsWith(SimpleTextDocValuesWriter.FIELD), "{0}", new BytesRefFormatter(scratch, BytesRefFormat.UTF8));
                 var fieldName = StripPrefix(SimpleTextDocValuesWriter.FIELD);
                 var field = new OneField();
 
                 fields[fieldName] = field;
 
                 ReadLine();
-                if (Debugging.AssertsEnabled) Debugging.Assert(StartsWith(SimpleTextDocValuesWriter.TYPE), scratch.Utf8ToString());
+                // LUCENENET specific - use wrapper BytesRefFormatter struct to defer building the string unless string.Format() is called
+                if (Debugging.AssertsEnabled) Debugging.Assert(StartsWith(SimpleTextDocValuesWriter.TYPE), "{0}", new BytesRefFormatter(scratch, BytesRefFormat.UTF8));
 
                 var dvType = (DocValuesType)Enum.Parse(typeof(DocValuesType), StripPrefix(SimpleTextDocValuesWriter.TYPE));
                 // if (Debugging.AssertsEnabled) Debugging.Assert(dvType != null); // LUCENENET: Not possible for an enum to be null in .NET
                 if (dvType == DocValuesType.NUMERIC)
                 {
                     ReadLine();
-                    if (Debugging.AssertsEnabled) Debugging.Assert(StartsWith(SimpleTextDocValuesWriter.MINVALUE), "got {0} field={1} ext={2}", scratch.Utf8ToString(), fieldName, ext);
+                    // LUCENENET specific - use wrapper BytesRefFormatter struct to defer building the string unless string.Format() is called
+                    if (Debugging.AssertsEnabled) Debugging.Assert(StartsWith(SimpleTextDocValuesWriter.MINVALUE), "got {0} field={1} ext={2}", new BytesRefFormatter(scratch, BytesRefFormat.UTF8), fieldName, ext);
                     field.MinValue = Convert.ToInt64(StripPrefix(SimpleTextDocValuesWriter.MINVALUE), CultureInfo.InvariantCulture);
                     ReadLine();
                     if (Debugging.AssertsEnabled) Debugging.Assert(StartsWith(SimpleTextDocValuesWriter.PATTERN));
@@ -435,7 +439,8 @@ public override void LookupOrd(int ord, BytesRef result)
                     }
                     _input.Seek(_field.DataStartFilePointer + ord * (9 + _field.Pattern.Length + _field.MaxLength));
                     SimpleTextUtil.ReadLine(_input, _scratch);
-                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextDocValuesWriter.LENGTH), "got {0} in={1}", _scratch.Utf8ToString(), _input.ToString());
+                    // LUCENENET specific - use wrapper BytesRefFormatter struct to defer building the string unless string.Format() is called
+                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextDocValuesWriter.LENGTH), "got {0} in={1}", new BytesRefFormatter(_scratch, BytesRefFormat.UTF8), _input);
                     int len;
                     try
                     {
@@ -539,7 +544,8 @@ public override void LookupOrd(long ord, BytesRef result)
 
                     _input.Seek(_field.DataStartFilePointer + ord * (9 + _field.Pattern.Length + _field.MaxLength));
                     SimpleTextUtil.ReadLine(_input, _scratch);
-                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextDocValuesWriter.LENGTH), "got {0} in={1}", _scratch.Utf8ToString(), _input.ToString());
+                    // LUCENENET specific - use wrapper BytesRefFormatter struct to defer building the string unless string.Format() is called
+                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextDocValuesWriter.LENGTH), "got {0} in={1}", new BytesRefFormatter(_scratch, BytesRefFormat.UTF8), _input);
                     int len;
                     try
                     {

diff --git a/src/Lucene.Net.Codecs/SimpleText/SimpleTextFieldsReader.cs b/src/Lucene.Net.Codecs/SimpleText/SimpleTextFieldsReader.cs
@@ -1,5 +1,6 @@
 using Lucene.Net.Diagnostics;
 using Lucene.Net.Index;
+using Lucene.Net.Util;
 using Lucene.Net.Util.Fst;
 using System;
 using System.Collections.Generic;
@@ -477,7 +478,8 @@ public override int NextPosition()
                 if (_readPositions)
                 {
                     SimpleTextUtil.ReadLine(_in, _scratch);
-                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextFieldsWriter.POS), "got line={0}", _scratch.Utf8ToString());
+                    // LUCENENET specific - use wrapper BytesRefFormatter struct to defer building the string unless string.Format() is called
+                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextFieldsWriter.POS), "got line={0}", new BytesRefFormatter(_scratch, BytesRefFormat.UTF8));
                     UnicodeUtil.UTF8toUTF16(_scratch.Bytes, _scratch.Offset + SimpleTextFieldsWriter.POS.Length, _scratch.Length - SimpleTextFieldsWriter.POS.Length,
                         _scratchUtf162);
                     pos = ArrayUtil.ParseInt32(_scratchUtf162.Chars, 0, _scratchUtf162.Length);
@@ -490,12 +492,14 @@ public override int NextPosition()
                 if (_readOffsets)
                 {
                     SimpleTextUtil.ReadLine(_in, _scratch);
-                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextFieldsWriter.START_OFFSET), "got line={0}", _scratch.Utf8ToString());
+                    // LUCENENET specific - use wrapper BytesRefFormatter struct to defer building the string unless string.Format() is called
+                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextFieldsWriter.START_OFFSET), "got line={0}", new BytesRefFormatter(_scratch, BytesRefFormat.UTF8));
                     UnicodeUtil.UTF8toUTF16(_scratch.Bytes, _scratch.Offset + SimpleTextFieldsWriter.START_OFFSET.Length,
                         _scratch.Length - SimpleTextFieldsWriter.START_OFFSET.Length, _scratchUtf162);
                     _startOffset = ArrayUtil.ParseInt32(_scratchUtf162.Chars, 0, _scratchUtf162.Length);
                     SimpleTextUtil.ReadLine(_in, _scratch);
-                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextFieldsWriter.END_OFFSET), "got line={0}", _scratch.Utf8ToString());
+                    // LUCENENET specific - use wrapper BytesRefFormatter struct to defer building the string unless string.Format() is called
+                    if (Debugging.AssertsEnabled) Debugging.Assert(StringHelper.StartsWith(_scratch, SimpleTextFieldsWriter.END_OFFSET), "got line={0}", new BytesRefFormatter(_scratch, BytesRefFormat.UTF8));
                     UnicodeUtil.UTF8toUTF16(_scratch.Bytes, _scratch.Offset + SimpleTextFieldsWriter.END_OFFSET.Length,
                         _scratch.Length - SimpleTextFieldsWriter.END_OFFSET.Length, _scratchUtf162);
                     _endOffset = ArrayUtil.ParseInt32(_scratchUtf162.Chars, 0, _scratchUtf162.Length);

diff --git a/src/Lucene.Net.Suggest/Suggest/Analyzing/FSTUtil.cs b/src/Lucene.Net.Suggest/Suggest/Analyzing/FSTUtil.cs
@@ -130,7 +130,7 @@ public static IList<Path<T>> IntersectPrefixPaths<T>(Automaton a, FST<T> fst)
                               .CopyFrom(nextArc), fst.Outputs.Add(path.Output, nextArc.Output), newInput));
                             int label = nextArc.Label; // used in assert
                             nextArc = nextArc.IsLast ? null : fst.ReadNextRealArc(nextArc, fstReader);
-                            if (Debugging.AssertsEnabled) Debugging.Assert(nextArc == null || label < nextArc.Label, "last: {0} next: {1}", label, (nextArc == null ? "" : nextArc.Label.ToString()));
+                            if (Debugging.AssertsEnabled) Debugging.Assert(nextArc == null || label < nextArc.Label, "last: {0} next: {1}", label, nextArc?.Label);
                         }
                     }
                 }

diff --git a/src/Lucene.Net.TestFramework/Analysis/LookaheadTokenFilter.cs b/src/Lucene.Net.TestFramework/Analysis/LookaheadTokenFilter.cs
@@ -216,7 +216,7 @@ protected virtual bool PeekToken()
                 else
                 {
                     // Make sure our input isn't messing up offsets:
-                    if (Debugging.AssertsEnabled) Debugging.Assert(startPosData.StartOffset == startOffset,"prev startOffset={0} vs new startOffset={1}", startPosData.StartOffset, startOffset + " inputPos=" + m_inputPos);
+                    if (Debugging.AssertsEnabled) Debugging.Assert(startPosData.StartOffset == startOffset, "prev startOffset={0} vs new startOffset={1} inputPos={2}", startPosData.StartOffset, startOffset, m_inputPos);
                 }
 
                 int endOffset = m_offsetAtt.EndOffset;
@@ -227,7 +227,7 @@ protected virtual bool PeekToken()
                 else
                 {
                     // Make sure our input isn't messing up offsets:
-                    if (Debugging.AssertsEnabled) Debugging.Assert(endPosData.EndOffset == endOffset,"prev endOffset={0} vs new endOffset={1}", endPosData.EndOffset, endOffset + " inputPos=" + m_inputPos);
+                    if (Debugging.AssertsEnabled) Debugging.Assert(endPosData.EndOffset == endOffset, "prev endOffset={0} vs new endOffset={1} inputPos={2}", endPosData.EndOffset, endOffset, m_inputPos);
                 }
 
                 tokenPending = true;

diff --git a/src/Lucene.Net.TestFramework/Analysis/MockTokenizer.cs b/src/Lucene.Net.TestFramework/Analysis/MockTokenizer.cs
@@ -219,20 +219,20 @@ protected virtual int ReadCodePoint()
             }
             else
             {
-                if (Debugging.AssertsEnabled) Debugging.Assert(!char.IsLowSurrogate((char)ch),"unpaired low surrogate: {0}", ch.ToString("x"));
+                if (Debugging.AssertsEnabled) Debugging.Assert(!char.IsLowSurrogate((char)ch),"unpaired low surrogate: {0:x}", ch);
                 off++;
                 if (char.IsHighSurrogate((char)ch))
                 {
                     int ch2 = ReadChar();
                     if (ch2 >= 0)
                     {
                         off++;
-                        if (Debugging.AssertsEnabled) Debugging.Assert(char.IsLowSurrogate((char)ch2),"unpaired high surrogate: {0}, followed by: {1}", ch.ToString("x"), ch2.ToString("x"));
+                        if (Debugging.AssertsEnabled) Debugging.Assert(char.IsLowSurrogate((char)ch2),"unpaired high surrogate: {0:x}, followed by: {1:x}", ch, ch2);
                         return Character.ToCodePoint((char)ch, (char)ch2);
                     }
                     else
                     {
-                        if (Debugging.AssertsEnabled) Debugging.Assert(false,"stream ends with unpaired high surrogate: {0}", ch.ToString("x"));
+                        if (Debugging.AssertsEnabled) Debugging.Assert(false,"stream ends with unpaired high surrogate: {0:x}", ch);
                     }
                 }
                 return ch;

diff --git a/src/Lucene.Net.TestFramework/Codecs/Lucene3x/PreFlexRWNormsConsumer.cs b/src/Lucene.Net.TestFramework/Codecs/Lucene3x/PreFlexRWNormsConsumer.cs
@@ -75,7 +75,7 @@ public PreFlexRWNormsConsumer(Directory directory, string segment, IOContext con
 
         public override void AddNumericField(FieldInfo field, IEnumerable<long?> values)
         {
-            if (Debugging.AssertsEnabled) Debugging.Assert(field.Number > lastFieldNumber,"writing norms fields out of order{0} -> {1}", lastFieldNumber, field.Number);
+            if (Debugging.AssertsEnabled) Debugging.Assert(field.Number > lastFieldNumber,"writing norms fields out of order {0} -> {1}", lastFieldNumber, field.Number);
             foreach (var n in values)
             {
                 if (((sbyte)(byte)(long)n) < sbyte.MinValue || ((sbyte)(byte)(long)n) > sbyte.MaxValue)

diff --git a/src/Lucene.Net.TestFramework/Codecs/Lucene3x/TermInfosWriter.cs b/src/Lucene.Net.TestFramework/Codecs/Lucene3x/TermInfosWriter.cs
@@ -243,10 +243,17 @@ private int CompareToLastTerm(int fieldNumber, BytesRef term)
         /// </summary>
         public void Add(int fieldNumber, BytesRef term, TermInfo ti)
         {
-            if (Debugging.AssertsEnabled) Debugging.Assert(CompareToLastTerm(fieldNumber, term) < 0 || (isIndex && term.Length == 0 && lastTerm.Length == 0), "Terms are out of order: field={0} (number {1}) lastField={2} (number {3}) text={4} lastText={5}", FieldName(fieldInfos, fieldNumber), fieldNumber, FieldName(fieldInfos, lastFieldNumber), lastFieldNumber, term.Utf8ToString(), lastTerm.Utf8ToString());
-
-            if (Debugging.AssertsEnabled) Debugging.Assert(ti.FreqPointer >= lastTi.FreqPointer,"freqPointer out of order ({0} < {1})", ti.FreqPointer, lastTi.FreqPointer);
-            if (Debugging.AssertsEnabled) Debugging.Assert(ti.ProxPointer >= lastTi.ProxPointer,"proxPointer out of order ({0} < {1})", ti.ProxPointer, lastTi.ProxPointer);
+            if (Debugging.AssertsEnabled)
+            {
+                Debugging.Assert(CompareToLastTerm(fieldNumber, term) < 0 || (isIndex && term.Length == 0 && lastTerm.Length == 0),
+                    "Terms are out of order: field={0} (number {1}) lastField={2} (number {3}) text={4} lastText={5}",
+                    FieldName(fieldInfos, fieldNumber), fieldNumber, FieldName(fieldInfos, lastFieldNumber), lastFieldNumber,
+                    // LUCENENET specific - use wrapper BytesRefFormatter struct to defer building the string unless string.Format() is called
+                    new BytesRefFormatter(term, BytesRefFormat.UTF8), new BytesRefFormatter(lastTerm, BytesRefFormat.UTF8));
+
+                Debugging.Assert(ti.FreqPointer >= lastTi.FreqPointer, "freqPointer out of order ({0} < {1})", ti.FreqPointer, lastTi.FreqPointer);
+                Debugging.Assert(ti.ProxPointer >= lastTi.ProxPointer, "proxPointer out of order ({0} < {1})", ti.ProxPointer, lastTi.ProxPointer);
+            }
 
             if (!isIndex && size % indexInterval == 0)
             {

diff --git a/src/Lucene.Net.TestFramework/Codecs/Lucene40/Lucene40PostingsWriter.cs b/src/Lucene.Net.TestFramework/Codecs/Lucene40/Lucene40PostingsWriter.cs
@@ -271,7 +271,7 @@ public override void AddPosition(int position, BytesRef payload, int startOffset
                 // and the numbers aren't that much smaller anyways.
                 int offsetDelta = startOffset - lastOffset;
                 int offsetLength = endOffset - startOffset;
-                if (Debugging.AssertsEnabled) Debugging.Assert(offsetDelta >= 0 && offsetLength >= 0,"startOffset={0},lastOffset={1}", startOffset, lastOffset + ",endOffset=" + endOffset);
+                if (Debugging.AssertsEnabled) Debugging.Assert(offsetDelta >= 0 && offsetLength >= 0, "startOffset={0},lastOffset={1},endOffset={2}", startOffset, lastOffset, endOffset);
                 if (offsetLength != lastOffsetLength)
                 {
                     proxOut.WriteVInt32(offsetDelta << 1 | 1);

diff --git a/src/Lucene.Net.TestFramework/Index/AssertingAtomicReader.cs b/src/Lucene.Net.TestFramework/Index/AssertingAtomicReader.cs
@@ -160,7 +160,7 @@ public override int DocID
         {
             get
             {
-                if (Debugging.AssertsEnabled) Debugging.Assert(doc == base.DocID," invalid DocID in {0} {1}", m_input.GetType(), base.DocID + " instead of " + doc);
+                if (Debugging.AssertsEnabled) Debugging.Assert(doc == base.DocID, " invalid DocID in {0} {1} instead of {2}", m_input.GetType(), base.DocID, doc);
                 return doc;
             }
         }
@@ -621,7 +621,7 @@ public override int DocID
             {
                 get
                 {
-                    if (Debugging.AssertsEnabled) Debugging.Assert(doc == base.DocID," invalid DocID in {0} {1}", m_input.GetType(), base.DocID + " instead of " + doc);
+                    if (Debugging.AssertsEnabled) Debugging.Assert(doc == base.DocID, " invalid DocID in {0} {1} instead of {2}", m_input.GetType(), base.DocID, doc);
                     return doc;
                 }
             }

diff --git a/src/Lucene.Net.TestFramework/Search/ShardSearchingTestBase.cs b/src/Lucene.Net.TestFramework/Search/ShardSearchingTestBase.cs
@@ -306,7 +306,7 @@ public ShardIndexSearcher(ShardSearchingTestBase.NodeState nodeState, long[] nod
                     this.outerInstance = nodeState;
                     this.nodeVersions = nodeVersions;
                     MyNodeID = nodeID;
-                    if (Debugging.AssertsEnabled) Debugging.Assert(MyNodeID == nodeState.MyNodeID,"myNodeID={0} NodeState.this.myNodeID={1}", nodeID, nodeState.MyNodeID);
+                    if (Debugging.AssertsEnabled) Debugging.Assert(MyNodeID == nodeState.MyNodeID,"myNodeID={0} nodeState.MyNodeID={1}", nodeID, nodeState.MyNodeID);
                 }
 
                 public override Query Rewrite(Query original)
@@ -419,7 +419,7 @@ public override CollectionStatistics CollectionStatistics(string field)
                         }
                         // Collection stats are pre-shared on reopen, so,
                         // we better not have a cache miss:
-                        if (Debugging.AssertsEnabled) Debugging.Assert(nodeStats != null,"myNodeID={0} nodeID={1}", MyNodeID, nodeID + " version=" + nodeVersions[nodeID] + " field=" + field);
+                        if (Debugging.AssertsEnabled) Debugging.Assert(nodeStats != null, "myNodeID={0} nodeID={1} version={2} field={3}", MyNodeID, nodeID, nodeVersions[nodeID], field);
 
                         long nodeDocCount = nodeStats.DocCount;
                         if (docCount >= 0 && nodeDocCount >= 0)

diff --git a/src/Lucene.Net.TestFramework/Store/MockDirectoryWrapper.cs b/src/Lucene.Net.TestFramework/Store/MockDirectoryWrapper.cs
@@ -1024,7 +1024,7 @@ protected override void Dispose(bool disposing)
                                         extras += "\n\nThese files we had previously tried to delete, but couldn't: " + pendingDeletions;
                                     }
 
-                                    if (Debugging.AssertsEnabled) Debugging.Assert(false,"unreferenced files: before delete:\n    {0}\n  after delete:\n    {1}", Arrays.ToString(startFiles), Arrays.ToString(endFiles) + extras);
+                                    if (Debugging.AssertsEnabled) Debugging.Assert(false, "unreferenced files: before delete:\n    {0}\n  after delete:\n    {1}{2}", startFiles, endFiles, extras);
                                 }
 
                                 DirectoryReader ir1 = DirectoryReader.Open(this);