Skip to content

Commit d7d1240

Browse files
authored
Fix double-counting of Automaton in AutomatonQuery#ramBytesUsed (#16390)
1 parent 1593f87 commit d7d1240

4 files changed

Lines changed: 89 additions & 1 deletion

File tree

lucene/CHANGES.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ Bug Fixes
191191
been aborted, so IndexWriter#rollback and abortMerges no longer block until the entire graph is
192192
built. (Jeho Jeong)
193193

194+
* GITHUB#16389: Fix double-counting of the underlying Automaton in AutomatonQuery#ramBytesUsed. (Sasilekha R)
195+
194196
* GITHUB#14049: Randomize KNN codec params in RandomCodec. Fixes scalar quantization div-by-zero
195197
when all values are identical. (Mike Sokolov)
196198

lucene/core/src/java/org/apache/lucene/search/AutomatonQuery.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,10 @@ public AutomatonQuery(
9898
this.automatonIsBinary = isBinary;
9999
this.compiled = new CompiledAutomaton(automaton, false, true, isBinary);
100100

101+
// compiled may already reference the same Automaton instance; only count its bytes once.
102+
long automatonBytes = compiled.sharesAutomaton(automaton) ? 0L : automaton.ramBytesUsed();
101103
this.ramBytesUsed =
102-
BASE_RAM_BYTES + term.ramBytesUsed() + automaton.ramBytesUsed() + compiled.ramBytesUsed();
104+
BASE_RAM_BYTES + term.ramBytesUsed() + automatonBytes + compiled.ramBytesUsed();
103105
}
104106

105107
@Override

lucene/core/src/java/org/apache/lucene/util/automaton/CompiledAutomaton.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,23 @@ public ByteRunnable getByteRunnable() {
492492
return nfaRunAutomaton;
493493
}
494494

