From 6af650b43fdc23b536a3771335611eacfd60256b Mon Sep 17 00:00:00 2001 From: Michael McCandless Date: Thu, 1 Nov 2012 23:00:24 +0000 Subject: [PATCH] cleanup TopNSearcher: don't copy same comparator instance in the paths git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1404818 13f79535-47bb-0310-9956-ffa450edef68 --- .../java/org/apache/lucene/util/fst/Util.java | 47 +++++++++---------- .../org/apache/lucene/util/fst/TestFSTs.java | 28 ++++++++--- 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/util/fst/Util.java b/lucene/core/src/java/org/apache/lucene/util/fst/Util.java index 4326b4dab74..256f72cd61e 100644 --- a/lucene/core/src/java/org/apache/lucene/util/fst/Util.java +++ b/lucene/core/src/java/org/apache/lucene/util/fst/Util.java @@ -232,16 +232,14 @@ public final class Util { } } - private static class FSTPath implements Comparable> { + private static class FSTPath { public FST.Arc arc; public T cost; public final IntsRef input; - final Comparator comparator; - public FSTPath(T cost, FST.Arc arc, Comparator comparator, IntsRef input) { + public FSTPath(T cost, FST.Arc arc, IntsRef input) { this.arc = new FST.Arc().copyFrom(arc); this.cost = cost; - this.comparator = comparator; this.input = input; } @@ -249,12 +247,21 @@ public final class Util { public String toString() { return "input=" + input + " cost=" + cost; } + } + + /** Compares first by the provided comparator, and then + * tie breaks by path.input. */ + private static class TieBreakByInputComparator implements Comparator> { + private final Comparator comparator; + public TieBreakByInputComparator(Comparator comparator) { + this.comparator = comparator; + } @Override - public int compareTo(FSTPath other) { - int cmp = comparator.compare(cost, other.cost); + public int compare(FSTPath a, FSTPath b) { + int cmp = comparator.compare(a.cost, b.cost); if (cmp == 0) { - return input.compareTo(other.input); + return a.input.compareTo(b.input); } else { return cmp; } @@ -283,7 +290,7 @@ public final class Util { this.maxQueueDepth = maxQueueDepth; this.comparator = comparator; - queue = new TreeSet>(); + queue = new TreeSet>(new TieBreakByInputComparator(comparator)); } // If back plus this arc is competitive then add to queue: @@ -326,7 +333,7 @@ public final class Util { System.arraycopy(path.input.ints, 0, newInput.ints, 0, path.input.length); newInput.ints[path.input.length] = path.arc.label; newInput.length = path.input.length+1; - final FSTPath newPath = new FSTPath(cost, path.arc, comparator, newInput); + final FSTPath newPath = new FSTPath(cost, path.arc, newInput); queue.add(newPath); @@ -344,7 +351,7 @@ public final class Util { startOutput = fst.outputs.getNoOutput(); } - FSTPath path = new FSTPath(startOutput, node, comparator, input); + FSTPath path = new FSTPath(startOutput, node, input); fst.readFirstTargetArc(node, path.arc, bytesReader); //System.out.println("add start paths"); @@ -402,7 +409,7 @@ public final class Util { //System.out.println(" empty string! cost=" + path.cost); // Empty string! path.input.length--; - results.add(new MinResult(path.input, path.cost, comparator)); + results.add(new MinResult(path.input, path.cost)); continue; } @@ -465,7 +472,7 @@ public final class Util { //System.out.println(" done!: " + path); T finalOutput = fst.outputs.add(path.cost, path.arc.output); if (acceptResult(path.input, finalOutput)) { - results.add(new MinResult(path.input, finalOutput, comparator)); + results.add(new MinResult(path.input, finalOutput)); } else { rejectCount++; assert rejectCount + topN <= maxQueueDepth: "maxQueueDepth (" + maxQueueDepth + ") is too small for topN (" + topN + "): rejected " + rejectCount + " paths"; @@ -492,24 +499,12 @@ public final class Util { /** Holds a single input (IntsRef) + output, returned by * {@link #shortestPaths shortestPaths()}. */ - public final static class MinResult implements Comparable> { + public final static class MinResult { public final IntsRef input; public final T output; - final Comparator comparator; - public MinResult(IntsRef input, T output, Comparator comparator) { + public MinResult(IntsRef input, T output) { this.input = input; this.output = output; - this.comparator = comparator; - } - - @Override - public int compareTo(MinResult other) { - int cmp = comparator.compare(output, other.output); - if (cmp == 0) { - return input.compareTo(other.input); - } else { - return cmp; - } } } diff --git a/lucene/core/src/test/org/apache/lucene/util/fst/TestFSTs.java b/lucene/core/src/test/org/apache/lucene/util/fst/TestFSTs.java index a39f9b55421..c96fba0ce0a 100644 --- a/lucene/core/src/test/org/apache/lucene/util/fst/TestFSTs.java +++ b/lucene/core/src/test/org/apache/lucene/util/fst/TestFSTs.java @@ -1336,12 +1336,12 @@ public class TestFSTs extends LuceneTestCase { if (e.getKey().startsWith(prefix)) { //System.out.println(" consider " + e.getKey()); matches.add(new Util.MinResult(Util.toIntsRef(new BytesRef(e.getKey().substring(prefix.length())), new IntsRef()), - e.getValue() - prefixOutput, minLongComparator)); + e.getValue() - prefixOutput)); } } assertTrue(matches.size() > 0); - Collections.sort(matches); + Collections.sort(matches, new TieBreakByInputComparator(minLongComparator)); if (matches.size() > topN) { matches.subList(topN, matches.size()).clear(); } @@ -1355,7 +1355,24 @@ public class TestFSTs extends LuceneTestCase { } } } - + + private static class TieBreakByInputComparator implements Comparator> { + private final Comparator comparator; + public TieBreakByInputComparator(Comparator comparator) { + this.comparator = comparator; + } + + @Override + public int compare(Util.MinResult a, Util.MinResult b) { + int cmp = comparator.compare(a.output, b.output); + if (cmp == 0) { + return a.input.compareTo(b.input); + } else { + return cmp; + } + } + } + // used by slowcompletor class TwoLongs { long a; @@ -1440,13 +1457,12 @@ public class TestFSTs extends LuceneTestCase { if (e.getKey().startsWith(prefix)) { //System.out.println(" consider " + e.getKey()); matches.add(new Util.MinResult>(Util.toIntsRef(new BytesRef(e.getKey().substring(prefix.length())), new IntsRef()), - outputs.newPair(e.getValue().a - prefixOutput.output1, e.getValue().b - prefixOutput.output2), - minPairWeightComparator)); + outputs.newPair(e.getValue().a - prefixOutput.output1, e.getValue().b - prefixOutput.output2))); } } assertTrue(matches.size() > 0); - Collections.sort(matches); + Collections.sort(matches, new TieBreakByInputComparator(minPairWeightComparator)); if (matches.size() > topN) { matches.subList(topN, matches.size()).clear(); }