Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ public void apply(Project project) {
"tests.verbose",
"Enables verbose test output mode (emits full test outputs immediately).",
false);
optionsInheritedAsProperties.add("tests.verbose");

Provider<Boolean> haltOnFailureOption =
buildOptions.addBooleanOption(
"tests.haltonfailure", "Stop processing on test failures.", true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public PreviousSearchState(
}
}

@Nightly // TODO: -Ptests.seed=A148FC8983CFA7A - takes 12 seconds. very slow. wall-clock dependent
public void testSimple() throws Exception {
final int numNodes = TestUtil.nextInt(random(), 1, 10);

Expand Down
90 changes: 40 additions & 50 deletions lucene/core/src/test/org/apache/lucene/util/TestPriorityQueue.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.lessThanOrEqualTo;

import com.carrotsearch.randomizedtesting.Xoroshiro128PlusRandom;
import com.carrotsearch.randomizedtesting.generators.RandomNumbers;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
Expand All @@ -30,7 +32,6 @@
import org.apache.lucene.tests.util.TestUtil;
import org.hamcrest.Matchers;

@SuppressWarnings("BoxedPrimitiveEquality")
public class TestPriorityQueue extends LuceneTestCase {

private static class IntegerQueue extends PriorityQueue<Integer> {
Expand Down Expand Up @@ -208,59 +209,48 @@ public void testAddAllDoesNotFitIntoQueue() {
() -> pq.addAll(list));
}

// TODO: incredibly slow
@Nightly
/** Randomly add and remove elements, comparing against the reference java.util.PriorityQueue. */
public void testRemovalsAndInsertions() {
Random random = random();
int numDocsInPQ = TestUtil.nextInt(random, 1, 100);
IntegerQueue pq = new IntegerQueue(numDocsInPQ);
Integer lastLeast = null;

// Basic insertion of new content
ArrayList<Integer> sds = new ArrayList<>(numDocsInPQ);
for (int i = 0; i < numDocsInPQ * 10; i++) {
Integer newEntry = Math.abs(random.nextInt());
sds.add(newEntry);
Integer evicted = pq.insertWithOverflow(newEntry);
pq.checkValidity();
if (evicted != null) {
assertTrue(sds.remove(evicted));
if (evicted != newEntry) {
assertSame(evicted, lastLeast);
int maxElement = RandomNumbers.randomIntBetween(random(), 1, 10_000);
int size = maxElement / 2 + 1;

var reference = new java.util.PriorityQueue<Integer>();
var pq = new IntegerQueue(size);

Random localRandom = new Xoroshiro128PlusRandom(random().nextLong());

// Lucene's PriorityQueue.remove uses reference equality, not .equals to determine which
// elements
// to remove (!).
HashMap<Integer, Integer> ints = new HashMap<>();
Comment on lines +222 to +225
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test had a huge iteration count, was checking tons of stuff unnecessarily... I rewrote it to just compare against Java's PriorityQueue... but it turned out that lucene's PriorityQueue.remove(T) is implemented as a linear scan through all elements, with reference equality (which is surprising to say the least...). That's why this "cache" of ints needs to be here.

PQ.remove is only used from one place within Lucene... maybe we should drop this method entirely (or at least change == to .equals to be less surprising?).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to open a followup issue on this!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


for (int i = 0, iters = size * 2; i < iters; i++) {
Integer element = ints.computeIfAbsent(localRandom.nextInt(maxElement), k -> k);

var action = localRandom.nextInt(100);
if (action < 25) {
// removals, possibly misses.
assertEquals("remove() difference: " + i, reference.remove(element), pq.remove(element));
} else {
// additions.
var dropped = pq.insertWithOverflow(element);

reference.add(element);
Integer droppedReference;
if (reference.size() > size) {
droppedReference = reference.remove();
} else {
droppedReference = null;
}
}
Integer newLeast = pq.top();
if ((lastLeast != null) && (newLeast != newEntry) && (newLeast != lastLeast)) {
// If there has been a change of least entry and it wasn't our new
// addition we expect the scores to increase
assertThat(newLeast, lessThanOrEqualTo(newEntry));
assertThat(newLeast, greaterThanOrEqualTo(lastLeast));
}
lastLeast = newLeast;
}

// Try many random additions to existing entries - we should always see
// increasing scores in the lowest entry in the PQ
for (int p = 0; p < 500000; p++) {
int element = (int) (random.nextFloat() * (sds.size() - 1));
Integer objectToRemove = sds.get(element);
assertSame(sds.remove(element), objectToRemove);
assertTrue(pq.remove(objectToRemove));
pq.checkValidity();
Integer newEntry = Math.abs(random.nextInt());
sds.add(newEntry);
assertNull(pq.insertWithOverflow(newEntry));
pq.checkValidity();
Integer newLeast = pq.top();
if ((objectToRemove != lastLeast) && (lastLeast != null) && (newLeast != newEntry)) {
// If there has been a change of least entry and it wasn't our new
// addition or the loss of our randomly removed entry we expect the
// scores to increase
assertThat(newLeast, lessThanOrEqualTo(newEntry));
assertThat(newLeast, greaterThanOrEqualTo(lastLeast));
assertEquals("insertWithOverflow() difference.", dropped, droppedReference);
}
lastLeast = newLeast;

assertEquals("insertWithOverflow() size difference?", reference.size(), pq.size());
assertEquals("top() difference?", reference.peek(), pq.top());
}

pq.checkValidity();
}

public void testIteratorEmpty() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void testReorderOnMerge() throws IOException {
KnnFloatVectorField vectorField = new KnnFloatVectorField("vector", new float[] {0});
doc.add(vectorField);

for (int i = 0; i < 10000; ++i) {
for (int i = 0; i < 1000; ++i) {
idField.setStringValue(Integer.toString(i));
int intValue = i % 2 == 0 ? 0 : i % 10;
bodyField.setStringValue(Integer.toString(intValue));
Expand Down Expand Up @@ -158,7 +158,7 @@ public void testReorderOnAddIndexes() throws IOException {
KnnFloatVectorField vectorField = new KnnFloatVectorField("vector", new float[] {0});
doc.add(vectorField);

for (int i = 0; i < 10000; ++i) {
for (int i = 0; i < 1000; ++i) {
idField.setStringValue(Integer.toString(i));
int intValue = i % 2 == 0 ? 0 : i % 10;
bodyField.setStringValue(Integer.toString(intValue));
Expand All @@ -171,7 +171,7 @@ public void testReorderOnAddIndexes() throws IOException {
}
}

for (int i = 0; i < 10000; i += 10) {
for (int i = 0; i < 1000; i += 10) {
w1.deleteDocuments(new Term("id", Integer.toString(i / 3)));
}

Expand Down
Loading