Can we use Collections.singletonMap when there are no partitions?
Done. Note though that BytesRef cannot be reused in the case of PerDimensionIndexingParams (i.e. multiple CLPs). This is not the common case, but it's not trivial to specialize it. Maybe as a second iteration. I did put a TODO in FacetFields to allow reuse.
why do we have VInt8.bytesNeeded? Who uses that?
Currently no one uses it, but it was there and I thought that it's a convenient API to keep. Why encode and then see how many bytes were occupied?
Anyway, neither the encoders nor the decoders use it. I have no strong feelings for keeping/removing it, so if you feel like it should be removed, I can do it.
Hmm, it's a little abusive how VInt8.decode changes the offset of the incoming BytesRef
It is, but that's the result of Java's lack of pass by reference. I.e., decode needs to return the caller two values: the decoded number and how many bytes were read.
Notice that in the previous byte variant, the method took a class Position, which is horrible. That's why I documented in decode() that it advances bytes.offset, so
the caller can restore it in the end. For instance, IntDecoder restores the offset to the original one in the end.
On LUCENE-4675 Robert gave me an idea to create a BytesRefIterator, and I started to play with it. I.e. it would wrap a BytesRef but add 'pos' and 'upto' indexes.
The user can modify 'pos' freely, withouth touching bytes.offset. That introduces an object allocation though, and since I'd want to reuse that object wherever
possible, I think I'll look at it after finishing this issue. It already contains too many changes.
I guess this is why you want an upto
No, I wanted upto because iterating up to bytes.length is incorrect. You need to iterate up to offset+length. BytesRefIterator.pos and BytesRefIterator.upto solve these cases for me.
looks like things got a bit slower (or possibly it's noise)
First, even if it's not noise, the slowdown IMO is worth the code simplification. But, I do believe that we'll see gains when there are more than 3 integers to encode/decode.
In fact, the facets test package has an EncodingSpeed class which measures the time it takes to encode/decode a large number of integers (a few thousands). When I compared the
result to 4x (i.e. without the patch), the decode time seemed to be ~x5 faster.
In this patch I added an Ant task "run-encoding-benchmark" which runs this class. Want to give it a try on your beast machine? For 4x, you can just copy the target to lucene/facet/build.xml, I believe it will work without issues.