From 7cf0ea452adb50d61fb198b31a7f5cf0b9b3ee1b Mon Sep 17 00:00:00 2001 From: Seebs Date: Thu, 20 May 2021 16:28:19 -0500 Subject: [PATCH] drop unused TxBitmap TxBitmap was a workaround for performance problems with doing individual-bit operations directly on RBF, used only in the large-writes path of importValue. With importValue no longer using that path, ever, there are zero remaining users of TxBitmap, and the test for it no longer exercises it. Solution: Remove it. --- fragment_internal_test.go | 37 ------------------- tx.go | 77 --------------------------------------- 2 files changed, 114 deletions(-) diff --git a/fragment_internal_test.go b/fragment_internal_test.go index e95351b5a..d7bf8e8dd 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -6091,40 +6091,3 @@ func TestBitmapGrowth(t *testing.T) { deltaSize, deltaOpN) } } - -func TestTxBitmap(t *testing.T) { - f, _, tx := mustOpenFragment(t, "i", "f", viewBSIGroupPrefix+"foo", 0, "") - defer f.Clean(t) - f.MaxOpN = 8 - cols := []uint64{1, 2, 3, 4, 5, 6, 65537, 131073} - vals := []int64{4, 4, 4, 4, 4, 4, 4, 4} - zeros := []int64{0, 0, 0, 0, 0, 0, 0, 0} - err := f.importValue(tx, cols, vals, 7, false) - if err != nil { - t.Fatalf("importing values: %v", err) - } - expected := []uint64{1, (4 * ShardWidth) + 1} - var got []uint64 - _ = tx.ForEach("i", "f", viewBSIGroupPrefix+"foo", 0, func(i uint64) error { - got = append(got, i) - return nil - }) - t.Logf("initial: %d", got) - err = f.importValue(tx, cols[1:], zeros[1:], 7, true) - if err != nil { - t.Fatalf("clearing values: %v", err) - } - got = got[:0] - _ = tx.ForEach("i", "f", viewBSIGroupPrefix+"foo", 0, func(i uint64) error { - got = append(got, i) - return nil - }) - if len(got) != len(expected) { - t.Fatalf("bitmap clear unsuccessful: expected %d, got %d", expected, got) - } - for i := range expected { - if expected[i] != got[i] { - t.Fatalf("bitmap clear unsuccessful: expected %d, got %d", expected, got) - } - } -} diff --git a/tx.go b/tx.go index 9a4806356..d0d083099 100644 --- a/tx.go +++ b/tx.go @@ -247,83 +247,6 @@ func (rr *RawRoaringData) Iterator() (roaring.RoaringIterator, error) { return roaring.NewRoaringIterator(rr.data) } -// TxBitmap represents a bitmap that acts as a cache in front of a transaction. -// Updates to the bitmap first pull in containers as needed and update them -// in memory. The changes can be flushed in bulk using Flush(). -type TxBitmap struct { - b *roaring.Bitmap - tx Tx - index string - field string - view string - shard uint64 - // Container keys we've already snagged containers for, even if - // those containers have since been deleted by remove ops. - seen map[uint64]struct{} -} - -func NewTxBitmap(tx Tx, index, field, view string, shard uint64) *TxBitmap { - return &TxBitmap{ - b: roaring.NewBitmap(), - tx: tx, - index: index, - field: field, - view: view, - shard: shard, - seen: make(map[uint64]struct{}), - } -} - -func (b *TxBitmap) Add(a ...uint64) (changed bool, err error) { - if err := b.ensureContainers(a...); err != nil { - return false, err - } - return b.b.Add(a...) -} - -func (b *TxBitmap) Remove(a ...uint64) (changed bool, err error) { - if err := b.ensureContainers(a...); err != nil { - return false, err - } - return b.b.Remove(a...) -} - -// ensureContainers pulls containers in from the transaction, if needed. -func (b *TxBitmap) ensureContainers(a ...uint64) error { - for _, v := range a { - key := highbits(v) - if _, ok := b.seen[key]; ok { - continue - } - c, err := b.tx.Container(b.index, b.field, b.view, b.shard, key) - if err != nil { - return err - } - b.seen[key] = struct{}{} - b.b.Containers.Put(key, c) - } - return nil -} - -// Flush writes all containers in the bitmap back to the transaction. -func (b *TxBitmap) Flush() error { - for it, _ := b.b.Containers.Iterator(0); it.Next(); { - key, c := it.Value() - if err := b.tx.PutContainer(b.index, b.field, b.view, b.shard, key, c); err != nil { - return err - } - delete(b.seen, key) - } - // remove containers we have seen but no longer have, because that means - // we deleted everything from them. - for key := range b.seen { - if err := b.tx.RemoveContainer(b.index, b.field, b.view, b.shard, key); err != nil { - return err - } - } - return nil -} - // GenericApplyFilter implements ApplyFilter in terms of tx.ContainerIterator, // as a convenience if a Tx backend hasn't implemented this new function yet. func GenericApplyFilter(tx Tx, index, field, view string, shard uint64, ckey uint64, filter roaring.BitmapFilter) (err error) {