[GitHub] [lucene-solr] jpountz commented on a change in pull request #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search
jpountz commented on a change in pull request #823: LUCENE-8939: Introduce
Shared Count Early Termination In Parallel Search
URL: https://github.com/apache/lucene-solr/pull/823#discussion_r319987093
##
File path:
lucene/core/src/java/org/apache/lucene/search/HitsThresholdChecker.java
##
@@ -0,0 +1,119 @@
+/*
+ * 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.search;
+
+import java.util.concurrent.atomic.LongAdder;
+import java.util.function.BooleanSupplier;
+
+/**
+ * Used for defining custom algorithms to allow searches to early terminate
+ */
+abstract class HitsThresholdChecker implements BooleanSupplier {
+ /**
+ * Implementation of HitsThresholdChecker which allows global hit counting
+ */
+ private static class GlobalHitsThresholdChecker extends HitsThresholdChecker
{
+private final int totalHitsThreshold;
+private final LongAdder globalHitCount;
Review comment:
AtomicLong would probably perform better. LongAdder usually performs better
to accumulate counts, but the fact that we frequently retrieve the count, which
is slower with LongAdder than AtomicLong, probably kills this benefit.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] jpountz commented on a change in pull request #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search
jpountz commented on a change in pull request #823: LUCENE-8939: Introduce
Shared Count Early Termination In Parallel Search
URL: https://github.com/apache/lucene-solr/pull/823#discussion_r320005574
##
File path:
lucene/core/src/java/org/apache/lucene/search/TopScoreDocCollector.java
##
@@ -191,32 +194,38 @@ public static TopScoreDocCollector create(int numHits,
int totalHitsThreshold) {
* objects.
*/
public static TopScoreDocCollector create(int numHits, ScoreDoc after, int
totalHitsThreshold) {
+return create(numHits, after,
HitsThresholdChecker.create(totalHitsThreshold));
+ }
+
+ public static TopScoreDocCollector create(int numHits, ScoreDoc after,
HitsThresholdChecker hitsThresholdChecker) {
Review comment:
similar comment here
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] jpountz commented on a change in pull request #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search
jpountz commented on a change in pull request #823: LUCENE-8939: Introduce
Shared Count Early Termination In Parallel Search
URL: https://github.com/apache/lucene-solr/pull/823#discussion_r320004946
##
File path: lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java
##
@@ -467,9 +467,19 @@ public TopDocs searchAfter(ScoreDoc after, Query query,
int numHits) throws IOEx
final CollectorManager manager = new
CollectorManager() {
+ private HitsThresholdChecker hitsThresholdChecker;
@Override
public TopScoreDocCollector newCollector() throws IOException {
-return TopScoreDocCollector.create(cappedNumHits, after,
TOTAL_HITS_THRESHOLD);
+
+if (hitsThresholdChecker == null) {
+ if (executor == null || leafSlices.length <= 1) {
+hitsThresholdChecker =
HitsThresholdChecker.create(TOTAL_HITS_THRESHOLD);
+ } else {
+hitsThresholdChecker =
HitsThresholdChecker.createShared(TOTAL_HITS_THRESHOLD);
+ }
+}
Review comment:
can we create it eagerly instead of lazily?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] jpountz commented on a change in pull request #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search
jpountz commented on a change in pull request #823: LUCENE-8939: Introduce
Shared Count Early Termination In Parallel Search
URL: https://github.com/apache/lucene-solr/pull/823#discussion_r320004644
##
File path: lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java
##
@@ -382,8 +383,19 @@ public static TopFieldCollector create(Sort sort, int
numHits, int totalHitsThre
* @return a {@link TopFieldCollector} instance which will sort the results
by
* the sort criteria.
*/
+ public static TopFieldCollector create(Sort sort, int numHits, FieldDoc
after, int totalHitsThreshold) {
+if (totalHitsThreshold < 0) {
+ throw new IllegalArgumentException("totalHitsThreshold must be >= 0, got
" + totalHitsThreshold);
+}
+
+return create(sort, numHits, after,
HitsThresholdChecker.create(totalHitsThreshold));
+ }
+
+ /**
+ * Same as above with an additional parameter to allow passing in the
threshold checker
+ */
public static TopFieldCollector create(Sort sort, int numHits, FieldDoc
after,
- int totalHitsThreshold) {
+ HitsThresholdChecker
hitsThresholdChecker) {
Review comment:
I'd have a preference for not exposing `HitsThresholdChecker` in the
user-facing API and instead providing users with a factory method for a
collector manager that can be used with multiple threads, e.g. `public static
CollectorManager createSharedManager(sort, numHits,
after, totalHitsThreshold)`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] jpountz commented on a change in pull request #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search
jpountz commented on a change in pull request #823: LUCENE-8939: Introduce
Shared Count Early Termination In Parallel Search
URL: https://github.com/apache/lucene-solr/pull/823#discussion_r319961589
##
File path:
lucene/core/src/java/org/apache/lucene/search/HitsThresholdChecker.java
##
@@ -0,0 +1,119 @@
+/*
+ * 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.search;
+
+import java.util.concurrent.atomic.LongAdder;
+import java.util.function.BooleanSupplier;
+
+/**
+ * Used for defining custom algorithms to allow searches to early terminate
+ */
+abstract class HitsThresholdChecker implements BooleanSupplier {
Review comment:
I don't think we actually need to implement BooleanSupplier? Maybe remove it
from the implemented interfaces, which will in turn help use a more explicit
method name?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
