From e4f0dd25a4472420f29d78b53902b9cc11135744 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Thu, 12 Dec 2024 12:01:32 -0700 Subject: [PATCH 1/8] Convert volatile long/double values to Atomic counterparts, #1063 --- src/Lucene.Net.Replicator/LocalReplicator.cs | 8 +- .../Lucene.Net.Tests._J-S.csproj | 6 +- .../Support/Threading/AtomicDoubleTest.cs | 255 +++++++++++ src/Lucene.Net/Index/IndexWriter.cs | 27 +- src/Lucene.Net/Index/IndexWriterConfig.cs | 4 +- src/Lucene.Net/Index/LiveIndexWriterConfig.cs | 27 +- src/Lucene.Net/Index/MergePolicy.cs | 14 +- src/Lucene.Net/Index/SegmentCommitInfo.cs | 11 +- .../Search/ControlledRealTimeReopenThread.cs | 7 +- .../Search/TimeLimitingCollector.cs | 24 +- src/Lucene.Net/Store/RateLimiter.cs | 23 +- .../Support/Threading/AtomicDouble.cs | 413 ++++++++++++++++++ 12 files changed, 755 insertions(+), 64 deletions(-) create mode 100644 src/Lucene.Net.Tests/Support/Threading/AtomicDoubleTest.cs create mode 100644 src/Lucene.Net/Support/Threading/AtomicDouble.cs diff --git a/src/Lucene.Net.Replicator/LocalReplicator.cs b/src/Lucene.Net.Replicator/LocalReplicator.cs index a7c06b7dfb..52f30e87ab 100644 --- a/src/Lucene.Net.Replicator/LocalReplicator.cs +++ b/src/Lucene.Net.Replicator/LocalReplicator.cs @@ -99,13 +99,15 @@ private class ReplicationSession public SessionToken Session { get; private set; } public RefCountedRevision Revision { get; private set; } - private long lastAccessTime; + // LUCENENET: was volatile long in Lucene, but that is not valid in .NET + // Instead, we use AtomicInt64 to ensure atomicity. + private readonly AtomicInt64 lastAccessTime; public ReplicationSession(SessionToken session, RefCountedRevision revision) { Session = session; Revision = revision; - lastAccessTime = Stopwatch.GetTimestamp(); // LUCENENET: Use the most accurate timer to determine expiration + lastAccessTime = new AtomicInt64(Stopwatch.GetTimestamp()); // LUCENENET: Use the most accurate timer to determine expiration } public virtual bool IsExpired(long expirationThreshold) @@ -115,7 +117,7 @@ public virtual bool IsExpired(long expirationThreshold) public virtual void MarkAccessed() { - lastAccessTime = Stopwatch.GetTimestamp(); // LUCENENET: Use the most accurate timer to determine expiration + lastAccessTime.Value = Stopwatch.GetTimestamp(); // LUCENENET: Use the most accurate timer to determine expiration } } diff --git a/src/Lucene.Net.Tests._J-S/Lucene.Net.Tests._J-S.csproj b/src/Lucene.Net.Tests._J-S/Lucene.Net.Tests._J-S.csproj index 22b6666d5b..23759b8471 100644 --- a/src/Lucene.Net.Tests._J-S/Lucene.Net.Tests._J-S.csproj +++ b/src/Lucene.Net.Tests._J-S/Lucene.Net.Tests._J-S.csproj @@ -22,7 +22,7 @@ - + Lucene.Net.Tests._J-S Lucene.Net @@ -80,8 +80,8 @@ - + - \ No newline at end of file + diff --git a/src/Lucene.Net.Tests/Support/Threading/AtomicDoubleTest.cs b/src/Lucene.Net.Tests/Support/Threading/AtomicDoubleTest.cs new file mode 100644 index 0000000000..226b071291 --- /dev/null +++ b/src/Lucene.Net.Tests/Support/Threading/AtomicDoubleTest.cs @@ -0,0 +1,255 @@ +using Lucene.Net.Support.Threading; +using Lucene.Net.Util; +using NUnit.Framework; +using System; +using System.Threading; + +namespace Lucene.Net.Threading +{ + /* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + /// + /// This is a modified copy of J2N's TestAtomicInt64, + /// modified to test + /// + [TestFixture] + public class AtomicDoubleTest : LuceneTestCase + { + private const int LONG_DELAY_MS = 50 * 50; + + /** + * fail with message "Unexpected exception" + */ + public void unexpectedException() + { + fail("Unexpected exception"); + } + + /** + * constructor initializes to given value + */ + [Test] + public void TestConstructor() + { + AtomicDouble ai = new AtomicDouble(1.0d); + assertEquals(1.0d, ai); + } + + /** + * default constructed initializes to zero + */ + [Test] + public void TestConstructor2() + { + AtomicDouble ai = new AtomicDouble(); + assertEquals(0.0d, ai.Value); + } + + /** + * get returns the last value set + */ + [Test] + public void TestGetSet() + { + AtomicDouble ai = new AtomicDouble(1); + assertEquals(1.0d, ai); + ai.Value = 2.0d; + assertEquals(2.0d, ai); + ai.Value = -3.0d; + assertEquals(-3.0d, ai); + + } + + /** + * compareAndSet succeeds in changing value if equal to expected else fails + */ + [Test] + public void TestCompareAndSet() + { + AtomicDouble ai = new AtomicDouble(1.0d); + assertTrue(ai.CompareAndSet(1.0d, 2.0d)); + assertTrue(ai.CompareAndSet(2.0d, -4.0d)); + assertEquals(-4.0d, ai.Value); + assertFalse(ai.CompareAndSet(-5.0d, 7.0d)); + assertFalse(7.0d.Equals(ai.Value)); + assertTrue(ai.CompareAndSet(-4.0d, 7.0d)); + assertEquals(7.0d, ai.Value); + } + + /** + * compareAndSet in one thread enables another waiting for value + * to succeed + */ + [Test] + public void TestCompareAndSetInMultipleThreads() + { + AtomicDouble ai = new AtomicDouble(1.0d); + Thread t = new Thread(() => + { + while (!ai.CompareAndSet(2.0d, 3.0d)) Thread.Yield(); + }); + try + { + t.Start(); + assertTrue(ai.CompareAndSet(1.0d, 2.0d)); + t.Join(LONG_DELAY_MS); + assertFalse(t.IsAlive); + assertEquals(ai.Value, 3.0d); + } + catch (Exception /*e*/) + { + unexpectedException(); + } + } + + // /** + // * repeated weakCompareAndSet succeeds in changing value when equal + // * to expected + // */ + //[Test] + // public void TestWeakCompareAndSet() + //{ + // AtomicDouble ai = new AtomicDouble(1); + // while (!ai.WeakCompareAndSet(1, 2)) ; + // while (!ai.WeakCompareAndSet(2, -4)) ; + // assertEquals(-4, ai.Value); + // while (!ai.WeakCompareAndSet(-4, 7)) ; + // assertEquals(7, ai.Value); + //} + + /** + * getAndSet returns previous value and sets to given value + */ + [Test] + public void TestGetAndSet() + { + AtomicDouble ai = new AtomicDouble(1.0d); + assertEquals(1.0d, ai.GetAndSet(0.0d)); + assertEquals(0.0d, ai.GetAndSet(-10.0d)); + assertEquals(-10.0d, ai.GetAndSet(1.0d)); + } + +#if FEATURE_SERIALIZABLE + /** + * a deserialized serialized atomic holds same value + */ + [Test] + public void TestSerialization() + { + AtomicDouble l = new AtomicDouble(); + + try + { + l.Value = 22.0d; + AtomicDouble r = Clone(l); + assertEquals(l.Value, r.Value); + } + catch (Exception /*e*/) + { + unexpectedException(); + } + } +#endif + + /** + * toString returns current value. + */ + [Test] + public void TestToString() + { + AtomicDouble ai = new AtomicDouble(); + for (double i = -12.0d; i < 6.0d; i += 1.0d) + { + ai.Value = i; + assertEquals(ai.ToString(), J2N.Numerics.Double.ToString(i)); + } + } + + /** + * intValue returns current value. + */ + [Test] + public void TestIntValue() + { + AtomicDouble ai = new AtomicDouble(); + for (double i = -12.0d; i < 6.0d; ++i) + { + ai.Value = i; + assertEquals((int)i, Convert.ToInt32(ai)); + } + } + + + /** + * longValue returns current value. + */ + [Test] + public void TestLongValue() + { + AtomicDouble ai = new AtomicDouble(); + for (double i = -12.0d; i < 6.0d; ++i) + { + ai.Value = i; + assertEquals((long)i, Convert.ToInt64(ai)); + } + } + + /** + * floatValue returns current value. + */ + [Test] + public void TestFloatValue() + { + AtomicDouble ai = new AtomicDouble(); + for (double i = -12.0d; i < 6.0d; ++i) + { + ai.Value = i; + assertEquals((float)i, Convert.ToSingle(ai)); + } + } + + /** + * doubleValue returns current value. + */ + [Test] + public void TestDoubleValue() + { + AtomicDouble ai = new AtomicDouble(); + for (double i = -12.0d; i < 6.0d; ++i) + { + ai.Value = i; + assertEquals((double)i, Convert.ToDouble(ai)); + } + } + + /** + * doubleValue returns current value. + */ + [Test] + public void TestComparisonOperators() + { + AtomicDouble ai = new AtomicDouble(6.0d); + assertTrue(5.0d < ai); + assertTrue(9.0d > ai); + assertTrue(ai > 4.0d); + assertTrue(ai < 7.0d); + assertFalse(ai < 6.0d); + assertTrue(ai <= 6.0d); + } + } +} diff --git a/src/Lucene.Net/Index/IndexWriter.cs b/src/Lucene.Net/Index/IndexWriter.cs index 1549e82d2d..f98cdd709f 100644 --- a/src/Lucene.Net/Index/IndexWriter.cs +++ b/src/Lucene.Net/Index/IndexWriter.cs @@ -228,13 +228,14 @@ public class IndexWriter : IDisposable, ITwoPhaseCommit private readonly Directory directory; // where this index resides private readonly Analyzer analyzer; // how to analyze text - private long changeCount; // increments every time a change is completed - private long lastCommitChangeCount; // last changeCount that was committed + // LUCENENET specific - since we don't have `volatile long` in .NET, we use AtomicInt64 + private readonly AtomicInt64 changeCount = new AtomicInt64(); // increments every time a change is completed + private readonly AtomicInt64 lastCommitChangeCount = new AtomicInt64(); // last changeCount that was committed private IList rollbackSegments; // list of segmentInfo we will fallback to if the commit fails internal volatile SegmentInfos pendingCommit; // set when a commit is pending (after prepareCommit() & before commit()) - internal long pendingCommitChangeCount; + internal readonly AtomicInt64 pendingCommitChangeCount = new AtomicInt64(); // LUCENENET specific - since we don't have `volatile long` in .NET, we use AtomicInt64 private ICollection filesToCommit; @@ -2197,7 +2198,7 @@ internal string NewSegmentName() // could close, re-open and re-return the same segment // name that was previously returned which can cause // problems at least with ConcurrentMergeScheduler. - changeCount++; + _ = changeCount.IncrementAndGet(); segmentInfos.Changed(); return "_" + SegmentInfos.SegmentNumberToString(segmentInfos.Counter++, allowLegacyNames: false); // LUCENENET specific - we had this right thru all of the betas, so don't change if the legacy feature is enabled } @@ -2821,7 +2822,7 @@ private void RollbackInternal() deleter.Checkpoint(segmentInfos, false); deleter.Refresh(); - lastCommitChangeCount = changeCount; + lastCommitChangeCount.Value = changeCount.Value; deleter.Refresh(); deleter.Dispose(); @@ -2957,7 +2958,7 @@ public virtual void DeleteAll() // Don't bother saving any changes in our segmentInfos readerPool.DropAll(false); // Mark that the index has changed - ++changeCount; + _ = changeCount.IncrementAndGet(); segmentInfos.Changed(); globalFieldNumberMap.Clear(); success = true; @@ -3128,7 +3129,7 @@ internal virtual void CheckpointNoSIS() UninterruptableMonitor.Enter(this); try { - changeCount++; + _ = changeCount.IncrementAndGet(); deleter.Checkpoint(segmentInfos, false); } finally @@ -3144,7 +3145,7 @@ internal void Changed() UninterruptableMonitor.Enter(this); try { - changeCount++; + _ = changeCount.IncrementAndGet(); segmentInfos.Changed(); } finally @@ -3935,7 +3936,7 @@ private void PrepareCommitInternal() // sneak into the commit point: toCommit = (SegmentInfos)segmentInfos.Clone(); - pendingCommitChangeCount = changeCount; + pendingCommitChangeCount.Value = changeCount.Value; // this protects the segmentInfos we are now going // to commit. this is important in case, eg, while @@ -4027,7 +4028,7 @@ public void SetCommitData(IDictionary commitUserData) try { segmentInfos.UserData = new Dictionary(commitUserData); - ++changeCount; + _ = changeCount.IncrementAndGet(); } finally { @@ -4170,7 +4171,7 @@ private void FinishCommit() infoStream.Message("IW", "commit: wrote segments file \"" + pendingCommit.GetSegmentsFileName() + "\""); } segmentInfos.UpdateGeneration(pendingCommit); - lastCommitChangeCount = pendingCommitChangeCount; + lastCommitChangeCount.Value = pendingCommitChangeCount.Value; rollbackSegments = pendingCommit.CreateBackupSegmentInfos(); // NOTE: don't use this.checkpoint() here, because // we do not want to increment changeCount: @@ -5144,8 +5145,8 @@ internal bool RegisterMerge(MergePolicy.OneMerge merge) int delCount = NumDeletedDocs(info); if (Debugging.AssertsEnabled) Debugging.Assert(delCount <= info.Info.DocCount); double delRatio = ((double)delCount) / info.Info.DocCount; - merge.EstimatedMergeBytes += (long)(info.GetSizeInBytes() * (1.0 - delRatio)); - merge.totalMergeBytes += info.GetSizeInBytes(); + _ = merge.estimatedMergeBytes.AddAndGet((long)(info.GetSizeInBytes() * (1.0 - delRatio))); + _ = merge.totalMergeBytes.AddAndGet(info.GetSizeInBytes()); } } diff --git a/src/Lucene.Net/Index/IndexWriterConfig.cs b/src/Lucene.Net/Index/IndexWriterConfig.cs index 7985d47327..8ac9cdbe9f 100644 --- a/src/Lucene.Net/Index/IndexWriterConfig.cs +++ b/src/Lucene.Net/Index/IndexWriterConfig.cs @@ -293,8 +293,8 @@ public object Clone() // so must declare it new. See: http://stackoverflow.com/q/82437 new public long WriteLockTimeout { - get => writeLockTimeout; - set => this.writeLockTimeout = value; + get => writeLockTimeout.Value; + set => writeLockTimeout.Value = value; } /// diff --git a/src/Lucene.Net/Index/LiveIndexWriterConfig.cs b/src/Lucene.Net/Index/LiveIndexWriterConfig.cs index 6dd186cbb6..f81aa7773d 100644 --- a/src/Lucene.Net/Index/LiveIndexWriterConfig.cs +++ b/src/Lucene.Net/Index/LiveIndexWriterConfig.cs @@ -1,4 +1,6 @@ -using Lucene.Net.Util; +using J2N.Threading.Atomic; +using Lucene.Net.Support.Threading; +using Lucene.Net.Util; using System; using System.Text; @@ -40,7 +42,7 @@ public class LiveIndexWriterConfig private readonly Analyzer analyzer; private volatile int maxBufferedDocs; - private double ramBufferSizeMB; + private readonly AtomicDouble ramBufferSizeMB; // LUCENENET specific: Changed from volatile double to AtomicDouble private volatile int maxBufferedDeleteTerms; private volatile int readerTermsIndexDivisor; private volatile IndexReaderWarmer mergedSegmentWarmer; @@ -48,7 +50,7 @@ public class LiveIndexWriterConfig // LUCENENET specific: Volatile fields are not CLS compliant, // so we are making them internal. This class cannot be inherited - // from outside of the assembly anyway, since it has no public + // from outside of the assembly anyway, since it has no public // constructors, so protected members are moot. // modified by IndexWriterConfig @@ -80,7 +82,8 @@ public class LiveIndexWriterConfig /// /// Timeout when trying to obtain the write lock on init. - internal long writeLockTimeout; + // LUCENENET specific - since we don't have `volatile long` in .NET, we use AtomicInt64 + internal readonly AtomicInt64 writeLockTimeout; /// /// that determines how documents are @@ -139,7 +142,7 @@ internal LiveIndexWriterConfig(Analyzer analyzer, LuceneVersion matchVersion) { this.analyzer = analyzer; this.matchVersion = matchVersion; - ramBufferSizeMB = IndexWriterConfig.DEFAULT_RAM_BUFFER_SIZE_MB; + ramBufferSizeMB = new AtomicDouble(IndexWriterConfig.DEFAULT_RAM_BUFFER_SIZE_MB); maxBufferedDocs = IndexWriterConfig.DEFAULT_MAX_BUFFERED_DOCS; maxBufferedDeleteTerms = IndexWriterConfig.DEFAULT_MAX_BUFFERED_DELETE_TERMS; readerTermsIndexDivisor = IndexWriterConfig.DEFAULT_READER_TERMS_INDEX_DIVISOR; @@ -151,7 +154,7 @@ internal LiveIndexWriterConfig(Analyzer analyzer, LuceneVersion matchVersion) openMode = Index.OpenMode.CREATE_OR_APPEND; similarity = IndexSearcher.DefaultSimilarity; mergeScheduler = new ConcurrentMergeScheduler(); - writeLockTimeout = IndexWriterConfig.WRITE_LOCK_TIMEOUT; + writeLockTimeout = new AtomicInt64(IndexWriterConfig.WRITE_LOCK_TIMEOUT); indexingChain = DocumentsWriterPerThread.DefaultIndexingChain; codec = Codec.Default; if (codec is null) @@ -175,7 +178,7 @@ internal LiveIndexWriterConfig(IndexWriterConfig config) maxBufferedDeleteTerms = config.MaxBufferedDeleteTerms; maxBufferedDocs = config.MaxBufferedDocs; mergedSegmentWarmer = config.MergedSegmentWarmer; - ramBufferSizeMB = config.RAMBufferSizeMB; + ramBufferSizeMB = new AtomicDouble(config.RAMBufferSizeMB); readerTermsIndexDivisor = config.ReaderTermsIndexDivisor; termIndexInterval = config.TermIndexInterval; matchVersion = config.matchVersion; @@ -185,7 +188,7 @@ internal LiveIndexWriterConfig(IndexWriterConfig config) openMode = config.OpenMode; similarity = config.Similarity; mergeScheduler = config.MergeScheduler; - writeLockTimeout = config.WriteLockTimeout; + writeLockTimeout = new AtomicInt64(config.WriteLockTimeout); indexingChain = config.IndexingChain; codec = config.Codec; infoStream = config.InfoStream; @@ -239,7 +242,7 @@ internal LiveIndexWriterConfig(IndexWriterConfig config) /// { /// //customize Lucene41PostingsFormat, passing minBlockSize=50, maxBlockSize=100 /// private readonly PostingsFormat tweakedPostings = new Lucene41PostingsFormat(50, 100); - /// + /// /// public override PostingsFormat GetPostingsFormatForField(string field) /// { /// if (field.Equals("fieldWithTonsOfTerms", StringComparison.Ordinal)) @@ -249,7 +252,7 @@ internal LiveIndexWriterConfig(IndexWriterConfig config) /// } /// } /// ... - /// + /// /// iwc.Codec = new MyLucene45Codec(); /// /// Note that other implementations may have their own parameters, or no parameters at all. @@ -351,7 +354,7 @@ public virtual double RAMBufferSizeMB { throw new ArgumentException("at least one of ramBufferSizeMB and maxBufferedDocs must be enabled"); } - this.ramBufferSizeMB = value; + this.ramBufferSizeMB.Value = value; } } @@ -587,4 +590,4 @@ public override string ToString() return sb.ToString(); } } -} \ No newline at end of file +} diff --git a/src/Lucene.Net/Index/MergePolicy.cs b/src/Lucene.Net/Index/MergePolicy.cs index efacf2e572..325eef7f2c 100644 --- a/src/Lucene.Net/Index/MergePolicy.cs +++ b/src/Lucene.Net/Index/MergePolicy.cs @@ -1,4 +1,5 @@ -using Lucene.Net.Diagnostics; +using J2N.Threading.Atomic; +using Lucene.Net.Diagnostics; using Lucene.Net.Support; using Lucene.Net.Support.Threading; using Lucene.Net.Util; @@ -128,12 +129,17 @@ public int MaxNumSegments // used by IndexWriter } private int maxNumSegments = -1; + // LUCENENET NOTE: original was volatile, using AtomicInt64 instead. + // Also, we expose the value as a `long` property instead of exposing + // the AtomicInt64 object itself. + internal readonly AtomicInt64 estimatedMergeBytes = new AtomicInt64(); + /// /// Estimated size in bytes of the merged segment. - public long EstimatedMergeBytes { get; internal set; } // used by IndexWriter // LUCENENET NOTE: original was volatile, but long cannot be in .NET + public long EstimatedMergeBytes => estimatedMergeBytes.Value; // Sum of sizeInBytes of all SegmentInfos; set by IW.mergeInit - internal long totalMergeBytes; // LUCENENET NOTE: original was volatile, but long cannot be in .NET + internal readonly AtomicInt64 totalMergeBytes = new AtomicInt64(); // LUCENENET NOTE: original was volatile, using AtomicInt64 instead internal IList readers; // used by IndexWriter @@ -413,7 +419,7 @@ public virtual string SegString(Directory dir) /// input total size. This is only set once the merge is /// initialized by . /// - public virtual long TotalBytesSize => totalMergeBytes; + public virtual long TotalBytesSize => totalMergeBytes.Value; /// /// Returns the total number of documents that are included with this merge. diff --git a/src/Lucene.Net/Index/SegmentCommitInfo.cs b/src/Lucene.Net/Index/SegmentCommitInfo.cs index 5ebf8ae023..374c108117 100644 --- a/src/Lucene.Net/Index/SegmentCommitInfo.cs +++ b/src/Lucene.Net/Index/SegmentCommitInfo.cs @@ -1,4 +1,5 @@ using J2N.Collections.Generic.Extensions; +using J2N.Threading.Atomic; using Lucene.Net.Support; using System; using System.Collections.Generic; @@ -58,7 +59,7 @@ public class SegmentCommitInfo // Track the per-generation updates files private readonly IDictionary> genUpdatesFiles = new Dictionary>(); - private long sizeInBytes = -1; // LUCENENET NOTE: This was volatile in the original, but long cannot be volatile in .NET + private readonly AtomicInt64 sizeInBytes = new AtomicInt64(-1L); // LUCENENET NOTE: This was volatile in the original, using AtomicInt64 instead /// /// Sole constructor. @@ -115,7 +116,7 @@ internal virtual void AdvanceDelGen() { delGen = nextWriteDelGen; nextWriteDelGen = delGen + 1; - sizeInBytes = -1; + sizeInBytes.Value = -1; } /// @@ -134,7 +135,7 @@ internal virtual void AdvanceFieldInfosGen() { fieldInfosGen = nextWriteFieldInfosGen; nextWriteFieldInfosGen = fieldInfosGen + 1; - sizeInBytes = -1; + sizeInBytes.Value = -1; } /// @@ -161,7 +162,7 @@ public virtual long GetSizeInBytes() { sum += Info.Dir.FileLength(fileName); } - sizeInBytes = sum; + sizeInBytes.Value = sum; } return sizeInBytes; @@ -198,7 +199,7 @@ public virtual ICollection GetFiles() internal void SetBufferedDeletesGen(long value) { bufferedDeletesGen = value; - sizeInBytes = -1; + sizeInBytes.Value = -1; } /// diff --git a/src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs b/src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs index 8c3e854bc7..95a07df8b2 100644 --- a/src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs +++ b/src/Lucene.Net/Search/ControlledRealTimeReopenThread.cs @@ -50,8 +50,13 @@ public class ControlledRealTimeReopenThread : ThreadJob, IDisposable private readonly long targetMinStaleNS; private readonly TrackingIndexWriter writer; private volatile bool finish; + + // LUCENENET note: these fields were originally volatile in Lucene, + // but since access to them is always done under a lock or with Interlocked, + // no need to make these AtomicInt64. private long waitingGen; private long searchingGen; + private readonly AtomicInt64 refreshStartGen = new AtomicInt64(); private readonly AtomicBoolean isDisposed = new AtomicBoolean(false); @@ -334,4 +339,4 @@ public override void Run() } } } -} \ No newline at end of file +} diff --git a/src/Lucene.Net/Search/TimeLimitingCollector.cs b/src/Lucene.Net/Search/TimeLimitingCollector.cs index 8a3993d20f..b116435a7a 100644 --- a/src/Lucene.Net/Search/TimeLimitingCollector.cs +++ b/src/Lucene.Net/Search/TimeLimitingCollector.cs @@ -1,4 +1,5 @@ using J2N.Threading; +using J2N.Threading.Atomic; using Lucene.Net.Support.Threading; using System; @@ -43,7 +44,7 @@ public class TimeLimitingCollector : ICollector { /// /// Thrown when elapsed search time exceeds allowed search time. - // LUCENENET: It is no longer good practice to use binary serialization. + // LUCENENET: It is no longer good practice to use binary serialization. // See: https://github.com/dotnet/corefx/issues/23584#issuecomment-325724568 #if FEATURE_SERIALIZABLE_EXCEPTIONS [Serializable] @@ -150,7 +151,7 @@ public TimeLimitingCollector(ICollector collector, Counter clock, long ticksAllo /// collector.SetBaseline(baseline); /// indexSearcher.Search(query, collector); /// - /// + /// /// /// public virtual void SetBaseline(long clockTime) @@ -173,7 +174,7 @@ public virtual void SetBaseline() /// A non greedy collector, upon a timeout, would throw a /// without allowing the wrapped collector to collect current doc. A greedy one would /// first allow the wrapped hit collector to collect current doc and only then - /// throw a . + /// throw a . /// public virtual bool IsGreedy { @@ -213,7 +214,7 @@ public virtual void SetNextReader(AtomicReaderContext context) SetBaseline(); } } - + public virtual void SetScorer(Scorer scorer) { collector.SetScorer(scorer); @@ -239,7 +240,7 @@ public virtual void SetCollector(ICollector collector) /// the global has never been accessed before. The thread /// returned from this method is started on creation and will be alive unless /// you stop the via . - /// + /// /// @lucene.experimental /// /// the global TimerThreads @@ -291,18 +292,19 @@ public sealed class TimerThread : ThreadJob // afford losing a tick or two. // // See section 17 of the Java Language Specification for details. - private readonly long time = 0; + // LUCENENET NOTE: despite the explanation above, this value is never modified. + private const long time = 0; private volatile bool stop = false; - private long resolution; + private readonly AtomicInt64 resolution; // LUCENENET: was volatile long, using AtomicInt64 instead internal readonly Counter counter; public TimerThread(long resolution, Counter counter) : base(THREAD_NAME) { - this.resolution = resolution; + this.resolution = new AtomicInt64(resolution); this.counter = counter; - this.IsBackground = (true); + this.IsBackground = true; } public TimerThread(Counter counter) @@ -319,7 +321,7 @@ public override void Run() try { - Thread.Sleep(TimeSpan.FromMilliseconds(Interlocked.Read(ref resolution))); + Thread.Sleep(TimeSpan.FromMilliseconds(resolution.Value)); } catch (Exception ie) when (ie.IsInterruptedException()) { @@ -346,7 +348,7 @@ public void StopTimer() public long Resolution { get => resolution; - set => this.resolution = Math.Max(value, 5); // 5 milliseconds is about the minimum reasonable time for a Object.wait(long) call. + set => this.resolution.Value = Math.Max(value, 5); // 5 milliseconds is about the minimum reasonable time for a Object.wait(long) call. } } } diff --git a/src/Lucene.Net/Store/RateLimiter.cs b/src/Lucene.Net/Store/RateLimiter.cs index 9e972110cc..192465e60f 100644 --- a/src/Lucene.Net/Store/RateLimiter.cs +++ b/src/Lucene.Net/Store/RateLimiter.cs @@ -1,4 +1,5 @@ using J2N; +using J2N.Threading.Atomic; using Lucene.Net.Support.Threading; using System; using System.Threading; @@ -46,7 +47,7 @@ public abstract class RateLimiter /// rate at or below the target. /// /// Note: the implementation is thread-safe - /// + /// /// /// the pause time in nano seconds public abstract long Pause(long bytes); @@ -56,9 +57,11 @@ public abstract class RateLimiter /// public class SimpleRateLimiter : RateLimiter { - private double mbPerSec; - private double nsPerByte; - private long lastNS; + // LUCENENET: these fields are volatile in Lucene, but that is not + // valid in .NET. Instead, we use AtomicInt64/AtomicDouble to ensure atomicity. + private readonly AtomicDouble mbPerSec = new AtomicDouble(); + private readonly AtomicDouble nsPerByte = new AtomicDouble(); + private readonly AtomicInt64 lastNS = new AtomicInt64(); // TODO: we could also allow eg a sub class to dynamically // determine the allowed rate, eg if an app wants to @@ -76,11 +79,11 @@ public SimpleRateLimiter(double mbPerSec) /// public override void SetMbPerSec(double mbPerSec) { - this.mbPerSec = mbPerSec; + this.mbPerSec.Value = mbPerSec; if (mbPerSec == 0) - nsPerByte = 0; + nsPerByte.Value = 0; else - nsPerByte = 1000000000.0 / (1024 * 1024 * mbPerSec); + nsPerByte.Value = 1000000000.0 / (1024 * 1024 * mbPerSec); } /// @@ -106,12 +109,12 @@ public override long Pause(long bytes) // TODO: this is purely instantaneous rate; maybe we // should also offer decayed recent history one? - var targetNS = lastNS = lastNS + ((long)(bytes * nsPerByte)); + var targetNS = /*lastNS =*/ lastNS.AddAndGet((long)(bytes * nsPerByte)); long startNS; var curNS = startNS = Time.NanoTime() /* ns */; if (lastNS < curNS) { - lastNS = curNS; + lastNS.Value = curNS; } // While loop because Thread.sleep doesn't always sleep @@ -139,4 +142,4 @@ public override long Pause(long bytes) } } } -} \ No newline at end of file +} diff --git a/src/Lucene.Net/Support/Threading/AtomicDouble.cs b/src/Lucene.Net/Support/Threading/AtomicDouble.cs new file mode 100644 index 0000000000..814cf0fe9e --- /dev/null +++ b/src/Lucene.Net/Support/Threading/AtomicDouble.cs @@ -0,0 +1,413 @@ +#region Copyright 2010 by Apache Harmony, Licensed under the Apache License, Version 2.0 +/* Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#endregion + +using J2N; +using J2N.Numerics; +using System; +using System.Diagnostics; +using System.Threading; + +#nullable enable + +namespace Lucene.Net.Support.Threading +{ + /// + /// A value that may be updated atomically. + /// An is used in applications such as atomically + /// stored and retrieved values, and cannot be used as a replacement + /// for a . However, this class does + /// implement implicit conversion to , so it can + /// be utilized with language features, tools and utilities that deal + /// with numerical operations. + /// + /// NOTE: This is a modified version of to support values. + /// It does not have the increment, decrement, and add methods because those operations are not atomic + /// due to the conversion to/from . + /// +#if FEATURE_SERIALIZABLE + [Serializable] +#endif + [DebuggerDisplay("{Value}")] + internal class AtomicDouble : Number, IEquatable, IEquatable, IFormattable, IConvertible + { + private long value; + + /// + /// Creates a new with the default initial value, 0. + /// + public AtomicDouble() + : this(0d) + { } + + /// + /// Creates a new with the given initial . + /// + /// The initial value. + public AtomicDouble(double value) + { + this.value = BitConversion.DoubleToInt64Bits(value); + } + + /// + /// Gets or sets the current value. Note that these operations can be done + /// implicitly by setting the to a . + /// + /// AtomicDouble aDouble = new AtomicDouble(4.0); + /// double x = aDouble; + /// + /// + /// + /// Properties are inherently not atomic. Operators such as += and -= should not + /// be used on because they perform both a separate get and a set operation. + /// + public double Value + { + get => BitConversion.Int64BitsToDouble(Interlocked.Read(ref this.value)); + set => Interlocked.Exchange(ref this.value, BitConversion.DoubleToInt64Bits(value)); + } + + /// + /// Atomically sets to the given value and returns the old value. + /// + /// The new value. + /// The previous value. + public double GetAndSet(double newValue) + { + return BitConversion.Int64BitsToDouble(Interlocked.Exchange(ref value, BitConversion.DoubleToInt64Bits(newValue))); + } + + /// + /// Atomically sets the value to the given updated value + /// if the current value equals the expected value. + /// + /// The expected value (the comparand). + /// The new value that will be set if the current value equals the expected value. + /// true if successful. A false return value indicates that the actual value + /// was not equal to the expected value. + public bool CompareAndSet(double expect, double update) + { + long expectLong = BitConversion.DoubleToInt64Bits(expect); + long updateLong = BitConversion.DoubleToInt64Bits(update); + long rc = Interlocked.CompareExchange(ref value, updateLong, expectLong); + return rc == expectLong; + } + + /// + /// Determines whether the specified is equal to the current . + /// + /// The to compare with the current . + /// true if is equal to the current ; otherwise, false. + public bool Equals(AtomicDouble? other) + { + if (other is null) + return false; + + // NOTE: comparing long values rather than floating point comparison + return value == other.value; + } + + /// + /// Determines whether the specified is equal to the current . + /// + /// The to compare with the current . + /// true if is equal to the current ; otherwise, false. + public bool Equals(double other) + { + // NOTE: comparing long values rather than floating point comparison + return value == BitConversion.DoubleToInt64Bits(other); + } + + /// + /// Determines whether the specified is equal to the current . + /// + /// If is a , the comparison is not done atomically. + /// + /// The to compare with the current . + /// true if is equal to the current ; otherwise, false. + public override bool Equals(object? other) + { + if (other is AtomicDouble ai) + return Equals(ai); + if (other is double i) + return Equals(i); + return false; + } + + /// + /// Returns the hash code for this instance. + /// + /// A 32-bit signed integer hash code. + public override int GetHashCode() + { + return Value.GetHashCode(); + } + + /// + /// Converts the numeric value of this instance to its equivalent string representation. + /// + /// The string representation of the value of this instance, consisting of + /// a negative sign if the value is negative, + /// and a sequence of digits ranging from 0 to 9 with no leading zeroes. + public override string ToString() + { + return J2N.Numerics.Double.ToString(Value); + } + + /// + /// Converts the numeric value of this instance to its equivalent string representation, + /// using the specified . + /// + /// A standard or custom numeric format string. + /// The string representation of the value of this instance as specified + /// by . + public override string ToString(string? format) + { + return J2N.Numerics.Double.ToString(Value, format); + } + + /// + /// Converts the numeric value of this instance to its equivalent string representation + /// using the specified culture-specific format information. + /// + /// An object that supplies culture-specific formatting information. + /// The string representation of the value of this instance as specified by . + public override string ToString(IFormatProvider? provider) + { + return J2N.Numerics.Double.ToString(Value, provider); + } + + /// + /// Converts the numeric value of this instance to its equivalent string representation using the + /// specified format and culture-specific format information. + /// + /// A standard or custom numeric format string. + /// An object that supplies culture-specific formatting information. + /// The string representation of the value of this instance as specified by + /// and . + public override string ToString(string? format, IFormatProvider? provider) + { + return J2N.Numerics.Double.ToString(Value, format, provider); + } + + #region IConvertible Members + + /// + public override byte ToByte() + { + return (byte)Value; + } + + /// + public override sbyte ToSByte() + { + return (sbyte)Value; + } + + /// + public override double ToDouble() + { + return Value; + } + + /// + public override float ToSingle() + { + return (float)Value; + } + + /// + public override int ToInt32() + { + return (int)Value; + } + + /// + public override long ToInt64() + { + return (long)Value; + } + + /// + public override short ToInt16() + { + return (short)Value; + } + + /// + /// Returns the for value type . + /// + /// + public TypeCode GetTypeCode() => ((IConvertible)Value).GetTypeCode(); + + bool IConvertible.ToBoolean(IFormatProvider? provider) => Convert.ToBoolean(Value); + + byte IConvertible.ToByte(IFormatProvider? provider) => Convert.ToByte(Value); + + char IConvertible.ToChar(IFormatProvider? provider) => Convert.ToChar(Value); + + DateTime IConvertible.ToDateTime(IFormatProvider? provider) => Convert.ToDateTime(Value); + + decimal IConvertible.ToDecimal(IFormatProvider? provider) => Convert.ToDecimal(Value); + + double IConvertible.ToDouble(IFormatProvider? provider) => Value; + + short IConvertible.ToInt16(IFormatProvider? provider) => Convert.ToInt16(Value); + + int IConvertible.ToInt32(IFormatProvider? provider) => Convert.ToInt32(Value); + + long IConvertible.ToInt64(IFormatProvider? provider) => Convert.ToInt64(Value); + + sbyte IConvertible.ToSByte(IFormatProvider? provider) => Convert.ToSByte(Value); + + float IConvertible.ToSingle(IFormatProvider? provider) => Convert.ToSingle(Value); + + object IConvertible.ToType(Type conversionType, IFormatProvider? provider) => ((IConvertible)Value).ToType(conversionType, provider); + + ushort IConvertible.ToUInt16(IFormatProvider? provider) => Convert.ToUInt16(Value); + + uint IConvertible.ToUInt32(IFormatProvider? provider) => Convert.ToUInt32(Value); + + ulong IConvertible.ToUInt64(IFormatProvider? provider) => Convert.ToUInt64(Value); + + #endregion IConvertible Members + + #region Operator Overrides + + /// + /// Implicitly converts an to a . + /// + /// The to convert. + public static implicit operator double(AtomicDouble atomicInt64) + { + return atomicInt64.Value; + } + + /// + /// Compares and for value equality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are equal; otherwise, false. + public static bool operator ==(AtomicDouble a1, AtomicDouble a2) + { + // NOTE: comparing long values rather than floating point comparison + return a1.value == a2.value; + } + + /// + /// Compares and for value inequality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are not equal; otherwise, false. + public static bool operator !=(AtomicDouble a1, AtomicDouble a2) + { + return !(a1 == a2); + } + + /// + /// Compares and for value equality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are equal; otherwise, false. + public static bool operator ==(AtomicDouble a1, double a2) + { + return a1.Value.Equals(a2); + } + + /// + /// Compares and for value inequality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are not equal; otherwise, false. + public static bool operator !=(AtomicDouble a1, double a2) + { + return !(a1 == a2); + } + + /// + /// Compares and for value equality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are equal; otherwise, false. + public static bool operator ==(double a1, AtomicDouble a2) + { + return a1.Equals(a2.Value); + } + + /// + /// Compares and for value inequality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are not equal; otherwise, false. + public static bool operator !=(double a1, AtomicDouble a2) + { + return !(a1 == a2); + } + + /// + /// Compares and for value equality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are equal; otherwise, false. + public static bool operator ==(AtomicDouble a1, double? a2) + { + return a1.Value.Equals(a2.GetValueOrDefault()); + } + + /// + /// Compares and for value inequality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are not equal; otherwise, false. + public static bool operator !=(AtomicDouble a1, double? a2) + { + return !(a1 == a2); + } + + /// + /// Compares and for value equality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are equal; otherwise, false. + public static bool operator ==(double? a1, AtomicDouble a2) + { + return a1.GetValueOrDefault().Equals(a2.Value); + } + + /// + /// Compares and for value inequality. + /// + /// The first number. + /// The second number. + /// true if the given numbers are not equal; otherwise, false. + public static bool operator !=(double? a1, AtomicDouble a2) + { + return !(a1 == a2); + } + + #endregion Operator Overrides + } +} From d3887853727b9c949b2a7295afa05c1631bfa152 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Fri, 13 Dec 2024 10:19:49 -0700 Subject: [PATCH 2/8] PR feedback --- .../Support/Threading/AtomicDouble.cs | 58 +++++++++++++------ 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/src/Lucene.Net/Support/Threading/AtomicDouble.cs b/src/Lucene.Net/Support/Threading/AtomicDouble.cs index 814cf0fe9e..62225e7ab9 100644 --- a/src/Lucene.Net/Support/Threading/AtomicDouble.cs +++ b/src/Lucene.Net/Support/Threading/AtomicDouble.cs @@ -39,6 +39,10 @@ namespace Lucene.Net.Support.Threading /// It does not have the increment, decrement, and add methods because those operations are not atomic /// due to the conversion to/from . /// + /// + /// Note that this class is set up to mimic double in Java, rather than the J2N class. + /// This may cause differences in comparing NaN values. + /// #if FEATURE_SERIALIZABLE [Serializable] #endif @@ -60,7 +64,7 @@ public AtomicDouble() /// The initial value. public AtomicDouble(double value) { - this.value = BitConversion.DoubleToInt64Bits(value); + this.value = BitConversion.DoubleToRawInt64Bits(value); } /// @@ -78,7 +82,7 @@ public AtomicDouble(double value) public double Value { get => BitConversion.Int64BitsToDouble(Interlocked.Read(ref this.value)); - set => Interlocked.Exchange(ref this.value, BitConversion.DoubleToInt64Bits(value)); + set => Interlocked.Exchange(ref this.value, BitConversion.DoubleToRawInt64Bits(value)); } /// @@ -88,7 +92,7 @@ public double Value /// The previous value. public double GetAndSet(double newValue) { - return BitConversion.Int64BitsToDouble(Interlocked.Exchange(ref value, BitConversion.DoubleToInt64Bits(newValue))); + return BitConversion.Int64BitsToDouble(Interlocked.Exchange(ref value, BitConversion.DoubleToRawInt64Bits(newValue))); } /// @@ -101,8 +105,8 @@ public double GetAndSet(double newValue) /// was not equal to the expected value. public bool CompareAndSet(double expect, double update) { - long expectLong = BitConversion.DoubleToInt64Bits(expect); - long updateLong = BitConversion.DoubleToInt64Bits(update); + long expectLong = BitConversion.DoubleToRawInt64Bits(expect); + long updateLong = BitConversion.DoubleToRawInt64Bits(update); long rc = Interlocked.CompareExchange(ref value, updateLong, expectLong); return rc == expectLong; } @@ -118,7 +122,7 @@ public bool Equals(AtomicDouble? other) return false; // NOTE: comparing long values rather than floating point comparison - return value == other.value; + return Interlocked.Read(ref value) == Interlocked.Read(ref other.value); } /// @@ -129,7 +133,7 @@ public bool Equals(AtomicDouble? other) public bool Equals(double other) { // NOTE: comparing long values rather than floating point comparison - return value == BitConversion.DoubleToInt64Bits(other); + return Interlocked.Read(ref value) == BitConversion.DoubleToRawInt64Bits(other)); } /// @@ -303,10 +307,14 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are equal; otherwise, false. - public static bool operator ==(AtomicDouble a1, AtomicDouble a2) + public static bool operator ==(AtomicDouble? a1, AtomicDouble? a2) { - // NOTE: comparing long values rather than floating point comparison - return a1.value == a2.value; + if (a1 is null) + return a2 is null; + if (a2 is null) + return false; + + return a1.Equals(a2); } /// @@ -315,7 +323,7 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are not equal; otherwise, false. - public static bool operator !=(AtomicDouble a1, AtomicDouble a2) + public static bool operator !=(AtomicDouble? a1, AtomicDouble? a2) { return !(a1 == a2); } @@ -326,8 +334,11 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are equal; otherwise, false. - public static bool operator ==(AtomicDouble a1, double a2) + public static bool operator ==(AtomicDouble? a1, double a2) { + if (a1 is null) + return false; + return a1.Value.Equals(a2); } @@ -337,7 +348,7 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are not equal; otherwise, false. - public static bool operator !=(AtomicDouble a1, double a2) + public static bool operator !=(AtomicDouble? a1, double a2) { return !(a1 == a2); } @@ -348,8 +359,11 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are equal; otherwise, false. - public static bool operator ==(double a1, AtomicDouble a2) + public static bool operator ==(double a1, AtomicDouble? a2) { + if (a2 is null) + return false; + return a1.Equals(a2.Value); } @@ -359,7 +373,7 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are not equal; otherwise, false. - public static bool operator !=(double a1, AtomicDouble a2) + public static bool operator !=(double a1, AtomicDouble? a2) { return !(a1 == a2); } @@ -370,8 +384,11 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are equal; otherwise, false. - public static bool operator ==(AtomicDouble a1, double? a2) + public static bool operator ==(AtomicDouble? a1, double? a2) { + if (a1 is null) + return !a2.HasValue; + return a1.Value.Equals(a2.GetValueOrDefault()); } @@ -381,7 +398,7 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are not equal; otherwise, false. - public static bool operator !=(AtomicDouble a1, double? a2) + public static bool operator !=(AtomicDouble? a1, double? a2) { return !(a1 == a2); } @@ -392,8 +409,11 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are equal; otherwise, false. - public static bool operator ==(double? a1, AtomicDouble a2) + public static bool operator ==(double? a1, AtomicDouble? a2) { + if (!a1.HasValue) + return a2 is null; + return a1.GetValueOrDefault().Equals(a2.Value); } @@ -403,7 +423,7 @@ public static implicit operator double(AtomicDouble atomicInt64) /// The first number. /// The second number. /// true if the given numbers are not equal; otherwise, false. - public static bool operator !=(double? a1, AtomicDouble a2) + public static bool operator !=(double? a1, AtomicDouble? a2) { return !(a1 == a2); } From c435fb2b91d5977f3435474e513ae986736c0313 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Fri, 13 Dec 2024 10:26:09 -0700 Subject: [PATCH 3/8] PR feedback take 2 --- src/Lucene.Net/Support/Threading/AtomicDouble.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Lucene.Net/Support/Threading/AtomicDouble.cs b/src/Lucene.Net/Support/Threading/AtomicDouble.cs index 62225e7ab9..20e5f1aa4b 100644 --- a/src/Lucene.Net/Support/Threading/AtomicDouble.cs +++ b/src/Lucene.Net/Support/Threading/AtomicDouble.cs @@ -364,7 +364,7 @@ public static implicit operator double(AtomicDouble atomicInt64) if (a2 is null) return false; - return a1.Equals(a2.Value); + return a2.Equals(a1); } /// @@ -411,10 +411,12 @@ public static implicit operator double(AtomicDouble atomicInt64) /// true if the given numbers are equal; otherwise, false. public static bool operator ==(double? a1, AtomicDouble? a2) { - if (!a1.HasValue) + if (a1 is null) return a2 is null; + if (a2 is null) + return false; - return a1.GetValueOrDefault().Equals(a2.Value); + return a2.Equals(a1.Value); } /// From 0bdf0ff948d8e0c4184aba4a27fe065fc4eac815 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Fri, 13 Dec 2024 10:27:43 -0700 Subject: [PATCH 4/8] PR feedback take 3 --- src/Lucene.Net/Support/Threading/AtomicDouble.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Lucene.Net/Support/Threading/AtomicDouble.cs b/src/Lucene.Net/Support/Threading/AtomicDouble.cs index 20e5f1aa4b..a5525f2098 100644 --- a/src/Lucene.Net/Support/Threading/AtomicDouble.cs +++ b/src/Lucene.Net/Support/Threading/AtomicDouble.cs @@ -387,9 +387,11 @@ public static implicit operator double(AtomicDouble atomicInt64) public static bool operator ==(AtomicDouble? a1, double? a2) { if (a1 is null) - return !a2.HasValue; + return a2 is null; + if (a2 is null) + return false; - return a1.Value.Equals(a2.GetValueOrDefault()); + return a1.Value.Equals(a2.Value); } /// From 6e57d830c905647554ba9486768c730a3114bfe4 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Fri, 13 Dec 2024 10:30:15 -0700 Subject: [PATCH 5/8] Fix syntax error typo --- src/Lucene.Net/Support/Threading/AtomicDouble.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lucene.Net/Support/Threading/AtomicDouble.cs b/src/Lucene.Net/Support/Threading/AtomicDouble.cs index a5525f2098..4436d535e6 100644 --- a/src/Lucene.Net/Support/Threading/AtomicDouble.cs +++ b/src/Lucene.Net/Support/Threading/AtomicDouble.cs @@ -133,7 +133,7 @@ public bool Equals(AtomicDouble? other) public bool Equals(double other) { // NOTE: comparing long values rather than floating point comparison - return Interlocked.Read(ref value) == BitConversion.DoubleToRawInt64Bits(other)); + return Interlocked.Read(ref value) == BitConversion.DoubleToRawInt64Bits(other); } /// From 553dede61fd202cb56d8a00068d665bf3e7c2a08 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Fri, 13 Dec 2024 19:55:53 -0700 Subject: [PATCH 6/8] PR feedback --- src/Lucene.Net/Support/Threading/AtomicDouble.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Lucene.Net/Support/Threading/AtomicDouble.cs b/src/Lucene.Net/Support/Threading/AtomicDouble.cs index 4436d535e6..f62789e643 100644 --- a/src/Lucene.Net/Support/Threading/AtomicDouble.cs +++ b/src/Lucene.Net/Support/Threading/AtomicDouble.cs @@ -339,7 +339,7 @@ public static implicit operator double(AtomicDouble atomicInt64) if (a1 is null) return false; - return a1.Value.Equals(a2); + return a1.Equals(a2); } /// @@ -391,7 +391,7 @@ public static implicit operator double(AtomicDouble atomicInt64) if (a2 is null) return false; - return a1.Value.Equals(a2.Value); + return a1.Equals(a2.Value); } /// From cc2650aab58c04dcda1fa338e1512eb45e9469d7 Mon Sep 17 00:00:00 2001 From: Shad Storhaug Date: Sat, 14 Dec 2024 14:21:53 +0700 Subject: [PATCH 7/8] Lucene.Net.Support.Threading.AtomicDouble: Changed parameter and local variable names to reflect AtomicDouble rather than AtomicInt64 --- src/Lucene.Net/Support/Threading/AtomicDouble.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Lucene.Net/Support/Threading/AtomicDouble.cs b/src/Lucene.Net/Support/Threading/AtomicDouble.cs index f62789e643..e46347271c 100644 --- a/src/Lucene.Net/Support/Threading/AtomicDouble.cs +++ b/src/Lucene.Net/Support/Threading/AtomicDouble.cs @@ -1,4 +1,4 @@ -#region Copyright 2010 by Apache Harmony, Licensed under the Apache License, Version 2.0 +#region Copyright 2010 by Apache Harmony, Licensed under the Apache License, Version 2.0 /* Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with * this work for additional information regarding copyright ownership. @@ -145,10 +145,10 @@ public bool Equals(double other) /// true if is equal to the current ; otherwise, false. public override bool Equals(object? other) { - if (other is AtomicDouble ai) - return Equals(ai); - if (other is double i) - return Equals(i); + if (other is AtomicDouble ad) + return Equals(ad); + if (other is double d) + return Equals(d); return false; } @@ -295,10 +295,10 @@ public override short ToInt16() /// /// Implicitly converts an to a . /// - /// The to convert. - public static implicit operator double(AtomicDouble atomicInt64) + /// The to convert. + public static implicit operator double(AtomicDouble atomicDouble) { - return atomicInt64.Value; + return atomicDouble.Value; } /// From 59b474f2776af6a040ea375d80b36fc74ef9ae63 Mon Sep 17 00:00:00 2001 From: Shad Storhaug Date: Sat, 14 Dec 2024 14:52:15 +0700 Subject: [PATCH 8/8] Lucene.Net.Support.Threading.AtomicDouble: Added missing TryFormat override --- .../Support/Threading/AtomicDouble.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Lucene.Net/Support/Threading/AtomicDouble.cs b/src/Lucene.Net/Support/Threading/AtomicDouble.cs index e46347271c..3dce80b217 100644 --- a/src/Lucene.Net/Support/Threading/AtomicDouble.cs +++ b/src/Lucene.Net/Support/Threading/AtomicDouble.cs @@ -208,6 +208,26 @@ public override string ToString(string? format, IFormatProvider? provider) return J2N.Numerics.Double.ToString(Value, format, provider); } + #region TryFormat + + /// + /// Tries to format the value of the current double instance into the provided span of characters. + /// + /// The span in which to write this instance's value formatted as a span of characters. + /// When this method returns, contains the number of characters that were written in + /// . + /// A span containing the characters that represent a standard or custom format string that + /// defines the acceptable format for . + /// An optional object that supplies culture-specific formatting information for + /// . + /// true if the formatting was successful; otherwise, false. + public override bool TryFormat(Span destination, out int charsWritten, ReadOnlySpan format = default, IFormatProvider? provider = null) + { + return J2N.Numerics.Double.TryFormat(Value, destination, out charsWritten, format, provider); + } + + #endregion + #region IConvertible Members ///