LantaoJin opened a new pull request, #16812:
URL: https://github.com/apache/lucene/pull/16812
### Description
`NativeVectorUtilSupport` (#15508) binds 22 functions from the optional
`dotProduct` library, but only `dotProduct` has a declared C signature. A
library that implements the other functions with the signatures their Java
names suggest runs into four problems. None of them shows with the example
library in `lucene/misc/src/c`, because it exports only `dotProduct`.
1. The pairwise helpers pass `a.byteSize()` as the length. For
`dotProductFloat`, `squareDistanceFloat` and `cosineFloat` that is four times
the element count, so a C loop over `n` elements reads past both heap arrays,
which are passed with `Linker.Option.critical(true)`. With 4-element float
arrays the dot product was 4010.0 instead of 10.0, because it read the next
array on the heap; with two 4M-element arrays and a 64 MB heap the JVM crashed
with SIGSEGV inside the native function.
2. Byte `cosine` is bound with an `int`-returning descriptor, so a
fractional result is lost: a C function returning `0.5f` came back as about
-9.3E7.
3. `l2normalize` ignores the native return value, so a native implementation
cannot report a zero vector, and `VectorUtil.l2normalize(new float[3])` returns
instead of throwing `IllegalArgumentException`.
4. Symbols are looked up by generic names (`cosine`, `dotProduct`, ...) in
every library loaded by the class loader and then in the linker's default
lookup, so an unrelated library could supply them.
This PR:
- passes the element count to the three float functions. A single length
argument is now always the element count of the first array argument, which is
what every other function already received;
- binds byte `cosine` as `float`-returning;
- defines that native `l2normalize` returns `NULL` for a zero vector and
leaves it unchanged; Java then throws when `throwOnZero` is set;
- prefixes all symbols with `lucene_` and looks them up only with
`SymbolLookup.loaderLookup()`;
- declares all 22 prototypes in `lucene/misc/src/c/dotProduct.h`, with the
length and layout of each argument, and renames the example's symbol to
`lucene_dotProduct`.
Note: This overlaps with #16795 only in the byte cosine fix. #16795
restructures how the handles are held, so the two PRs conflict in
`NativeVectorUtilSupport`; if #16795 merges first, I will rebase this PR onto
its structure, where the symbol lookup change moves into its `NativeLibrary`
holder.
**Testing.** CI never loads a native library, so the existing tests do not
reach this class, and this PR adds no new tests: substituting the native
handles in tests is the subject of #15840, which #16795 is addressing. If this
PR merges first, #16795's Java-backed handles need to expect element counts for
the float functions. I verified the change by running the existing tests in
native mode against a scalar C library that implements 17 of the 22 functions
to the new contract and counts its calls (the library is not part of this PR):
```
./gradlew :lucene:core:test --tests
'org.apache.lucene.internal.vectorization.*' \
--tests 'org.apache.lucene.util.TestVectorUtil' -Ptests.seed=DEADBEEF
-Ptests.vectorsize=default \
-Ptests.jvmargs="-Dlucene.useNativeDotProduct=true
-Djava.library.path=<dir>"
```
| Build | Library symbols | Result (364 tests) |
|---|---|---|
| main | unprefixed | 86 failures: float vectors, byte cosine (including
`TestVectorScorer`), `testNormalizeZeroThrows` |
| this PR | `lucene_` | 0 failures, all 17 functions called natively |
| this PR | unprefixed | 0 failures, no native calls |
With the prefixed library, the full `:lucene:core:test` in native mode also
passes (8,849 tests). The same tests pass with the example library, with
default build options and with `-Ptests.defaultvectorization=true`, and
`./gradlew :lucene:core:check :lucene:misc:check -x test
-Pvalidation.errorprone=true` passes. Tested on Linux aarch64 (Neoverse-N1)
with JDK 25.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
