From 971d60617b3126d77996e5fa6c39278c203db406 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 15 Jun 2017 11:15:48 -0500 Subject: [PATCH] Update runRemove with run binary search --- roaring/roaring.go | 37 +++++++++++++++----------------- roaring/roaring_internal_test.go | 8 +++---- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 354bda36e..1f5c9be58 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1401,28 +1401,25 @@ func (c *container) bitmapRemove(v uint32) bool { return true } +// runRemove removes v from a run container, and returns true if v was removed. func (c *container) runRemove(v uint32) bool { - // TODO binary search - for i, iv := range c.runs { - if v <= iv.last { - if v < iv.start { - return false - } - c.unmap() - if v == iv.last && v == iv.start { - c.runs = append(c.runs[:i], c.runs[i+1:]...) - } else if v == iv.last { - c.runs[i].last -= 1 - } else if v == iv.start { - c.runs[i].start += 1 - } else if v > iv.start { - c.runs[i].last = v - 1 - c.runs = append(c.runs[:i+1], append([]interval32{{start: v + 1, last: iv.last}}, c.runs[i+1:]...)...) - } - return true - } + i, contains := binSearchRuns(v, c.runs) + if !contains { + return false } - return false + c.unmap() + if v == c.runs[i].last && v == c.runs[i].start { + c.runs = append(c.runs[:i], c.runs[i+1:]...) + } else if v == c.runs[i].last { + c.runs[i].last -= 1 + } else if v == c.runs[i].start { + c.runs[i].start += 1 + } else if v > c.runs[i].start { + last := c.runs[i].last + c.runs[i].last = v - 1 + c.runs = append(c.runs[:i+1], append([]interval32{{start: v+1, last: last}}, c.runs[i+1:]...)...) + } + return true } // max returns the maximum value in the container. diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 29b95f52d..8e083c9d0 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -295,17 +295,17 @@ func TestRunRemove(t *testing.T) { {44, []interval32{{start: 3, last: 5}, {start: 7, last: 7}, {start: 9, last: 9}, {start: 15, last: 15}}, false}, } - for _, test := range tests { + for i, test := range tests { c.mapped = true ret := c.remove(test.op) if ret != test.expRet || !reflect.DeepEqual(c.runs, test.exp) { - t.Fatalf("Unexpected result removing %v from runs. Expected %v, got %v. Expected %v, got %v", test.op, test.expRet, ret, test.exp, c.runs) + t.Fatalf("test #%v Unexpected result removing %v from runs. Expected %v, got %v. Expected %v, got %v", i, test.op, test.expRet, ret, test.exp, c.runs) } if ret && c.mapped { - t.Fatalf("container was not unmapped although bit %v was removed", test.op) + t.Fatalf("test #%v container was not unmapped although bit %v was removed", i, test.op) } if !ret && !c.mapped { - t.Fatalf("container was unmapped although bit %v was not removed", test.op) + t.Fatalf("test #%v container was unmapped although bit %v was not removed", i, test.op) } } }