Correct termBytePool reference in FreqProxTermsEnum and clarify pool sharing

[neoremind]

FreqProxTermsEnum is the bridge between the in-memory hash+pool and the codec writer, it uses termsPool to retrieve real term bytes when iterating terms in lexicographical order during segment flush, but the constructor passes terms.bytePool where it should be terms.termBytePool. There is no problem now because we do pool sharing today, we use the same ByteBlockPool to store both term bytes and their corresponding postings data, so the two 1) terms.bytePool (major responsibility is to store terms' posting data in memory) and 2) terms.termBytePool (used to store terms' bytes) point to the same pool. Semantically speaking, FreqProxTermsEnum's constructor should use the right reference even though they refer to the same pool.

Add a bit clarification of how the in-memory things before segment flush work based on my investigation. The below class diagram shows the relationship between TermsHash, BytesRefHash and the relevant pools.

TermsHash maintains three pool references:

• intPool (IntBlockPool): Stores per-term posting streams' write cursors. Each cursor tracks the offset in bytePool where the next postings write should go. For example, when we store one field's term M in doc N, the cursor directs to where term M's doc N's doc ID delta, freq, and pos should get persisted.
• bytePool (ByteBlockPool): Today we do pool sharing, it stores both term bytes and postings streams (doc ID delta, freq, pos, offsets, payloads) together in the same pool.
• termBytePool (ByteBlockPool): This points to the same bytePool above.

BytesRefHash:
For each term, it stores a bytesStart offset pointing into termBytePool, and supports fast hash-based find/add operations to dedup terms.

IntBlockPool:
Each 4-bytes represents a write cursor, the current position in bytePool where the next byte for that term's stream posting data should be written to.

ByteBlockPool:
We interleave term bytes and their respective postings data in this single pool now. The postings data for each term forms a linked list of growing slices. For frequent terms like "the", "a", "and", the linked list grows long, span multiple slices, they are not contiguous, usually hop. For rare terms, usually just the term length prefix + term bytes + a 5-byte first postings slice.

[msokolov]

to my way of thinking, we should eliminate the confusing dual reference to the same object and replace it with a single reference? Is there some reason to maintain two distinct references to the same underlying pool?

[neoremind]

@msokolov thanks, good callout! I searched the codebase, this was introduced back in 2010/07 in this commit.

There seems no specific reason. But my guess is that originally the two pools were separate but merged later. I can prove that as it's fairly easy to separate them by modifying several lines of code and lucene works well offline on my local machine. By merging them, in theory, from my view, one clear win is that when we add terms, the term bytes and the postings slice are adjacent, so they reside in one cache line during the initial write, also we can avoid another indirection of pool reference as well. However, the downside of merging them is that, as I illustrated in #11608 (comment), for frequently seen terms, the BytesRefHash lookup has to compare term bytes, with a shared pool, the cache line it pulls also carries postings slices leading to potential cache thrashing; also during the sorting phase before flush, the radix sort walks over term bytes many times, interleaved design might be bad for spatial locality as sorting only cares about terms not posting.

I plan to try to separate the terms' bytes and terms' posting pool, and vet with luceneutil wikipedia benchmark and some micro JMH benchmarks spanning the UUID case (many unique terms) and the wikipedia-ish frequent-term case to see if this can be a generic one or not. Will share results once I finish.

[neoremind]

@msokolov I've investigated what if we make the two pools separate and shared the full findings with benchmarks in #16574, seeking guidance now, would love your thoughts as well. As for this PR, the semantic correction is still a valid one and would be a premise if we go with separation; I can pause the PR for getting enough feedback from #16574 to further see to removing the dual reference or instead we keep them separate genuinely.