Skip to content

Commit 08464f0

Browse files
authored
Merge pull request #14 from weaviate/or_performance_improvements_p2
performance: Or method - dynamic newContainer vs insertAt
2 parents 4c33a1b + 104f1ff commit 08464f0

File tree

1 file changed

+22
-25
lines changed

1 file changed

+22
-25
lines changed

bitmap_opt.go

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -284,12 +284,11 @@ func orContainers(a, b, res *Bitmap, buf []uint16) {
284284
off = b.keys.val(bi)
285285
bc := b.getContainer(off)
286286
if c := containerOrAlt(ac, bc, buf, 0); len(c) > 0 && getCardinality(c) > 0 {
287-
// create a new container and update the key offset to this container.
288-
289287
// Since buffer is used in containers merge, result container has to be copied
290-
// to the bitmap immediately and to let buffer be reused for next merge.
291-
// Therefore container can not be copied at the end of method execution like
292-
// other containers from bitmaps a or b.
288+
// to the bitmap immediately to let buffer be reused in next merge,
289+
// contrary to unique containers from bitmap a and b copied at the end of method execution
290+
291+
// create a new container and update the key offset to this container.
293292
offset := res.newContainerNoClr(uint16(len(c)))
294293
copy(res.data[offset:], c)
295294
res.setKey(ak, offset)
@@ -399,26 +398,24 @@ func orContainersInRange(a, b *Bitmap, bi, bn int, buf []uint16) {
399398
boff := b.keys.val(bi)
400399
bc := b.getContainer(boff)
401400
if c := containerOrAlt(ac, bc, buf, runInline); len(c) > 0 {
402-
// Previously merged container were replacing the old one,
403-
// first moving data to the right to free enough space for the
404-
// merged container to fit.
405-
// That solution turned out to be slower for large datasets than
406-
// appending bitmap with entirely new container, as moving data
407-
// is not needed in that case.
408-
// Reference to prev container is then forgotten resulting in
409-
// memory not being used optimally.
410-
411401
// Since buffer is used in containers merge, result container has to be copied
412-
// to the bitmap immediately and to let buffer be reused for next merge.
413-
// Therefore container can not be copied at the end of method execution like
414-
// other containers from bitmap b.
415-
offset := a.newContainerNoClr(uint16(len(c)))
416-
copy(a.data[offset:], c)
417-
a.setKey(ak, offset)
418-
419-
// // make room for container, replacing smaller one and update key offset to new container.
420-
// a.insertAt(aoff, c)
421-
// a.setKey(ak, aoff)
402+
// to the bitmap immediately to let buffer be reused in next merge,
403+
// contrary to unique containers from bitmap b copied at the end of method execution
404+
405+
// Replacing previous container with merged one, that requires moving data
406+
// to the right to make enough space for merged container is slower
407+
// than appending bitmap with entirely new container and "forgetting" old one
408+
// for large bitmaps, so it is performed only on small ones
409+
if an > 10 {
410+
// create a new container and update the key offset to this container.
411+
offset := a.newContainerNoClr(uint16(len(c)))
412+
copy(a.data[offset:], c)
413+
a.setKey(ak, offset)
414+
} else {
415+
// make room for container, replacing smaller one and update key offset to new container.
416+
a.insertAt(aoff, c)
417+
a.setKey(ak, aoff)
418+
}
422419
}
423420
ai++
424421
bi++
@@ -448,7 +445,7 @@ func orContainersInRange(a, b *Bitmap, bi, bn int, buf []uint16) {
448445

449446
if sizeContainers > 0 {
450447
// ensure enough space for new containers and keys,
451-
// allocate required memory just once avoid copying underlying data slice multiple times
448+
// allocate required memory just once to avoid copying underlying data slice multiple times
452449
a.expandNoLengthChange(sizeContainers + sizeKeys)
453450
a.expandKeys(sizeKeys)
454451

0 commit comments

Comments
 (0)