This consolidates a number of changes. The first is significant
reductions in allocation and copying during UnionInPlace
operations on very sparse containers -- for instance, combining
two array containers with one item each.
We fix up the logic for identifying and handling cases where
only one of the containers being unioned together has a given
key.
We generally favor cloning an existing container over unioning
it into a new empty container.
When unioning two containers, we were using unionIntoTargetSingle
on those two containers, into an empty bitmap. For more, we were
creating an empty bitmap, then unioning all the others into
it; it's faster to clone the first, then union the others into
it.
The overall logic for UnionInPlace is cleaned up and simplified
a bit. However, it's then complexified a bit, because it turns
out that while it's a bad idea to convert single-item arrays to
bitmaps to union them, by a few hundred items, the bitmap
conversion saves a lot of time even if it costs an allocation.
The value of N picked here is sort of arbitrary, but
512 seems to be about right. The big problem is a massive
performance hit in cases where, say, there's only a
couple of items per container, and the bitmap conversion
is extremely expensive. If you wait until N reaches
the array size cap, though, you take a very noticeable
performance hit (can be a factor of 2.5-3 in simple
testing).
We also add some stat counters, and rename an internal
method on the `handledIters` type.
Add a benchmark to test a specific case where UnionInPlace is
underperforming the naive union operation badly.
Also, the UnionBulk test was reusing a bitmap, meaning that it ended
up doing a lot of unions into a bitmap that already had all the
bits it was supposed to have. This broke a couple of other tests
in unexpected ways.
We also now use UnionInPlace in importRoaring, and test it
in the container combinations tests via a wrapper.
UnionInPlace is still heavily affected by
https://github.com/pilosa/pilosa/issues/1875 where containers that exist in an
incoming bitmap can cause massive unnecessary allocations of bitmap containers
when an array of short length is all that's needed.
get the count of the existing fragment and compare it to the incoming bits to
decide which should be unioned into the other. This should generally result in
far fewer allocations, though there is much work that needs to be done within
UnionInPlace to further improve things.
unrelatedly, I added a TODO to change the long-query-time option to move it out
of cluster. It should probably be happening at the API level so that different
handlers can reuse it, but if we're going to do that we'll want to make sure
that any potentially time intensive operations are pulled into api from
handler (e.g. protobuf decoding)
This greatly simplifies the code, and with the recent addition of DirectAddN and
DirectRemoveN should be as or more performant than doing the separate bitmap and
union (in most cases, unsorted data could still be slower). Perhaps more
importantly, it is also less allocation heavy than the union approach. Also
makes it trivial to get the counts of changed bits, so I've cleaned up the stats
to show number of bits we're importing/clearing and the number of bits that
actually changed.
There existed a case where two goroutines would try to
CreateIfNotExists the same fragment, and the first would
create it, but not put it in the fragments table, then
drop the lock, try to broadcast a message, and if it
succeeded then populate the fragments table. The second
would come along during the broadcast, not find an
entry, try to create one, and fail because the file was
already locked.
Basic problem: At least one test in server/ will fail
if we don't delay to send out broadcast messages. Everything
will lock up if we can wait forever (or even just a very
long time) for the message broadcast. We don't ever want
to have an inconsistent state -- so we don't want to either
fail to get a fragment when one's been created, or get one
that's about to be deleted if the broadcast fails.
So, creation and stashing in the fragments table is
atomic and immediate. After that, we optimistically attempt
to broadcast. If we fail, we fail. We delay up to about
50ms for the broadcast to be done, but after that return
anyway. This way, if things are going well everything
works, and if there's unexpected delays, things work except
some nodes in a cluster may not know about available
shards on other nodes sometimes. But that would have
happened anyway. A proper fix is beyond the scope of this
patch.
