Chris Hegarty comments

Results 113 comments of


                                            Chris Hegarty

Make HNSW merges cheaper on heap

The on-heap memory used for the per-node neighbour array during building the HNSW graph has been significantly reduced, by approximately 3-4x, see https://github.com/apache/lucene/pull/14527.

Remove vector values copy() methods, moving IndexInput.clone() and temp storage into lower-level interfaces

> that we instead have a mutable Scorer that can accept a new target vector. Yes, that is something that I've noodled on for a while now too - a...

Remove vector values copy() methods, moving IndexInput.clone() and temp storage into lower-level interfaces

> Maybe we could add a RandomVectorScorer.setTarget(int node) method that would only be implemented by the Scorers returned from ScorerSuppliers? Let's defer a double addressing scorer to follow on change,...

Remove vector values copy() methods, moving IndexInput.clone() and temp storage into lower-level interfaces

FYI - I created the following issue to track the possibility of adding a scorer interface that scores one ordinal against another, without the need to create an instance of...

Remove vector values copy() methods, moving IndexInput.clone() and temp storage into lower-level interfaces

thanks @msokolov I'll take another look at why the off-heap scorer is allocating so much.

Remove vector values copy() methods, moving IndexInput.clone() and temp storage into lower-level interfaces

The threading and reuse model in the current API is subtle, but works well, as it mostly minimises the potential for garbage. Looking at this again, it's almost like anywhere...

Remove vector values copy() methods, moving IndexInput.clone() and temp storage into lower-level interfaces

@msokolov ok, this is that I was thinking - [f1e0007](https://github.com/apache/lucene/pull/13872/commits/f1e0007b15245db3a048ebf5bf340e70929f21b7). There might be other places too, this was just a start to see how it fairs in benchmarks.

Remove vector values copy() methods, moving IndexInput.clone() and temp storage into lower-level interfaces

Looking again in a bit more detail, I dislike that it is not possible to get access to the vector values without *always* creating at least one slice. Previously this...

Remove vector values copy() methods, moving IndexInput.clone() and temp storage into lower-level interfaces

I am now completely confused by this change. And I think that my last commit broke the threading model - even though all tests pass!. I'm not saying that we...

Remove vector values copy() methods, moving IndexInput.clone() and temp storage into lower-level interfaces

>As a side note, I see that IndexInput is Closeable -- but I don't think we ever close() the ones we allocate? Is this a problem? Not a problem per...