We add a new ops log type(pair), AddRoaring and RemoveRoaring,
which set and clear the bits from a provided roaring bitmap.
This also compels us to consider additional sanity checking
during tests.
This patch replaces a lot of circumstances in which containers
were being copied with circumstances in which they are shared,
using copy-on-write semantics.
To achieve this, we emulate somewhat the design of go's
native `append` function. Operations on a container may optionally
yield a new container. A container can be marked "frozen",
after which no operation should ever write to it in any way;
that applies both to the container itself and the backing store
it refers to, if any. So for instance, instead of:
c.arrayToBitmap()
we now write:
c = c.arrayToBitmap()
Operations which need to modify a container in any way
need to be able to return a new container, which is a modified
copy of the previous container. This applies to operations
like add/remove, but also to things like unmapping memory-mapped
storage, or changing a container's type.
Bitmaps do not support the same copy-on-write semantics,
currently, but "copying" a bitmap and sharing the containers
instead of duplicating them is *much* cheaper than copying
the containers.
Bitmaps do support a .Freeze method, which currently copies
the previous bitmap, making a new one with the same container
pointers, and freezes the individual containers. Use this
if you need a writeable copy of a bitmap -- the resulting
bitmap can safely have its set of containers modified, and
bitmap operators that would want to modify the containers
will use copy-on-write for that.
The primary motivation of this is to reduce the cost of the
row cache used by fragments. As a secondary issue, the row cache
is no longer updated on writes -- that update was actually a
race condition waiting to happen. Rather, writes to a row
invalidate the cache entry for that row. The row cache is
created by creating a new bitmap, and freezing the relevant
containers from the fragment's storage. In the case where
nothing is being written, the row cache grows to contain
bitmaps containing all those containers, but never copies
any containers. If nothing's being read, the row cache is
never created, and the containers are in general not getting
frozen. The only circumstance where copies have to happen is
when things are read (and thus stored in the row cache) and
later modified. In that case, each read freezes objects, and
the first write to a container after it's been frozen will
create a new copy.
We drop the enterprise/b btree implementation, because we
don't really need it anymore -- we now provide that
implementation by default in the open source product anyway.
Along with this, there's a lot of other changes which
improve support for nil containers, as a cheaper representation
for empty containers. Operations which we know will provide
an empty container can always short-circuit and just yield
a nil *Container. Similarly, operations which would provide
a full container can return a single shared full container
object (which is frozen). The higher-level (non type-specific)
container ops are now using that logic to short-circuit
operations for empty and full containers. (For instance,
difference of anything minus an empty container is the
original thing, union of anything and empty is the original
thing, and so on.)
The Containers interface adds "Update" and "UpdateEvery"
methods, based in part on the "Put" interface provided
by the underlying btree implementation; Update performs
a possible update in-place of a container for a given
key, bypassing the need to replicate the search for that
key in the container. UpdateEvery loops through all the
containers.
Containers do not strictly guarantee that they won't
return nil `*Container` objects. However, the container
iterators won't return those -- empty containers aren't
interesting. Some tests are updated to reflect this.
Some of the container internals, like N(), or the isArray()
and related functions, accept nil container pointers. Some,
like Thaw(), do not. For the array(), bitmap(), and runs()
methods, roaringparanoia enables an explicit panic on a nil
container explaining the problem, but the intent is that those
should never be called unless you already know you have the
right kind of container, so by default they don't perform
the extra checks. In most cases, this is already covered
because a nil container is empty, and there's no operation
we can perform that requires us to inspect the contents of
an empty container. This is passing a fair amount of testing,
but the testing may not be comprehensive enough.
The overall impact of this is pretty trivial performance-wise.
In our default roaring/ benchmarks, a few things get a few
percent faster, or slower. The advantage is that, with
read-heavy workloads, the row cache no longer eats up incredible
amounts of memory.
For a smallish test case, pilosa's memory usage (RES in top) after
startup was ~2.5GB. Without this patch, simply reading every
row a few times got memory usage to about 9GB, which seemed
reasonably stable. With this patch, memory usage went to about
3GB. This will be less noticeable in mixed read/write loads,
but it should be consistently significantly lower.
In addition to dropping things from the rowCache on modifications,
we also stopped performing a full count on a modified row when
not using a cache of a kind that would use that count, and don't
repopulate the rowCache regardless. We don't want every write
to imply a corresponding read after it.
There's a lot of room for possible future optimizations in
terms of things like in-place operations, and some of the
row/rowSegment code is a little suspicious to me, but I don't
think it should be *worse* in any cases.
So with the switch to a new linter, we get a lot of new warnings,
and the majority of them are harmless probably, but a few might be
real. Variously just use _ to suppress warnings, or report errors.
There's probably things here that deserve better fixes, but we can
always revisit it.
Since we always use the same cmp function, we don't need to
actually *call* a function -- we can just inline it. Or, in
fact, omit the computation entirely; comparing the result of
the subtraction to zero is (very slightly) more expensive than
comparing the magnitudes of two numbers.
Also fix a spurious comment and gofmt issues.
The upstream btree code has a test file, this is an import
of that test file, adjusted/adapted to make it work with our
de-genericized uint64/*Container implementation, so we have some
tests and benchmarks available for the btree implementation itself.
Did some benchmarking with b+tree values. The actual interactions
appear to be slightly inconsistent; some values seem to help more
in cases with higher OpN in benchmarks, others with lower OpN. It
appears that the practical consideration may be what happens
when a snapshot gets triggered; smaller kx/kd appear to reduce
costs there, but increase costs between snapshots. This is a
bit of guesswork.
Numbers are slighly under powers of 2, because that means that the
total actual sizes of k and d end up fitting nicely in alloc
pool sizes.
