[Investigate] Specialize BytesRef for literal byte arrays to save 8 bytes

[msfroh]

Description

I was reviewing a PR that dealt with some Protobuf today and saw that their ByteString class (which is pretty similar to Lucene's BytesRef) is abstract with a pair of concrete subclasses -- LiteralByteString and BoundedByteString. The BoundedByteString has length and offset members, while LiteralByteString is just a wrapper around byte[]. As a result, LiteralByteString is a whole 8 bytes smaller. Woohoo!

This got me thinking -- how many BytesRef instances out there have offset == 0 and length == bytes.length?

Within a lot of "hot" Lucene code, I believe the answer is "not many", since we do a very good job of reusing BytesRef instances forever. That said, all of the Term constructors end up producing BytesRefs of known (fixed) length. So the potential benefit is clearly non-zero. (Maybe close to zero?)

I'm thinking of trying to make BytesRef abstract and sealed with a pair of subclasses, similar to the Protobuf approach. Obviously, this means replacing direct field access with getters (and setters), but I think those can be bimorphically inlined.

[dweiss]

The costs of polymorphism may outweigh the cost of memory savings here. Hard to tell without trying.

[rmuir]

This class should only be used as a pointer (BytesREF) and not as a string class. Why would you have many many pointers where such a savings would help so much?

[msfroh]

The costs of polymorphism may outweigh the cost of memory savings here. Hard to tell without trying.

I'm also concerned about this one. My knowledge of Java inlining mostly comes from https://blunders.io/posts/es-benchmark-4-inlining, but I get the impression that if there are 1 or 2 implementations, then things will eventually get inlined. Making the class sealed with exactly two implementations is important to make sure we don't end up hitting the megamorphic case. Of course, the bimorphic inlining is probably still a little more expensive than accessing a public field. (I can't imagine that it would ever be cheaper.)

This class should only be used as a pointer (BytesREF) and not as a string class. Why would you have many many pointers where such a savings would help so much?

Yeah... that's where I'm concerned that it may be a waste of time.

Within Lucene, I think BytesRefs fall into two buckets:

1. BytesRefs that hold or wrap long-lived resizeable, reused buffers. For these, the offset and length are key to reuse (and we pay the 8 bytes anyway).
2. Really short-lived BytesRefs created from user input. Examples are in Term, KeywordField, the various pack methods in the *Point classes.

The first bucket needs to spend the extra 8 bytes anyway, and there aren't a lot of them. The second bucket is where we could save some bytes (since we're allocating a bunch of variable-length wrappers for fixed byte arrays), but the objects are all so short-lived that it might not matter.