From 4356997dbbd444e43dc889b06e2ac56af1400aca Mon Sep 17 00:00:00 2001 From: Shad Storhaug Date: Sun, 10 Mar 2024 21:38:14 +0700 Subject: [PATCH] Lucene.Net.Search.ReferenceContext: Converted from a sealed class to a ref struct to eliminate the heap allocation. Converted TestControlledRealTimeReopenThread.TestStraightForwardDemonstration() to test using ReferenceContext to verify functionality. See #920. --- .../TestControlledRealTimeReopenThread.cs | 88 ++++++++++++++----- .../Support/Search/ReferenceContext.cs | 22 +++-- 2 files changed, 80 insertions(+), 30 deletions(-) diff --git a/src/Lucene.Net.Tests/Search/TestControlledRealTimeReopenThread.cs b/src/Lucene.Net.Tests/Search/TestControlledRealTimeReopenThread.cs index b7ec6dc3dc..ee9d3a4df8 100644 --- a/src/Lucene.Net.Tests/Search/TestControlledRealTimeReopenThread.cs +++ b/src/Lucene.Net.Tests/Search/TestControlledRealTimeReopenThread.cs @@ -770,15 +770,36 @@ public void TestStraightForwardDemonstration() controlledRealTimeReopenThread.Start(); //An indexSearcher only sees Doc1 - IndexSearcher indexSearcher = searcherManager.Acquire(); - try + + // In Java, to obtain a threadsafe IndexSearcher reference, the following pattern could + // be used. This also works in .NET. + //IndexSearcher indexSearcher = searcherManager.Acquire(); + //try + //{ + // TopDocs topDocs = indexSearcher.Search(new MatchAllDocsQuery(), 1); + // assertEquals(1, topDocs.TotalHits); //There is only one doc + //} + //finally + //{ + // searcherManager.Release(indexSearcher); + //} + + // However, in .NET it can be done like this with less code. We get an instance of + // ReferenceContext in a using block so the call to searcherManager.Release() + // happens implicitly. ReferenceContext is a ref struct so it doesn't allocate + // on the heap and will be deallocated at the end of this block automatically. + using (var context = searcherManager.GetContext()) { + IndexSearcher indexSearcher = context.Reference; TopDocs topDocs = indexSearcher.Search(new MatchAllDocsQuery(), 1); assertEquals(1, topDocs.TotalHits); //There is only one doc } - finally + + using (var context = searcherManager.GetContext()) { - searcherManager.Release(indexSearcher); + IndexSearcher indexSearcher = context.Reference; + TopDocs topDocs = indexSearcher.Search(new MatchAllDocsQuery(), 1); + assertEquals(1, topDocs.TotalHits); //There is only one doc } //Add a 2nd document @@ -789,32 +810,42 @@ public void TestStraightForwardDemonstration() //Demonstrate that we can only see the first doc because we haven't //waited 1 sec or called WaitForGeneration - indexSearcher = searcherManager.Acquire(); - try + + // In Java, to obtain a threadsafe IndexSearcher reference, the following pattern could + // be used. This also works in .NET. + //indexSearcher = searcherManager.Acquire(); + //try + //{ + // TopDocs topDocs = indexSearcher.Search(new MatchAllDocsQuery(), 1); + // assertEquals(1, topDocs.TotalHits); //Can see both docs due to auto refresh after 1.1 secs + //} + //finally + //{ + // searcherManager.Release(indexSearcher); + //} + + // However, in .NET it can be done like this with less code. We get an instance of + // ReferenceContext in a using block so the call to searcherManager.Release() + // happens implicitly. ReferenceContext is a ref struct so it doesn't allocate + // on the heap and will be deallocated at the end of this block automatically. + using (var context = searcherManager.GetContext()) { + IndexSearcher indexSearcher = context.Reference; TopDocs topDocs = indexSearcher.Search(new MatchAllDocsQuery(), 1); assertEquals(1, topDocs.TotalHits); //Can see both docs due to auto refresh after 1.1 secs } - finally - { - searcherManager.Release(indexSearcher); - } //Demonstrate that we can see both docs after we wait a little more //then 1 sec so that controlledRealTimeReopenThread max interval is exceeded //and it calls MaybeRefresh Thread.Sleep(1100); //wait 1.1 secs as ms - indexSearcher = searcherManager.Acquire(); - try + using (var context = searcherManager.GetContext()) { + IndexSearcher indexSearcher = context.Reference; TopDocs topDocs = indexSearcher.Search(new MatchAllDocsQuery(), 1); assertEquals(2, topDocs.TotalHits); //Can see both docs due to auto refresh after 1.1 secs } - finally - { - searcherManager.Release(indexSearcher); - } //Add a 3rd document @@ -831,16 +862,29 @@ public void TestStraightForwardDemonstration() stopwatch.Stop(); assertTrue(stopwatch.Elapsed.TotalMilliseconds <= 200 + 30); //30ms is fudged factor to account for call overhead. - indexSearcher = searcherManager.Acquire(); - try + // In Java, to obtain a threadsafe IndexSearcher reference, the following pattern could + // be used. This also works in .NET. + //indexSearcher = searcherManager.Acquire(); + //try + //{ + // TopDocs topDocs = indexSearcher.Search(new MatchAllDocsQuery(), 1); + // assertEquals(3, topDocs.TotalHits); //Can see both docs due to auto refresh after 1.1 secs + //} + //finally + //{ + // searcherManager.Release(indexSearcher); + //} + + // However, in .NET it can be done like this with less code. We get an instance of + // ReferenceContext in a using block so the call to searcherManager.Release() + // happens implicitly. ReferenceContext is a ref struct so it doesn't allocate + // on the heap and will be deallocated at the end of this block automatically. + using (var context = searcherManager.GetContext()) { + IndexSearcher indexSearcher = context.Reference; TopDocs topDocs = indexSearcher.Search(new MatchAllDocsQuery(), 1); assertEquals(3, topDocs.TotalHits); //Can see both docs due to auto refresh after 1.1 secs } - finally - { - searcherManager.Release(indexSearcher); - } controlledRealTimeReopenThread.Dispose(); searcherManager.Dispose(); diff --git a/src/Lucene.Net/Support/Search/ReferenceContext.cs b/src/Lucene.Net/Support/Search/ReferenceContext.cs index 0c98168c22..3d3ae50475 100644 --- a/src/Lucene.Net/Support/Search/ReferenceContext.cs +++ b/src/Lucene.Net/Support/Search/ReferenceContext.cs @@ -1,4 +1,6 @@ using System; +using System.Diagnostics.CodeAnalysis; +#nullable enable namespace Lucene.Net.Search { @@ -22,28 +24,32 @@ namespace Lucene.Net.Search /// /// holds a reference instance and /// ensures it is properly de-referenced from its corresponding - /// when is called. This class is primarily intended - /// to be used with a using block. + /// when is called. This struct is intended + /// to be used with a using block to simplify releasing a reference + /// such as a instance. /// /// LUCENENET specific /// - /// The reference type - public sealed class ReferenceContext : IDisposable + /// The reference type. + public ref struct ReferenceContext where T : class { - private readonly ReferenceManager referenceManager; - private T reference; + [SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary suppression", Justification = "This is a SonarCloud issue")] + [SuppressMessage("Major Code Smell", "S2933:Fields that are only assigned in the constructor should be \"readonly\"", Justification = "Structs are known to have performance issues with readonly fields")] + [SuppressMessage("Style", "IDE0044:Add readonly modifier", Justification = "Structs are known to have performance issues with readonly fields")] + private ReferenceManager referenceManager; + private T? reference; internal ReferenceContext(ReferenceManager referenceManager) { - this.referenceManager = referenceManager; + this.referenceManager = referenceManager ?? throw new ArgumentNullException(nameof(referenceManager)); this.reference = referenceManager.Acquire(); } /// /// The reference acquired from the . /// - public T Reference => reference; + public readonly T Reference => reference ?? throw new ObjectDisposedException(nameof(ReferenceContext)); /// /// Ensures the reference is properly de-referenced from its .