[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-528262274 Thank you for merging and reviewing, @jpountz ! and thank you @jimczi and @mikemccand for reviewing! --

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-528237811 Thanks for approving, @mikemccand ! @jpountz , @jimczi Does this look ok to merge now? -

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-527765667 Folks, post the discussion above, I am assuming this is ready to merge? ---

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-527515974 > In the unsorted index case, where we skip by impacts once we collect more than the 1000 by default, we ar

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-527464137 @jpountz I updated the PR per your comments -- please take a look and let me know if it seems fine. --

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-527285002 > > https://issues.apache.org/jira/browse/LUCENE-8681 > > I am not sure I see how that solves the problem

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-527284505 > https://issues.apache.org/jira/browse/LUCENE-8681 I am not sure I see how that solves the problem?

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-527109501 > > I think the output I pasted above does mention the tasks run? > > Hmm the first few `luceneutil` resu

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526845639 > I'm trying to understand the behavior change Lucene users will see with this, when using concurrent searching

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526845091 > Please don't run "real" benchmarks with `wikimedium10k` -- that may be fast to execute but the results are no

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526540019 Thanks @jimczi ! This is an automated message

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526179514 @jimczi Updated per our discussion. Although, a user can potentially implement `HitsThresholdChecker` and give

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526164337 > Package protected might not be a good idea after all, sorry, users that wants to set the `totalHitsThreshold`

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526157715 @jimczi Updated This is an automated message f

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526152703 Ran precommit on the latest iteration -- came in clean. @jimczi Looks good to be committed?

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526122213 > > Also, I believe having the abstraction will let future implementations customize the threshold logic withou

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526113533 > > So, essentially, we make implicit CollectorManager implementations owned by IndexSearcher instead of separa

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526102145 > > That would require a CollectorManager implementation > > Not necessarily, you could create a `GlobalS

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-526075360 > I wonder if the shared count should be used automatically in `IndexSearcher#searchAfter` if the executor is n

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-525754830 @jimczi Thanks, I updated per your comments. Please let me know if it seems fine. RE: benchmarks, the nu

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-525720523 @jimczi Updated the PR with implementation for `TopScoreDocsCollector'. Haven't added the corresponding `Collec

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-525695991 > > Looks like the performance is consistent and we see no degradation. WDYT? > > I don't think that thes

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-525634541 I opened a JIRA for the follow up : https://issues.apache.org/jira/browse/LUCENE-8958 Will post a patch

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-525287168 @jimczi Thanks for reviewing the PR. I will follow up with a JIRA and a PR for `TopScoreDocCollector` th

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-524725795 @jimczi Looks like I partially misunderstood your earlier comments -- sorry about that. I have raised a version

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-524385885 > The reason for that is since `AtomicInteger.get()` is not guaranteed to be thread safe, so that was a hack wa

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-524382441 > I don't see what the modification is. The early termination logic is the same, the only diff is that the

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-524335774 > Sorry I don't follow. The logic for early termination should be the same in `TopFieldCollector` than the one

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-524332090 @jimczi I did consider doing that, but went down this route to explicitly decouple the early termination code p

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-524157568 Any thoughts on this one? This seems like a reasonable way to avoid collecting extra hits with minimal overhead

[GitHub] [lucene-solr] atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search

atris commented on issue #823: LUCENE-8939: Introduce Shared Count Early Termination In Parallel Search URL: https://github.com/apache/lucene-solr/pull/823#issuecomment-518522813 cc @jpountz This is an automated message from