Skip to content

Commit 29ddb24

Browse files
SWEEP: Upgraded to the navigable APIs on SortedSet<T> and SortedDictionary<TKey, TValue> (#1268)
* Lucene.Net.Codecs.SimpleText.SimpleTextTermVectorsReader.SimpleTVTermsEnum: Use GetViewAfter() to match the upstream code. * Lucene.Net.Codecs.Compressing.CompressingTermVectorsWriter: Use SortedSet<T>.TryGetLast() rather than Max, since the latter is obsolete. Fixed implementation to throw InvalidOperationException in the same case that Java would throw a NoSuchElementException (when the collection is empty). * Lucene.Net.Grouping.AbstractFirstPassGroupingCollector: Removed LINQ calls and replaced them with navigable collection APIs, which will reduce allocations. * Lucene.Net.Grouping.AbstractGrouFacetCollector: Converted calls to the obsolete SortedSet<T>.Max property to calls to RemoveLast() and TryGetLast(). * Lucene.Net.Grouping.SearchGroup.GroupMerger<T>: Replaced calls to the obsolete SortedSet<T> properties Min and Max with RemoveFirst() and RemoveLast() * Lucene.Net.Search.Suggest.Analyzing.BlendedInfixSuggester: Replaced call to obsolete SortedSet<T>.Min property with TryGetFirst(). * Lucene.Net.Util.Fst.Util: Replaced calls to obsolete SortedSet<T> properties Min and Max with RemoveFirst(), RemoveLast() and TryGetLast() * Lucene.Net.Analysis.MockCharFilter: Replaced call to obsolete SortedSet<T>.TryGetPredecessor() overload with the new overload. * Lucene.Net.Search.PostingsHighlight.PostingsHighlighter: Replaced call to obsolete SortedSet<T>.GetViewBetween() method and replaced with GetView(). * Lucene.Net.Codecs.RAMOnly.RamOnlyPostingsFormat.RAMTermsEnum: Changed implementation to use GetViewAfter() and TryGetLast() of SortedDictionary<TKey, TValue> rather than resorting to LINQ calls. * Lucene.Net.Codecs.Lucene40.Lucene40DocValuesWriter: Added comment defending the use of LINQ for this specific case rather than using GetViewBefore(). * Update src/Lucene.Net.Grouping/AbstractFirstPassGroupingCollector.cs Added discard as per PR feedback Co-authored-by: Paul Irwin <paulirwin@gmail.com> * Update src/Lucene.Net.Grouping/AbstractGroupFacetCollector.cs Added discard as per PR feedback Co-authored-by: Paul Irwin <paulirwin@gmail.com> --------- Co-authored-by: Paul Irwin <paulirwin@gmail.com>
1 parent dff28e7 commit 29ddb24

11 files changed

Lines changed: 55 additions & 63 deletions

File tree

src/Lucene.Net.Codecs/SimpleText/SimpleTextTermVectorsReader.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -362,12 +362,7 @@ internal SimpleTVTermsEnum(JCG.SortedDictionary<BytesRef, SimpleTVPostings> term
362362

363363
public override SeekStatus SeekCeil(BytesRef text)
364364
{
365-
var newTerms = new JCG.SortedDictionary<BytesRef, SimpleTVPostings>(_terms.Comparer);
366-
foreach (var p in _terms)
367-
if (p.Key.CompareTo(text) >= 0)
368-
newTerms.Add(p.Key, p.Value);
369-
370-
_iterator = newTerms.GetEnumerator();
365+
_iterator = _terms.GetViewAfter(text).GetEnumerator();
371366

372367
// LUCENENET specific: Since in .NET we don't have a HasNext() method, we need
373368
// to call MoveNext(). Since we need

src/Lucene.Net.Grouping/AbstractFirstPassGroupingCollector.cs

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
using Lucene.Net.Diagnostics;
22
using Lucene.Net.Index;
33
using Lucene.Net.Support;
4-
using Lucene.Net.Support.Threading;
54
using System;
65
using System.Collections.Generic;
76
using System.IO;
8-
using System.Linq;
97
using JCG = J2N.Collections.Generic;
108

119
namespace Lucene.Net.Search.Grouping
@@ -241,18 +239,7 @@ public virtual void Collect(int doc)
241239

242240
// We already tested that the document is competitive, so replace
243241
// the bottom group with this new group.
244-
//CollectedSearchGroup<TGroupValue> bottomGroup = orderedGroups.PollLast();
245-
CollectedSearchGroup<TGroupValue> bottomGroup;
246-
UninterruptableMonitor.Enter(m_orderedGroups);
247-
try
248-
{
249-
bottomGroup = m_orderedGroups.Last();
250-
m_orderedGroups.Remove(bottomGroup);
251-
}
252-
finally
253-
{
254-
UninterruptableMonitor.Exit(m_orderedGroups);
255-
}
242+
m_orderedGroups.RemoveLast(out CollectedSearchGroup<TGroupValue> bottomGroup);
256243
if (Debugging.AssertsEnabled) Debugging.Assert(m_orderedGroups.Count == topNGroups - 1);
257244

258245
groupMap.Remove(bottomGroup.GroupValue);
@@ -270,7 +257,9 @@ public virtual void Collect(int doc)
270257
m_orderedGroups.Add(bottomGroup);
271258
if (Debugging.AssertsEnabled) Debugging.Assert(m_orderedGroups.Count == topNGroups);
272259

273-
int lastComparerSlot = m_orderedGroups.Last().ComparerSlot;
260+
// LUCENENET: We know this call cannot fail because we just added a group, so we can safely ignore the return value.
261+
_ = m_orderedGroups.TryGetLast(out CollectedSearchGroup<TGroupValue> lastGroup);
262+
int lastComparerSlot = lastGroup.ComparerSlot;
274263
foreach (FieldComparer fc in comparers)
275264
{
276265
fc.SetBottom(lastComparerSlot);
@@ -315,16 +304,8 @@ public virtual void Collect(int doc)
315304
CollectedSearchGroup<TGroupValue> prevLast;
316305
if (m_orderedGroups != null)
317306
{
318-
UninterruptableMonitor.Enter(m_orderedGroups);
319-
try
320-
{
321-
prevLast = m_orderedGroups.Last();
322-
m_orderedGroups.Remove(group);
323-
}
324-
finally
325-
{
326-
UninterruptableMonitor.Exit(m_orderedGroups);
327-
}
307+
m_orderedGroups.TryGetLast(out prevLast);
308+
m_orderedGroups.Remove(group);
328309
if (Debugging.AssertsEnabled) Debugging.Assert(m_orderedGroups.Count == topNGroups - 1);
329310
}
330311
else
@@ -344,7 +325,11 @@ public virtual void Collect(int doc)
344325
{
345326
m_orderedGroups.Add(group);
346327
if (Debugging.AssertsEnabled) Debugging.Assert(m_orderedGroups.Count == topNGroups);
347-
var newLast = m_orderedGroups.Last();
328+
if (!m_orderedGroups.TryGetLast(out CollectedSearchGroup<TGroupValue> newLast))
329+
{
330+
// LUCENENET: Added because Java would throw NoSuchElementException if orderedGroups is empty.
331+
throw new InvalidOperationException("orderedGroups must not be empty");
332+
}
348333
// If we changed the value of the last group, or changed which group was last, then update bottom:
349334
if (group == newLast || prevLast != newLast)
350335
{
@@ -390,7 +375,10 @@ private void BuildSortedSet()
390375

391376
foreach (FieldComparer fc in comparers)
392377
{
393-
fc.SetBottom(m_orderedGroups.Last().ComparerSlot);
378+
if (!m_orderedGroups.TryGetLast(out CollectedSearchGroup<TGroupValue> lastGroup))
379+
// LUCENENET: Added because Java would throw NoSuchElementException if orderedGroups is empty.
380+
throw new InvalidOperationException("orderedGroups must not be empty");
381+
fc.SetBottom(lastGroup.ComparerSlot);
394382
}
395383
}
396384

src/Lucene.Net.Grouping/AbstractGroupFacetCollector.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,16 +184,16 @@ public virtual void AddFacetCount(BytesRef facetValue, int count)
184184
{
185185
return;
186186
}
187-
var max = facetEntries.Max;
188-
if (max != null)
189-
facetEntries.Remove(max);
187+
facetEntries.RemoveLast(out _);
190188
}
191189
facetEntries.Add(facetEntry);
192190

193191
if (facetEntries.Count == maxSize)
194192
{
195-
var max = facetEntries.Max;
196-
currentMin = max != null ? max.Count : 0;
193+
// LUCENENET: We can safely ignore the return value of TryGetLast() here, because we
194+
// know that the collection is not empty (we just added an entry to it).
195+
_ = facetEntries.TryGetLast(out FacetEntry last);
196+
currentMin = last.Count;
197197
}
198198
}
199199

src/Lucene.Net.Grouping/SearchGroup.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -392,16 +392,14 @@ private void UpdateNextGroup(int topN, ShardIter<T> shard)
392392
// Prune un-competitive groups:
393393
while (queue.Count > topN)
394394
{
395-
MergedGroup<T> group = queue.Max;
396-
queue.Remove(group);
395+
queue.RemoveLast(out MergedGroup<T> group);
397396
//System.out.println("PRUNE: " + group);
398397
group.IsInQueue = false;
399398
}
400399
}
401400

402401
public virtual ICollection<SearchGroup<T>> Merge(IList<ICollection<SearchGroup<T>>> shards, int offset, int topN)
403402
{
404-
405403
int maxQueueSize = offset + topN;
406404

407405
//System.out.println("merge");
@@ -423,8 +421,7 @@ public virtual ICollection<SearchGroup<T>> Merge(IList<ICollection<SearchGroup<T
423421

424422
while (queue.Count != 0)
425423
{
426-
MergedGroup<T> group = queue.Min;
427-
queue.Remove(group);
424+
queue.RemoveFirst(out MergedGroup<T> group);
428425
group.IsProcessed = true;
429426
//System.out.println(" pop: shards=" + group.shards + " group=" + (group.groupValue is null ? "null" : (((BytesRef) group.groupValue).utf8ToString())) + " sortValues=" + Arrays.toString(group.topValues));
430427
if (count++ >= offset)

src/Lucene.Net.Highlighter/PostingsHighlight/PostingsHighlighter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,8 +436,8 @@ protected internal virtual IDictionary<string, object[]> HighlightFieldsAsObject
436436
Term floor = new Term(field, "");
437437
Term ceiling = new Term(field, UnicodeUtil.BIG_TERM);
438438

439-
// LUCENENET: Call custom GetViewBetween overload to mimic Java's exclusive upper bound behavior.
440-
var fieldTerms = queryTerms.GetViewBetween(floor, lowerValueInclusive: true, ceiling, upperValueInclusive: false);
439+
// LUCENENET: Call J2N's GetView overload to mimic Java's exclusive upper bound behavior.
440+
var fieldTerms = queryTerms.GetView(floor, fromInclusive: true, ceiling, toInclusive: false);
441441

442442
// TODO: should we have some reasonable defaults for term pruning? (e.g. stopwords)
443443

src/Lucene.Net.Suggest/Suggest/Analyzing/BlendedInfixSuggester.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,10 +240,12 @@ private static void BoundedTreeAdd(JCG.SortedSet<Lookup.LookupResult> results, L
240240
{
241241
if (results.Count >= num)
242242
{
243-
var first = results.Min; // "get" our first object so we don't cross threads
243+
if (!results.TryGetFirst(out Lookup.LookupResult first)) // "get" our first object so we don't cross threads
244+
// LUCENENET: Java would throw NoSuchElementException here, so we are also throwing in this case.
245+
throw new InvalidOperationException("Expected at least one result in the set, but there were none.");
244246
if (first.Value < result.Value)
245247
// Code similar to the java TreeMap class
246-
results.Remove(first);
248+
results.Remove(first); // LUCENENET: Calling RemoveFirst() would be redundant here, since we already have the value.
247249
else
248250
return;
249251
}

src/Lucene.Net.TestFramework/Analysis/MockCharFilter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,9 @@ protected override int Correct(int currentOff)
103103
{
104104
int ret;
105105
// LUCENENET NOTE: TryGetPredecessor is equivalent to TreeMap.lowerEntry() in Java
106-
if (corrections.TryGetPredecessor(currentOff + 1, out KeyValuePair<int, int> lastEntry))
106+
if (corrections.TryGetPredecessor(currentOff + 1, out _, out int lastEntryValue))
107107
{
108-
ret = currentOff + lastEntry.Value;
108+
ret = currentOff + lastEntryValue;
109109
}
110110
else
111111
{

src/Lucene.Net.TestFramework/Codecs/Lucene40/Lucene40DocValuesWriter.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,10 @@ private void AddFixedDerefBytesField(FieldInfo field, IndexOutput data, IndexOut
389389
{
390390
brefDummy = new BytesRef();
391391
}
392-
//int ord = dictionary.HeadSet(brefDummy).Size();
392+
// LUCENENET: This LINQ call will actually be less overhead than the
393+
// equivalent check using dictionary.GetViewBefore(brefDummy, false).Count
394+
// due to all of the range checks during the tree walk to determine the count.
395+
// In this case, LINQ doesn't allocate anything so will always be faster.
393396
int ord = dictionary.Count(@ref => @ref.CompareTo(brefDummy) < 0);
394397
w.Add(ord);
395398
}

src/Lucene.Net.TestFramework/Codecs/RAMOnly/RAMOnlyPostingsFormat.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using Lucene.Net.Util;
99
using System;
1010
using System.Collections.Generic;
11-
using System.Linq;
1211
using JCG = J2N.Collections.Generic;
1312

1413
namespace Lucene.Net.Codecs.RAMOnly
@@ -391,8 +390,7 @@ private void EnsureEnumeratorInitialized() // LUCENENET specific - factored out
391390
}
392391
else
393392
{
394-
//It = RamField.TermToDocs.tailMap(Current).Keys.GetEnumerator();
395-
it = ramField.termToDocs.Where(kvpair => string.CompareOrdinal(kvpair.Key, current) >= 0).Select(pair => pair.Key).GetEnumerator();
393+
it = ramField.termToDocs.GetViewAfter(current).Keys.GetEnumerator();
396394
}
397395
}
398396
}
@@ -407,7 +405,12 @@ public override SeekStatus SeekCeil(BytesRef term)
407405
}
408406
else
409407
{
410-
if (current.CompareToOrdinal(ramField.termToDocs.Last().Key) > 0)
408+
if (!ramField.termToDocs.TryGetLast(out string lastKey, out _))
409+
// LUCENENET: Java would throw a NoSuchElementException here, so we throw InvalidOperationException
410+
// to indicate that the dictionary is empty when it shouldn't be.
411+
throw new InvalidOperationException("The termToDocs dictionary is empty, but it should have at least one term.");
412+
413+
if (current.CompareToOrdinal(lastKey) > 0)
411414
{
412415
return SeekStatus.END;
413416
}

src/Lucene.Net/Codecs/Compressing/CompressingTermVectorsWriter.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,14 @@ private int[] FlushFieldNums()
472472

473473
int numDistinctFields = fieldNums.Count;
474474
if (Debugging.AssertsEnabled) Debugging.Assert(numDistinctFields > 0);
475-
int bitsRequired = PackedInt32s.BitsRequired(fieldNums.Max);
475+
// LUCENENET specific - Java would throw a NoSuchElementException, but in .NET we throw an
476+
// InvalidOperationException instead, since the collection is empty and .NET doesn't throw
477+
// in this case. In practice, this exception should never happen.
478+
if (!fieldNums.TryGetLast(out int last))
479+
{
480+
throw new InvalidOperationException("fieldNums must not be empty");
481+
}
482+
int bitsRequired = PackedInt32s.BitsRequired(last);
476483
int token = (Math.Min(numDistinctFields - 1, 0x07) << 5) | bitsRequired;
477484
vectorsStream.WriteByte((byte)token);
478485
if (numDistinctFields - 1 >= 0x07)

0 commit comments

Comments
 (0)