Optimize allocations and copies for SortedRanges.insert when it is effectively an append

[jcferretti]

Related to DHE https://github.com/deephaven-ent/iris/pull/651 and https://deephaven.atlassian.net/browse/DH-15310

The testing for this change also found a pre-existing bug in SortedRangesTyped.java.
The method ensureCanAppend is missing a check to ensure a packed range (eg for SortedRangesShort) doesn't overflow.
DB tests began failing because with the new code this old bug was being hit.

     @Override
     protected final SortedRanges ensureCanAppend(final int newLastPos, final long unpackedNewLastKey, final boolean writeCheck) {
         final long absUnpackedLastKey = Math.abs(unpackedNewLastKey);
+        if (!fitsForAppend(absUnpackedLastKey)) {
+            return null;
+        }

[rcaudy]

Some pedantic suggestions, an optimization question, and a suggestion for further optimization. Change looks correct as-is, however.

[rcaudy]

I believe we can optimize the same way if our last range overlaps other's last range, right? Worth a little more complexity?

[jcferretti]

I thought about it but punted due to complexity; I'd like the delta to go to Bard and Genesis so on the enterprise PR I think that is a good tradeoff. We could just do more here in the community PR, and also port that to vermillion in enterprise once the Bard fix rolls forward to vermillion... but the truth is I shouldn't be working on any of this due to priorities, so I don't want to put another couple of hours just now. Is not obvious to me how common that additional optimization would be hit either; maybe one of you guys Ryan or Charles can make an argument from the perspective of how some particular operation works (eg, byExternal for merges, join in other cases or whatever).

[rcaudy]

I think this is a reasonable answer. I think the case you've optimized is common. I'm certain we could also handle the overlapping case at the cost of a bit more complexity, but I'm not sure that a single overlapping range is much more common than general overlapping.

[rcaudy]

This looks like a candidate for an array copy, but I assume it's written this way to address different offsets and different internal widths?

[jcferretti]

Yeah, because essentially we are not guaranteed that our two sortedRanges are of the same underlying "type" (eg if they are SortedRangesInt or SortedRangesShort, the unpack part may apply an offset and other fun).
We "could" [TM] optimize to array copy if both are SortedRangesLong (no offset to apply in that case).

[rcaudy]

Ack. This is exactly what I expected.