Skip to content

Commit

Permalink
Remove unnecessary use of NoInlining and add traceability for valid u…
Browse files Browse the repository at this point in the history
…ses, #931 (#1134)

* Use nameof for traceability to StackTraceHelper

* Remove unnecessary NoInlining and add traceability, #931
  • Loading branch information
paulirwin authored Mar 10, 2025
1 parent 690b175 commit 54505e8
Show file tree
Hide file tree
Showing 68 changed files with 111 additions and 224 deletions.
13 changes: 5 additions & 8 deletions src/Lucene.Net.Misc/Util/Fst/ListOfOutputs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
using Lucene.Net.Store;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Text;
using JCG = J2N.Collections.Generic;

Expand Down Expand Up @@ -34,11 +32,11 @@ namespace Lucene.Net.Util.Fst
/// output by calling <see cref="Builder{T}.Add(Int32sRef,T)"/> multiple
/// times. The builder will then combine the outputs using
/// the <see cref="Outputs{T}.Merge(T,T)"/> method.
///
///
/// <para>The resulting FST may not be minimal when an input has
/// more than one output, as this requires pushing all
/// multi-output values to a final state.
///
///
/// </para>
/// <para>NOTE: the only way to create multiple outputs is to
/// add the same input to the FST multiple times in a row. This is
Expand All @@ -48,11 +46,11 @@ namespace Lucene.Net.Util.Fst
/// <see cref="UpToTwoPositiveInt64Outputs"/> instead since it stores
/// the outputs more compactly (by stealing a bit from each
/// long value).
///
///
/// </para>
/// <para>NOTE: this cannot wrap itself (ie you cannot make an
/// FST with List&lt;List&lt;Object&gt;&gt; outputs using this).
///
///
/// @lucene.experimental
/// </para>
/// </summary>
Expand Down Expand Up @@ -176,7 +174,6 @@ public override string OutputToString(object output)
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override object Merge(object first, object second)
{
IList<T> outputList = new JCG.List<T>();
Expand Down Expand Up @@ -224,4 +221,4 @@ public IList<T> AsList(object output)
}
}
}
}
}
1 change: 0 additions & 1 deletion src/Lucene.Net.Misc/Util/Fst/UpToTwoPositiveIntOutputs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,6 @@ public override string OutputToString(object output)
return output.ToString();
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override object Merge(object first, object second)
{
if (Debugging.AssertsEnabled)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ protected override bool SortTermsByUnicode

// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
if (StackTraceHelper.DoesStackTraceContainMethod("Merge"))
if (StackTraceHelper.DoesStackTraceContainMethod(nameof(SegmentMerger.Merge)))
{
unicodeSortOrder = false;
if (LuceneTestCase.Verbose)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,12 @@ protected internal override bool SortTermsByUnicode()
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
if (StackTraceHelper.DoesStackTraceContainMethod("Merge"))
{
unicodeSortOrder = false;
if (LuceneTestCase.Verbose)
{
Console.WriteLine("NOTE: PreFlexRW codec: forcing legacy UTF16 vector term sort order");
}
// LUCENENET TODO: This does not seem to be hit, unused?
unicodeSortOrder = false;
if (LuceneTestCase.Verbose)
{
Console.WriteLine("NOTE: PreFlexRW codec: forcing legacy UTF16 vector term sort order");
}
}

return unicodeSortOrder;
Expand Down
6 changes: 3 additions & 3 deletions src/Lucene.Net.TestFramework/Store/MockDirectoryWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ private bool MustSync()
return @delegate is NRTCachingDirectory;
}

[MethodImpl(MethodImplOptions.NoInlining)]
[MethodImpl(MethodImplOptions.NoInlining)] // Stack trace needed intact in TestIndexWriterExceptions,
public override void Sync(ICollection<string> names)
{
UninterruptableMonitor.Enter(this);
Expand Down Expand Up @@ -519,7 +519,7 @@ internal virtual void MaybeThrowIOExceptionOnOpen(string name)
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
[MethodImpl(MethodImplOptions.NoInlining)] // Stack trace needed intact in TestIndexWriterExceptions
public override void DeleteFile(string name)
{
UninterruptableMonitor.Enter(this);
Expand Down Expand Up @@ -576,7 +576,7 @@ private void MaybeYield()
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
[MethodImpl(MethodImplOptions.NoInlining)] // Stack trace needed intact in TestIndexWriterExceptions
private void DeleteFile(string name, bool forced)
{
UninterruptableMonitor.Enter(this);
Expand Down
12 changes: 6 additions & 6 deletions src/Lucene.Net.Tests/Index/TestConcurrentMergeScheduler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ public override void Eval(MockDirectoryWrapper dir)
{
// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
bool isDoFlush = StackTraceHelper.DoesStackTraceContainMethod("Flush");
bool isClose = StackTraceHelper.DoesStackTraceContainMethod("Close") ||
StackTraceHelper.DoesStackTraceContainMethod("Dispose");
bool isDoFlush = StackTraceHelper.DoesStackTraceContainMethod(nameof(DocumentsWriterPerThread.Flush));
bool isClose = StackTraceHelper.DoesStackTraceContainMethod(nameof(IndexWriter.Close)) || // LUCENENET NOTE: Close is aggressively inlined, so likely won't hit this case, but would hit Dispose
StackTraceHelper.DoesStackTraceContainMethod(nameof(IndexWriter.Dispose));

if (isDoFlush && !isClose && Random.NextBoolean())
{
Expand Down Expand Up @@ -334,7 +334,7 @@ public ConcurrentMergeSchedulerAnonymousClass(int maxMergeCount, CountdownEvent
this.failed = failed;
}

protected override void DoMerge(MergePolicy.OneMerge merge)
protected internal override void DoMerge(MergePolicy.OneMerge merge)
{
try
{
Expand Down Expand Up @@ -385,7 +385,7 @@ public TrackingCMS()
SetMaxMergesAndThreads(5, 5);
}

protected override void DoMerge(MergePolicy.OneMerge merge)
protected internal override void DoMerge(MergePolicy.OneMerge merge)
{
totMergedBytes += merge.TotalBytesSize;
base.DoMerge(merge);
Expand Down Expand Up @@ -432,7 +432,7 @@ public override void Eval(MockDirectoryWrapper dir)
{
// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
if (StackTraceHelper.DoesStackTraceContainMethod("DoMerge"))
if (StackTraceHelper.DoesStackTraceContainMethod(nameof(ConcurrentMergeScheduler.DoMerge)))
{
throw new IOException("now failing during merge");
}
Expand Down
6 changes: 3 additions & 3 deletions src/Lucene.Net.Tests/Index/TestIndexWriterDelete.cs
Original file line number Diff line number Diff line change
Expand Up @@ -975,8 +975,8 @@ public override void Eval(MockDirectoryWrapper dir)
// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
bool seen =
StackTraceHelper.DoesStackTraceContainMethod("ApplyDeletesAndUpdates") ||
StackTraceHelper.DoesStackTraceContainMethod("SlowFileExists");
StackTraceHelper.DoesStackTraceContainMethod(nameof(BufferedUpdatesStream.ApplyDeletesAndUpdates)) ||
StackTraceHelper.DoesStackTraceContainMethod(nameof(IndexWriter.SlowFileExists));

if (!seen)
{
Expand All @@ -994,7 +994,7 @@ public override void Eval(MockDirectoryWrapper dir)
{
// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
if (StackTraceHelper.DoesStackTraceContainMethod("ApplyDeletesAndUpdates"))
if (StackTraceHelper.DoesStackTraceContainMethod(nameof(BufferedUpdatesStream.ApplyDeletesAndUpdates)))
{
if (Verbose)
{
Expand Down
14 changes: 8 additions & 6 deletions src/Lucene.Net.Tests/Index/TestIndexWriterExceptions.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using J2N.Threading;
using J2N.Threading.Atomic;
using Lucene.Net.Analysis;
using Lucene.Net.Codecs;
using Lucene.Net.Codecs.Lucene40;
using Lucene.Net.Diagnostics;
using Lucene.Net.Documents;
using Lucene.Net.Index.Extensions;
Expand Down Expand Up @@ -699,7 +701,7 @@ public override void Eval(MockDirectoryWrapper dir)
// which will always be true if the first case is true. The name of the variable implies it checking for "Append"
// but that is not a method on FreqProxTermsWriterPerField.
bool sawAppend = StackTraceHelper.DoesStackTraceContainMethod(nameof(FreqProxTermsWriterPerField), nameof(FreqProxTermsWriterPerField.Flush));
bool sawFlush = StackTraceHelper.DoesStackTraceContainMethod("Flush");
bool sawFlush = StackTraceHelper.DoesStackTraceContainMethod(nameof(FreqProxTermsWriterPerField.Flush));

if (sawAppend && sawFlush && count++ >= 30)
{
Expand Down Expand Up @@ -1810,7 +1812,7 @@ public override IndexInput OpenInput(string name, IOContext context)
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
if (doFail
&& name.StartsWith("segments_", StringComparison.Ordinal)
&& StackTraceHelper.DoesStackTraceContainMethod("Read"))
&& StackTraceHelper.DoesStackTraceContainMethod(nameof(SegmentInfos.Read)))
{
throw UnsupportedOperationException.Create("expected UOE");
}
Expand Down Expand Up @@ -2349,9 +2351,9 @@ public override void Eval(MockDirectoryWrapper dir)

// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
bool sawSeal = StackTraceHelper.DoesStackTraceContainMethod("SealFlushedSegment");
bool sawWrite = StackTraceHelper.DoesStackTraceContainMethod("WriteLiveDocs")
|| StackTraceHelper.DoesStackTraceContainMethod("WriteFieldUpdates");
bool sawSeal = StackTraceHelper.DoesStackTraceContainMethod(nameof(DocumentsWriterPerThread.SealFlushedSegment));
bool sawWrite = StackTraceHelper.DoesStackTraceContainMethod(nameof(Lucene40LiveDocsFormat.WriteLiveDocs))
|| StackTraceHelper.DoesStackTraceContainMethod(nameof(ReadersAndUpdates.WriteFieldUpdates));

// Don't throw exc if we are "flushing", else
// the segment is aborted and docs are lost:
Expand Down Expand Up @@ -2527,7 +2529,7 @@ public override void Eval(MockDirectoryWrapper dir)
{
// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
bool maybeFail = StackTraceHelper.DoesStackTraceContainMethod("RollbackInternal");
bool maybeFail = StackTraceHelper.DoesStackTraceContainMethod(nameof(IndexWriter.RollbackInternal));

if (maybeFail && Random.Next(10) == 0)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Lucene.Net.Tests/Index/TestIndexWriterReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,7 @@ public override void Eval(MockDirectoryWrapper dir)
{
// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
if (shouldFail && StackTraceHelper.DoesStackTraceContainMethod("GetReadOnlyClone"))
if (shouldFail && StackTraceHelper.DoesStackTraceContainMethod(nameof(ReadersAndUpdates.GetReadOnlyClone)))
{
if (Verbose)
{
Expand Down
10 changes: 5 additions & 5 deletions src/Lucene.Net.Tests/Index/TestIndexWriterWithThreads.cs
Original file line number Diff line number Diff line change
Expand Up @@ -433,11 +433,11 @@ public override void Eval(MockDirectoryWrapper dir)
{
// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
bool sawAbortOrFlushDoc = StackTraceHelper.DoesStackTraceContainMethod("Abort")
|| StackTraceHelper.DoesStackTraceContainMethod("FinishDocument");
bool sawClose = StackTraceHelper.DoesStackTraceContainMethod("Close")
|| StackTraceHelper.DoesStackTraceContainMethod("Dispose");
bool sawMerge = StackTraceHelper.DoesStackTraceContainMethod("Merge");
bool sawAbortOrFlushDoc = StackTraceHelper.DoesStackTraceContainMethod(nameof(DocumentsWriterPerThread.Abort))
|| StackTraceHelper.DoesStackTraceContainMethod(nameof(DocFieldProcessor.FinishDocument));
bool sawClose = StackTraceHelper.DoesStackTraceContainMethod(nameof(IndexWriter.Close))
|| StackTraceHelper.DoesStackTraceContainMethod(nameof(IndexWriter.Dispose));
bool sawMerge = StackTraceHelper.DoesStackTraceContainMethod(nameof(IndexWriter.Merge));

if (sawAbortOrFlushDoc && !sawClose && !sawMerge)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Lucene.Net.Tests/TestMergeSchedulerExternal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ protected override void HandleMergeException(Exception t)
outerInstance.excCalled = true;
}

protected override void DoMerge(MergePolicy.OneMerge merge)
protected internal override void DoMerge(MergePolicy.OneMerge merge)
{
outerInstance.mergeCalled = true;
base.DoMerge(merge);
Expand All @@ -97,7 +97,7 @@ public override void Eval(MockDirectoryWrapper dir)
{
// LUCENENET specific: for these to work in release mode, we have added [MethodImpl(MethodImplOptions.NoInlining)]
// to each possible target of the StackTraceHelper. If these change, so must the attribute on the target methods.
if (StackTraceHelper.DoesStackTraceContainMethod("DoMerge"))
if (StackTraceHelper.DoesStackTraceContainMethod(nameof(ConcurrentMergeScheduler.DoMerge)))
{
throw new IOException("now failing during merge");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ public override void StartDocument(int numStoredFields)
++numBufferedDocs;
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override void FinishDocument()
{
endOffsets[numBufferedDocs - 1] = bufferedDocs.Length;
Expand Down Expand Up @@ -240,7 +239,6 @@ private bool TriggerFlush()
return bufferedDocs.Length >= chunkSize || numBufferedDocs >= MAX_DOCUMENTS_PER_CHUNK; // chunks of at least chunkSize bytes
}

[MethodImpl(MethodImplOptions.NoInlining)]
private void Flush()
{
indexWriter.WriteIndex(numBufferedDocs, fieldsStream.Position); // LUCENENET specific: Renamed from getFilePointer() to match FileStream
Expand Down Expand Up @@ -363,7 +361,6 @@ public override void WriteField(FieldInfo info, IIndexableField field)
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override void Abort()
{
IOUtils.DisposeWhileHandlingException(this);
Expand All @@ -389,7 +386,6 @@ public override void Finish(FieldInfos fis, int numDocs)
if (Debugging.AssertsEnabled) Debugging.Assert(bufferedDocs.Length == 0);
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override int Merge(MergeState mergeState)
{
int docCount = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,6 @@ protected override void Dispose(bool disposing)
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override void Abort()
{
IOUtils.DisposeWhileHandlingException(this);
Expand All @@ -331,7 +330,6 @@ public override void StartDocument(int numVectorFields)
curDoc = AddDocData(numVectorFields);
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override void FinishDocument()
{
// append the payload bytes of the doc after its terms
Expand Down Expand Up @@ -390,7 +388,6 @@ private bool TriggerFlush()
return termSuffixes.Length >= chunkSize || pendingDocs.Count >= MAX_DOCUMENTS_PER_CHUNK;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private void Flush()
{
int chunkDocs = pendingDocs.Count;
Expand Down Expand Up @@ -879,7 +876,6 @@ public override void AddProx(int numProx, DataInput positions, DataInput offsets
curField.totalPositions += numProx;
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override int Merge(MergeState mergeState)
{
int docCount = 0;
Expand Down
7 changes: 2 additions & 5 deletions src/Lucene.Net/Codecs/FieldsConsumer.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
using Lucene.Net.Diagnostics;
using System;
using System.Diagnostics;
using System.Runtime.CompilerServices;

namespace Lucene.Net.Codecs
{
Expand Down Expand Up @@ -77,11 +75,10 @@ public void Dispose()
/// <summary>
/// Called during merging to merge all <see cref="Fields"/> from
/// sub-readers. this must recurse to merge all postings
/// (terms, docs, positions, etc.). A
/// (terms, docs, positions, etc.). A
/// <see cref="PostingsFormat"/> can override this default
/// implementation to do its own merging.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
public virtual void Merge(MergeState mergeState, Fields fields)
{
foreach (string field in fields)
Expand All @@ -97,4 +94,4 @@ public virtual void Merge(MergeState mergeState, Fields fields)
}
}
}
}
}
4 changes: 2 additions & 2 deletions src/Lucene.Net/Codecs/Lucene40/Lucene40LiveDocsFormat.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public override IBits ReadLiveDocs(Directory dir, SegmentCommitInfo info, IOCont
return liveDocs;
}

[MethodImpl(MethodImplOptions.NoInlining)]
[MethodImpl(MethodImplOptions.NoInlining)] // Stack trace needed intact in TestIndexWriterExceptions, TestIndexWriterOnDiskFull
public override void WriteLiveDocs(IMutableBits bits, Directory dir, SegmentCommitInfo info, int newDelCount, IOContext context)
{
string filename = IndexFileNames.FileNameFromGeneration(info.Info.Name, DELETES_EXTENSION, info.NextDelGen);
Expand All @@ -122,4 +122,4 @@ public override void Files(SegmentCommitInfo info, ICollection<string> files)
}
}
}
}
}
2 changes: 0 additions & 2 deletions src/Lucene.Net/Codecs/Lucene40/Lucene40StoredFieldsWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ protected override void Dispose(bool disposing)
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override void Abort()
{
try
Expand Down Expand Up @@ -288,7 +287,6 @@ public override void Finish(FieldInfos fis, int numDocs)
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
public override int Merge(MergeState mergeState)
{
int docCount = 0;
Expand Down
Loading

0 comments on commit 54505e8

Please sign in to comment.