You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by s1monw <gi...@git.apache.org> on 2018/12/12 15:40:57 UTC
[GitHub] lucene-solr pull request #526: LUCENE-8608: Extract utility class to iterate...
GitHub user s1monw opened a pull request:
https://github.com/apache/lucene-solr/pull/526
LUCENE-8608: Extract utility class to iterate over terms docs
Today we re-implement the same algorithm in various places
when we want to consume all docs for a set/list of terms. This
caused serious slowdowns for instance in the case of applying
updates fixed in LUCENE-8602. This change extracts the common
usage and shares the interation code including logic to reuse
Terms and PostingsEnum instances as much as possble and adds
tests for it.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/s1monw/lucene-solr extract_terms_seeker
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/526.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #526
----
commit ed7f8531c0274c36d7cacf1abc4894d27592167c
Author: Simon Willnauer <si...@...>
Date: 2018-12-07T21:17:26Z
LUCENE-8608: Extract utility class to iterate over terms docs
Today we re-implement the same algorithm in various places
when we want to consume all docs for a set/list of terms. This
caused serious slowdowns for instance in the case of applying
updates fixed in LUCENE-8602. This change extracts the common
usage and shares the interation code including logic to reuse
Terms and PostingsEnum instances as much as possble and adds
tests for it.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #526: LUCENE-8608: Extract utility class to iterate...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/lucene-solr/pull/526
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #526: LUCENE-8608: Extract utility class to iterate...
Posted by mikemccand <gi...@git.apache.org>.
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/526#discussion_r241373334
--- Diff: lucene/core/src/test/org/apache/lucene/index/TestFrozenBufferedUpdates.java ---
@@ -0,0 +1,104 @@
+/*
+ * 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.
+ */
+
+package org.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import com.carrotsearch.randomizedtesting.generators.RandomPicks;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.document.Field;
+import org.apache.lucene.document.StringField;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.BytesRefArray;
+import org.apache.lucene.util.BytesRefIterator;
+import org.apache.lucene.util.Counter;
+import org.apache.lucene.util.FixedBitSet;
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.index.FrozenBufferedUpdates.TermDocsIterator;
+import org.apache.lucene.util.TestUtil;
+
+public class TestFrozenBufferedUpdates extends LuceneTestCase {
+
+ public void testTermDocsIterator() throws IOException {
--- End diff --
Nice random test :)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #526: LUCENE-8608: Extract utility class to iterate...
Posted by mikemccand <gi...@git.apache.org>.
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/526#discussion_r241372971
--- Diff: lucene/core/src/java/org/apache/lucene/index/FrozenBufferedUpdates.java ---
@@ -821,4 +759,93 @@ public String toString() {
boolean any() {
return deleteTerms.size() > 0 || deleteQueries.length > 0 || fieldUpdatesCount > 0 ;
}
+
+ /**
+ * This class helps iterating a term dictionary and consuming all the docs for each terms.
+ * It accepts a field, value tuple and returns a {@link DocIdSetIterator} if the field has an entry
+ * for the given value. It has an optimized way of iterating the term dictionary if the terms are
+ * passed in sorted order and makes sure terms and postings are reused as much as possible.
+ */
+ static final class TermDocsIterator {
+ private final TermsProvider provider;
+ private String field;
+ private TermsEnum termsEnum;
+ private PostingsEnum postingsEnum;
+ private final boolean sortedTerms;
+ private BytesRef readerTerm;
+
+ @FunctionalInterface
+ interface TermsProvider {
+ Terms terms(String field) throws IOException;
+ }
+
+ TermDocsIterator(Fields fields, boolean sortedTerms) {
+ this(fields::terms, sortedTerms);
+ }
+
+ TermDocsIterator(LeafReader reader, boolean sortedTerms) {
+ this(reader::terms, sortedTerms);
+ }
+
+ private TermDocsIterator(TermsProvider provider, boolean sortedTerms) {
+ this.sortedTerms = sortedTerms;
+ this.provider = provider;
+ }
+
+ private void nextField(String field) throws IOException {
+ if (this.field == null || this.field.equals(field) == false) {
+ this.field = field;
+ Terms terms = provider.terms(field);
+ if (terms != null) {
+ termsEnum = terms.iterator();
+ if (sortedTerms) {
+ readerTerm = termsEnum.next();
+ }
+ } else {
+ termsEnum = null;
+ }
+ }
+ }
+
+ DocIdSetIterator nextTerm(String field, BytesRef term) throws IOException {
+ nextField(field);
+ if (termsEnum != null) {
+ if (sortedTerms) {
+ // in the sorted case we can take advantage of the "seeking forward" property
+ // this allows us depending on the term dict impl to reuse data-strucutres internally
+ // which speed up iteration over terms and docs significantly.
+ int cmp = term.compareTo(readerTerm);
+ if (cmp < 0) {
+ return null;
--- End diff --
Maybe add comment `// requested term does not exist in this segment`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #526: LUCENE-8608: Extract utility class to iterate...
Posted by mikemccand <gi...@git.apache.org>.
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/526#discussion_r241372182
--- Diff: lucene/core/src/java/org/apache/lucene/index/FrozenBufferedUpdates.java ---
@@ -821,4 +759,93 @@ public String toString() {
boolean any() {
return deleteTerms.size() > 0 || deleteQueries.length > 0 || fieldUpdatesCount > 0 ;
}
+
+ /**
+ * This class helps iterating a term dictionary and consuming all the docs for each terms.
+ * It accepts a field, value tuple and returns a {@link DocIdSetIterator} if the field has an entry
+ * for the given value. It has an optimized way of iterating the term dictionary if the terms are
+ * passed in sorted order and makes sure terms and postings are reused as much as possible.
+ */
+ static final class TermDocsIterator {
+ private final TermsProvider provider;
+ private String field;
+ private TermsEnum termsEnum;
+ private PostingsEnum postingsEnum;
+ private final boolean sortedTerms;
+ private BytesRef readerTerm;
+
+ @FunctionalInterface
+ interface TermsProvider {
+ Terms terms(String field) throws IOException;
+ }
+
+ TermDocsIterator(Fields fields, boolean sortedTerms) {
+ this(fields::terms, sortedTerms);
+ }
+
+ TermDocsIterator(LeafReader reader, boolean sortedTerms) {
+ this(reader::terms, sortedTerms);
+ }
+
+ private TermDocsIterator(TermsProvider provider, boolean sortedTerms) {
+ this.sortedTerms = sortedTerms;
+ this.provider = provider;
+ }
+
+ private void nextField(String field) throws IOException {
--- End diff --
Maybe simply name `setField` or `changeField`? `nextField` made me think it was somehow following along in sorted field order or something.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #526: LUCENE-8608: Extract utility class to iterate...
Posted by mikemccand <gi...@git.apache.org>.
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/526#discussion_r241372586
--- Diff: lucene/core/src/java/org/apache/lucene/index/FrozenBufferedUpdates.java ---
@@ -821,4 +759,93 @@ public String toString() {
boolean any() {
return deleteTerms.size() > 0 || deleteQueries.length > 0 || fieldUpdatesCount > 0 ;
}
+
+ /**
+ * This class helps iterating a term dictionary and consuming all the docs for each terms.
+ * It accepts a field, value tuple and returns a {@link DocIdSetIterator} if the field has an entry
+ * for the given value. It has an optimized way of iterating the term dictionary if the terms are
+ * passed in sorted order and makes sure terms and postings are reused as much as possible.
+ */
+ static final class TermDocsIterator {
+ private final TermsProvider provider;
+ private String field;
+ private TermsEnum termsEnum;
+ private PostingsEnum postingsEnum;
+ private final boolean sortedTerms;
+ private BytesRef readerTerm;
+
+ @FunctionalInterface
+ interface TermsProvider {
+ Terms terms(String field) throws IOException;
+ }
+
+ TermDocsIterator(Fields fields, boolean sortedTerms) {
+ this(fields::terms, sortedTerms);
+ }
+
+ TermDocsIterator(LeafReader reader, boolean sortedTerms) {
+ this(reader::terms, sortedTerms);
+ }
+
+ private TermDocsIterator(TermsProvider provider, boolean sortedTerms) {
+ this.sortedTerms = sortedTerms;
+ this.provider = provider;
+ }
+
+ private void nextField(String field) throws IOException {
+ if (this.field == null || this.field.equals(field) == false) {
+ this.field = field;
+ Terms terms = provider.terms(field);
+ if (terms != null) {
+ termsEnum = terms.iterator();
+ if (sortedTerms) {
+ readerTerm = termsEnum.next();
+ }
+ } else {
+ termsEnum = null;
+ }
+ }
+ }
+
+ DocIdSetIterator nextTerm(String field, BytesRef term) throws IOException {
+ nextField(field);
+ if (termsEnum != null) {
+ if (sortedTerms) {
+ // in the sorted case we can take advantage of the "seeking forward" property
--- End diff --
Maybe we should confirm the incoming `term` is `>=` the last term, under assert?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #526: LUCENE-8608: Extract utility class to iterate over t...
Posted by s1monw <gi...@git.apache.org>.
Github user s1monw commented on the issue:
https://github.com/apache/lucene-solr/pull/526
@mikemccand I pushed changes. I think it's ready
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr pull request #526: LUCENE-8608: Extract utility class to iterate...
Posted by mikemccand <gi...@git.apache.org>.
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/526#discussion_r241372770
--- Diff: lucene/core/src/java/org/apache/lucene/index/FrozenBufferedUpdates.java ---
@@ -821,4 +759,93 @@ public String toString() {
boolean any() {
return deleteTerms.size() > 0 || deleteQueries.length > 0 || fieldUpdatesCount > 0 ;
}
+
+ /**
+ * This class helps iterating a term dictionary and consuming all the docs for each terms.
+ * It accepts a field, value tuple and returns a {@link DocIdSetIterator} if the field has an entry
+ * for the given value. It has an optimized way of iterating the term dictionary if the terms are
+ * passed in sorted order and makes sure terms and postings are reused as much as possible.
+ */
+ static final class TermDocsIterator {
+ private final TermsProvider provider;
+ private String field;
+ private TermsEnum termsEnum;
+ private PostingsEnum postingsEnum;
+ private final boolean sortedTerms;
+ private BytesRef readerTerm;
+
+ @FunctionalInterface
+ interface TermsProvider {
+ Terms terms(String field) throws IOException;
+ }
+
+ TermDocsIterator(Fields fields, boolean sortedTerms) {
+ this(fields::terms, sortedTerms);
+ }
+
+ TermDocsIterator(LeafReader reader, boolean sortedTerms) {
+ this(reader::terms, sortedTerms);
+ }
+
+ private TermDocsIterator(TermsProvider provider, boolean sortedTerms) {
+ this.sortedTerms = sortedTerms;
+ this.provider = provider;
+ }
+
+ private void nextField(String field) throws IOException {
+ if (this.field == null || this.field.equals(field) == false) {
+ this.field = field;
+ Terms terms = provider.terms(field);
+ if (terms != null) {
+ termsEnum = terms.iterator();
+ if (sortedTerms) {
+ readerTerm = termsEnum.next();
+ }
+ } else {
+ termsEnum = null;
+ }
+ }
+ }
+
+ DocIdSetIterator nextTerm(String field, BytesRef term) throws IOException {
+ nextField(field);
+ if (termsEnum != null) {
+ if (sortedTerms) {
+ // in the sorted case we can take advantage of the "seeking forward" property
+ // this allows us depending on the term dict impl to reuse data-strucutres internally
--- End diff --
`data-strucutres` -> `data-structures`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org