495+
/**
496+
* Returns {@code true} if this {@link CompiledAutomaton} internally holds the given {@link
497+
* Automaton} instance. This is only the case on the {@link AUTOMATON_TYPE#NORMAL} paths built
498+
* without a UTF-8 conversion (typically {@code isBinary=true} construction): the DFA path retains
499+
* it as {@link #automaton}, and the NFA path retains it inside the internal NFA runner. Returns
500+
* {@code false} for {@link AUTOMATON_TYPE#NONE}, {@link AUTOMATON_TYPE#ALL} and {@link
501+
* AUTOMATON_TYPE#SINGLE} -- those short-circuit and keep no reference -- and for {@code
502+
* isBinary=false} construction, where the internal automaton is a distinct UTF-8 conversion of
503+
* the input.
504+
*
505+
* <p>Useful for callers that keep their own reference to the same {@link Automaton} and want to
506+
* avoid double-counting it in their own {@link Accountable#ramBytesUsed()} calculation.
507+
*/
508+
public boolean sharesAutomaton(Automaton a) {
509+
return a == automaton || (nfaRunAutomaton != null && nfaRunAutomaton.getAutomaton() == a);
510+
}
511+
495512
/**
496513
* Get a {@link TransitionAccessor} instance, it will be different depending on whether a NFA or
497514
* DFA is passed in, and does not guarantee returning non-null object

lucene/core/src/test/org/apache/lucene/search/TestAutomatonQuery.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,16 @@
3434
import org.apache.lucene.store.Directory;
3535
import org.apache.lucene.tests.index.RandomIndexWriter;
3636
import org.apache.lucene.tests.util.LuceneTestCase;
37+
import org.apache.lucene.tests.util.RamUsageTester;
3738
import org.apache.lucene.tests.util.Rethrow;
3839
import org.apache.lucene.tests.util.TestUtil;
3940
import org.apache.lucene.tests.util.automaton.AutomatonTestUtil;
4041
import org.apache.lucene.util.BytesRef;
42+
import org.apache.lucene.util.RamUsageEstimator;
4143
import org.apache.lucene.util.automaton.Automata;
4244
import org.apache.lucene.util.automaton.Automaton;
4345
import org.apache.lucene.util.automaton.Operations;
46+
import org.apache.lucene.util.automaton.RegExp;
4447

4548
public class TestAutomatonQuery extends LuceneTestCase {
4649
private Directory directory;
@@ -252,4 +255,68 @@ public void testBiggishAutomaton() {
252255
Collections.sort(terms);
253256
new AutomatonQuery(new Term("foo", "bar"), Automata.makeStringUnion(terms));
254257
}
258+
259+
public void testRamBytesUsedDoesNotDoubleCountSharedDeterministicAutomaton() {
260+
// Same shape as PrefixQuery: an already-deterministic binary automaton passed with
261+
// isBinary=true. CompiledAutomaton keeps a reference to the same Automaton instance
262+
// via runAutomaton.automaton; naively summing automaton + compiled would double-count.
263+
Automaton prefix = PrefixQuery.toAutomaton(new BytesRef("prefix"));
264+
AutomatonQuery q = new AutomatonQuery(new Term(FN, "prefix"), prefix, true);
265+
assertSame(prefix, q.getCompiled().automaton);
266+
assertRamBytesExcludesSharedAutomaton(q, prefix);
267+
}
268+
269+
public void testRamBytesUsedDoesNotDoubleCountSharedNfaAutomaton() {
270+
// isBinary=true with a non-deterministic input drives CompiledAutomaton down the NFA
271+
// path, where nfaRunAutomaton wraps the same Automaton instance.
272+
Automaton nfa = new Automaton();
273+
int start = nfa.createState();
274+
int a1 = nfa.createState();
275+
int a2 = nfa.createState();
276+
nfa.setAccept(a1, true);
277+
nfa.setAccept(a2, true);
278+
nfa.addTransition(start, a1, 'a', 'a');
279+
nfa.addTransition(start, a2, 'a', 'a');
280+
nfa.finishState();
281+
assertFalse(nfa.isDeterministic());
282+
283+
AutomatonQuery q = new AutomatonQuery(new Term(FN, "nfa"), nfa, true);
284+
assertNull(q.getCompiled().automaton);
285+
assertTrue(q.getCompiled().sharesAutomaton(nfa));
286+
assertRamBytesExcludesSharedAutomaton(q, nfa);
287+
}
288+
289+
public void testRamBytesUsedIsBinaryFalseCountsOuterAutomaton() {
290+
// isBinary=false path: CompiledAutomaton converts to UTF-8 internally, so the outer
291+
// automaton and compiled hold distinct Automaton instances. Both must be counted --
292+
// this test guards against a future refactor accidentally dropping the outer bytes.
293+
Automaton a =
294+
Operations.determinize(new RegExp("abc.*").toAutomaton(), DEFAULT_DETERMINIZE_WORK_LIMIT);
295+
AutomatonQuery q = new AutomatonQuery(new Term(FN, "regex"), a);
296+
assertFalse(q.getCompiled().sharesAutomaton(a));
297+
long reported = q.ramBytesUsed();
298+
assertTrue(
299+
"outer automaton must still contribute on the isBinary=false path",
300+
reported >= a.ramBytesUsed());
301+
long actual = RamUsageTester.ramUsed(q);
302+
assertEquals((double) actual, (double) reported, (double) actual * 0.10);
303+
}
304+
305+
// Asserts that ramBytesUsed() does not include the shared Automaton twice.
306+
// Reported bytes must equal (shallow AutomatonQuery + term + compiled), i.e., the
307+
// shared automaton is accounted for only once (via compiled). If the bug were
308+
// present, reported would be inflated by exactly sharedAutomaton.ramBytesUsed().
309+
private static void assertRamBytesExcludesSharedAutomaton(
310+
AutomatonQuery q, Automaton sharedAutomaton) {
311+
long expected =
312+
RamUsageEstimator.shallowSizeOfInstance(AutomatonQuery.class)
313+
+ q.term.ramBytesUsed()
314+
+ q.getCompiled().ramBytesUsed();
315+
assertEquals(
316+
"shared Automaton must not be counted twice (would over-report by "
317+
+ sharedAutomaton.ramBytesUsed()
318+
+ " bytes)",
319+
expected,
320+
q.ramBytesUsed());
321+
}
255322
}

0 commit comments

Comments
 (0)