Add a MemorySegment Vector scorer - for scoring without copying on-heap

[ChrisHegarty]

Add a MemorySegment Vector scorer - for scoring without copying on-heap.

The vector scorer loads values directly from the backing memory segment when available. Otherwise, if the vector data spans across segments, or is a query vector, the scorer copies the vector data on-heap.

A benchmark shows ~2x performance improvement of this scorer over the default copy-on-heap scorer. The benchmark need a little more scrutiny and evaluation on different platforms. Here's the results on a Max M2:

Benchmark                                      (size)   Mode  Cnt  Score   Error   Units
VectorScorerBenchmark.binaryDotProductDefault    1024  thrpt    5  1.391 ± 0.016  ops/us
VectorScorerBenchmark.binaryDotProductMemSeg     1024  thrpt    5  3.013 ± 0.207  ops/us

The scorer currently only operates on vectors with an element size of byte, since loading vector data from float[] (the fallback), is only supported in JDK 22. We can evaluate if and how to support floats separately. See https://bugs.openjdk.org/browse/JDK-8318678

The vector scorer is implicitly tied to the Panama Vector Utils implementation - you can only have a Memory segment scorer if the Panama vector implementation is present. There is a little room for improvement in how these things are initialised and structured.

[ChrisHegarty]

I wanna generalise this so we can use it for scalar quantised too - so a vector byte size + N bytes

[benwtrent]

While I don't foresee us doing this, but it is conceivable that the flat vector storage will change.

I think we have adequate test coverage to catch such a weird behavior, but maybe we need to change the name or add a comment reflecting that it relies on how Lucene95 stored the flat vectors?

[uschindler]

I don't like the additional Provider interface. There should only be one instantiating all implementations that can be vectorized.

[msokolov]

So excited to see this finally come to fruition! No more double-buffering!

[uschindler]

Looks much better now. I will have a closer look later, so not yet a +1

[uschindler]

How can i change the review to "undecided"?

[ChrisHegarty]

How can i change the review to "undecided"?

I re-requested ur review - so there is no official reviewer yet. Take ur time. I have some luceneutil benchmarks to run, etc.

[uschindler]

How can i change the review to "undecided"?

I re-requested ur review - so there is no official reviewer yet. Take ur time. I have some luceneutil benchmarks to run, etc.

The benchmark code seems broken after your changes.

Dismissing Uwe's review, since he is undecided. Can be explicitly added later, when we convince him ;-)

[ChrisHegarty]

Dismissing Uwe's review, since he is undecided. Can be explicitly added later, when we convince him ;-)

(Oh, this looks harsh!) I hope that I did this right. If not, I apologise. No offence intended. I just want to reflect @uschindler's comment above about being currently undecided.

[ChrisHegarty]

This test creates a big file, but it's currently the only way to test that the offset calculations are correctly handled without overflow (if implemented using int)!

[uschindler]

Back from vacation. Will look into that till Tuesday!

[uschindler]

To me this looks fine. Very clean.
It's not easy to undertstand, but at some point we may change this to be in core classes and we can get rid of all those wrappers.

One thing to think about: I had another idea, which may also help to allow direct use of our vectors from NIOFSDirectory and ByteBuffersDirectory (used by NRT):

How about changing from MemorySegment to ByteBuffer views? They can also be used by the vectorization code.

This would also allow to have the interface we need in main classes and not required to be in Java 21 soureSet. In addition ByteBuffersDirectory and NIOFSDircetory could directly return bytebuffer views, too.

We may add a method like getByteBufferSlice(....).

What do you think?

[ChrisHegarty]

We may add a method like getByteBufferSlice(....).

I experimented locally with similar before, and the performance impact when converting to/from MemorySegment was horrible. I don't disagree that for NIO/BB dir implementations this could be useful, but it would not intersect with the memory segment implementation, since it performs horribly (when converting from BB to MS, and vice versa).

[uschindler]

We may add a method like getByteBufferSlice(....).

I experimented locally with similar before, and the performance impact when converting to/from MemorySegment was horrible. I don't disagree that for NIO/BB dir implementations this could be useful, but it would not intersect with the memory segment implementation, since it performs horribly (when converting from BB to MS, and vice versa).

OK, this is not good. From my code review it looked like the DirectByteBuffer also only has a addres and therefor the whole thing is identical by performance (except that we have to get a MemorySegment slice first and then call toByteBuffer()). So this is interesting, but then I we can't do anything.

Mabe theres some optimizations miissing in ByteBuffer support of vector API.

So looks fine then. Working on a final review.

[uschindler]

One more question: How can we migrate to a later index format version? Because we have the MemorySegmentFlatVectorScorer (which has no knowledge about version) and this one returns the Lucene99MemorySegment* classes.

So in my opinion this is only solvable if we rename also the MemorySegmentFlatVectorScorer to Lucene99MemorySegmentFlatVectorScorer and also adapt the getter name.

Except this inconsistency it looks fine. A bit hairy but if performance is high prio, I accept this as a step towards getting rid of IndexInput totally.

[uschindler]

now all fine.

[uschindler]

Looks like first Java 22 build also worked fine, so no API incompatibilities in JDK (foreign preview vs final): https://jenkins.thetaphi.de/job/Lucene-main-Linux/48322/consoleText