No need to go through the indirection of 2 wrapped functions, just put the logic in plain
methods. Also, we can just outright set the field if there's no executor.
Our collector managers have a `supportsConcurrency` flag to optimize the case
when they are used in a single thread. This PR proposes to remove this flag now
that the optimization doesn't do much as a result of #13943.
This reverts commit 1ee4f8a111.
We have observed performance regressions that can be linked to #13221. We will need to revise the logic that such change introduced in main and branch_10x. While we do so, I propose that we bake it out of branch_10_0 and we release Lucene 10 without it.
Closes#13856
1. Rearrange/rename the parameters to be more idiomatic (e.g., follow conventions of Arrays#... methods)
2. Add assert to ensure expected sortedness we may rely on in the future (so we're not trappy)
3. Migrate PostingsReader to call VectorUtil instead of VectorUtilSupport (so it benefits from the common assert)
In Lucene 8.4, we updated postings to work on long[] arrays internally. This
allowed us to workaround the lack of explicit vectorization (auto-vectorization
doesn't detect all the scenarios that we would like to handle) support in the
JVM by summing up two integers in one operation for instance.
With explicit vectorization now available, it looks like we can get more
benefits from the ability to compare multiple intetgers in one operations than
from summing up two integers in one operation. Moving back to ints helps
compare 2x more integers at once vs. longs.
PR #13692 tried to speed up advancing by using branchless binary search, but while this yielded a speedup on my machine, this yielded a slowdown on nightly benchmarks.
This PR tries a different approach using vectorization. Experimentation suggests that it speeds up queries that advance to the next few doc IDs, such as `AndHighHigh`.
127 times out of 128, nextDoc() returns the next doc ID in the buffer.
Currently, we check if the current doc is equal to the last doc ID in the block
to know if we need to refill. We can do better by comparing the current index
in the block with the block size, which is a bit more efficient since the
latter is a constant.
`TopScoreDocCollectorManager` has a dependency on `HitsThresholdChecker`, which
is essentially a shared counter that is incremented until it reaches the total
hits threshold, when the scorer can start dynamically pruning hits.
A consequence of this removal is that dynamic pruning may start later, as soon
as:
- either the current slice collected `totalHitsThreshold` hits,
- or another slice collected `totalHitsThreshold` hits and the current slice
collected enough hits (up to 1,024) to check the shared
`MaxScoreAccumulator`.
So in short, it exchanges a bit more work globally in favor of a bit less
contention. A longer-term goal of mine is to stop specializing our
`CollectorManager`s based on whether they are going to be used concurrently or
not.
`LeafSimScorer` is a specialization of a `SimScorer` for a given segment. It
doesn't add much value, but benchmarks suggest that it adds measurable overhead
to queries sorted by score.
When `totalHitsThreshold` is `Integer.MAX_VALUE`, dynamic pruning is never used
and all hits get evaluated. Thus, the minimum competitive score always stays at
zero, and there is nothing to exchange across slices.
Currently, we traverse the BKD tree or perform a binary search using DocValues first, and then check whether the count can be obtained in the count() method of IndexSortSortedNumericDocValuesRangeQuery.
we should consider providing a mechanism to perform this check beforehand, avoid unnecessary processing when dealing with a sparseRange
It is sometimes possible for `MaxScoreBulkScorer` to compute windows that don't
contain many candidate matches, resulting in more time spent evaluating maximum
scores per window than evaluating candidate matches on this window.
This PR introduces a heuristic that tries to require at least 32 candidate
matches per clause per window to amortize the per-window overhead. This results
in a speedup for the `OrMany` task.
This comes from observations on https://tantivy-search.github.io/bench/ for
exhaustive evaluation like `TOP_100_COUNT`. `collect()` is often inlined, but
other methods that we'd like to see inlined like `PostingsEnum#nextDoc()` are
not always inlined. This PR decreases the compiled size of `collect()` to make
more room for other methods to be inlined.
It does so by moving an assertion to `AssertingScorable` and extracting an
uncommon code path to a method.
I was looking at some queries where Lucene performs significantly worse than
Tantivy at https://tantivy-search.github.io/bench/, and found out that we get
quite some overhead from implementing `BooleanScorer` on top of `BulkScorer`
(effectively implemented by `DefaultBulkScorer` since it only runs term queries
as boolean clauses) rather than `Scorer` directly.
The `CountOrHighHigh` and `CountOrHighMed` tasks are a bit noisy on my machine,
so I did 3 runs on wikibigall, and all of them had speedups for these two
tasks, often with a very low p-value.
In theory, this change could make things slower when the inner query has a
specialized bulk scorer, such as `MatchAllDocsQuery` or a conjunction. It does
feel right to optimize for term queries though.
Follow-up to #13871, getting another speedup from relatively trivial changes:
* avoid redundant `end()` call by directly storing the end value for sub-iterator that we don't use for anything else
* also save most `get(...)` calls for this sub-iterator
* avoid redundant `start()` call by grabbing `start()` directly from `nextInterval`
* replace `getFirst()` with `get(0)`, it looks nice but has needless overhead in my testing (not sure why, but profiling clearly shows it to be slower, maybe just a result of having `get()`'s code hot in the cache with a higher likelihood or something esoteric like that)
* avoid using an iterator for loop for a random access list, this is probably the biggest win in this PR
Generate cleaner code for PForUtil that has no dead parameters.
Also:
PForUtil instances always create their own `ForUtil`, so we can inline
that into the field declaration. Also, we can save cycles
for accessing the input on PostingsDecodingUtil.
Surprisingly, the combination of these cleanups yields a small but
statistically fully visible speedup that the compiler isn't able to get
to on its own it seems.
`BlockMaxConjunctionBulkScorer` only checks if it can early exit based on
impacts once per window, and windows are computed using impact blocks of the
leading clause. So this logic is defeated if the leading clause produces a
single block (e.g. `ConstantScoreQuery`). This commit addresses this problem by
artificially lowering the window size to contain ~128 docs of the leading
clause.
MaxScoreBulkScorer partitions scorers into a set of essential scorers and a set
of non-essential scorers, depending on the maximum scores produced by scorers
and on the current minimum competitive score. An increase of the minimum
competitive score has the potential to yield a more favorable partitioning, but
repartitioning can also be expensive.
In order to repartition when necessary while avoiding to repartition too often,
this PR tracks the minimum value of the minimum competitive score that would
produce a more favorable partitioning, and repartitions scorers whenever the
minimum competitive score exceeds this threshold.
This commit tries to save calls to `madvise` which are not necessary, either
because they map to the OS' default, or because the advice would be overridden
later on anyway. I have not noticed specific problems with this, but it seems
desirable to keep calls to `madvise` to a minimum.
As a consequence:
- Files that are open with `ReadAdvice.NORMAL` do not call `madvise` since
this is the OS' default.
- Compound files are always open with `ReadAdvice.NORMAL`, and the actual is
only set when opening sub files of these compound files.
To make the latter less trappy, the `IOContext` parameter has been removed from
`CompoundFormat#getCompoundReader`.
We updated TestGenerateBwcIndices to create int7 HNSW indices instead of int8 with #13874.
The corresponding python code part of the release wizard needs to be updated accordingly.
It's in the title, extracting shared parts across both classes.
Almost exclusively mechanical changes with the exception of the introduction
of an array summing util.
Backport of #13895 to 10x
This is a continuation of #13588, where we avoided allocating liveDocs
for segments that have the __soft_deletes field but no values in it.
However, that PR only addressed the reading side. This change fixes the
writing scenario with IndexWriter.
Relates #13588
These two share a lot of code, in particular the impacts implementation is 100% identical.
We can save a lot of code and potentially some cycles for method invocations by
drying things up. The changes are just mechanical field movements with the following exceptions:
1. One of the two implementations was using a bytes ref builder, one a bytes ref for holding the
serialized impacts. The `BytesRef` variant is faster so I used that for both when extracting.
2. Some simple arithmetic simplifications around the levels that should be obvious.
3. Removed the the logic for an index without positions in `BlockImpactsPostingsEnum`, that was dead code,
we only set this thing up if there's positions.
Lazy initialize these fields. They consume/cause a lot of memory/GC because they are
allocated frequently (~7% of all allocations in luceneutil's wikimedia medium run for me).
This does not cause any measurable slowdown as far as runtime is concerned and since these are not
even needed for all instances (in fact they are rarely used in the queries the benchmark explores)
save qutie a few CPU cycles for collecting and allocating them.
* Make generated archive files reproducible
This should ensure deterministic archive files and fix issues with changing checksums even
though the codebase has not changed
Adrien Grand
Armin Braun
Ignacio Vera
Luca Cavanna
Greg Miller
panguixin
ljak
Benjamin Trent
Shubham Chaudhary
Lu Xugang
linfn
zhouhui
Nhat Nguyen
Prudhvi Godithi
Dawid Weiss
Rene Groeschke