From e471b462b61a33b3d574c6823c1e8f6403b873bc Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Mon, 31 Jan 2022 11:04:20 -0600 Subject: [PATCH] remove all occurences of Bitmap.Source --- fragment.go | 33 --------------- fragment_internal_test.go | 25 +----------- roaring/roaring.go | 53 ------------------------ roaring/source.go | 85 --------------------------------------- 4 files changed, 1 insertion(+), 195 deletions(-) delete mode 100644 roaring/source.go diff --git a/fragment.go b/fragment.go index 41b1f6a79..093f6c763 100644 --- a/fragment.go +++ b/fragment.go @@ -285,39 +285,6 @@ func (f *fragment) Open() error { return nil } -// emptyStorage is the common case for importStorage/applyStorage where they -// get no data. It tries to write the current storage to the provided file, -// which is assumed to be the file they didn't get any data from. -func (f *fragment) emptyStorage(file *os.File) (bool, error) { - if f.holder.Opts.ReadOnly { - return false, errors.New("can't flush/create storage for read-only holder") - } - // No data. We'll mark this for no mapping, clear any existing - // mapped containers, and set the Source to nil. We also have no - // ops. - f.opN = 0 - f.ops = 0 - f.storage.SetOps(0, 0) - - f.storage.PreferMapping(false) - _, err := f.storage.RemapRoaringStorage(nil) - f.storage.SetSource(nil) - if err != nil { - return false, fmt.Errorf("applying/importing storage: no data, and clearing old mapping also failed: %v", err) - } - // Write the existing storage out to the file so it's - // a valid Roaring file thereafter. nothing to unmarshal. - // In the unlikely event that this happened even though we - // had significant data, we're not mapping it, but that's - // harmless even if it's not maximally efficient. - bi := bufio.NewWriter(file) - if _, err = f.storage.WriteTo(bi); err != nil { - return false, fmt.Errorf("init storage file: %s", err) - } - bi.Flush() - return false, nil -} - // openStorage opens the storage bitmap. Does nothing in RBF-world and will be removed soon. func (f *fragment) openStorage(unmarshalData bool) error { if !f.idx.NeedsSnapshot() { diff --git a/fragment_internal_test.go b/fragment_internal_test.go index e079c05ef..f8b1ec526 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -3460,31 +3460,8 @@ func (f *fragment) sanityCheck(t testing.TB) { // Clean used to delete fragments, but doesn't anymore -- deleting is // handled by the testhook.TempDir when appropriate. +// TODO(jaffee): this can likely go away entirely... it was doing snapshot/source/generation stuff that it no longer needs to. func (f *fragment) Clean(t testing.TB) { - f.mu.Lock() - // we need to ensure that we unlock the mutex before terminating - // the clean operation, but we need it held during the sanity - // check or else, in some cases, the background snapshot queue - // can decide to pick it up. - func() { - // should we skip snapshot queue stuff under bolt/rbf? - defer f.mu.Unlock() - - // rbf doesn't need snapshot, so this stuff is skipped. - // The snapshot queue stuff doesn't work under rbf. - if f.idx.NeedsSnapshot() { - err := f.holder.SnapshotQueue.Await(f) - if err != nil { - t.Fatalf("snapshot failed before sanity check: %v", err) - } - f.sanityCheck(t) - if f.storage != nil && f.storage.Source != nil { - if f.storage.Source.Dead() { - t.Fatalf("cleaning up fragment %s, source %s, source already dead", f.path(), f.storage.Source.ID()) - } - } - } - }() errc := f.Close() if errc != nil { t.Fatalf("error closing fragment: %v", errc) diff --git a/roaring/roaring.go b/roaring/roaring.go index cdcf25b89..f632aaca4 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -167,7 +167,6 @@ type ContainerIterator interface { // Bitmap represents a roaring bitmap. type Bitmap struct { Containers Containers - Source Source // User-defined flags. Flags byte @@ -248,7 +247,6 @@ func (b *Bitmap) Freeze() *Bitmap { // Create a copy of the bitmap structure. other := &Bitmap{ Containers: b.Containers.Freeze(), - Source: b.Source, } return other @@ -609,20 +607,13 @@ func (b *Bitmap) OffsetRange(offset, start, end uint64) *Bitmap { hi0, hi1 := highbits(start), highbits(end) citer, _ := b.Containers.Iterator(hi0) other := NewSliceBitmap() - mappedAny := false for citer.Next() { k, c := citer.Value() if k >= hi1 { break } - if c.Mapped() { - mappedAny = true - } other.Containers.Put(off+(k-hi0), c.Freeze()) } - if b.Source != nil && mappedAny { - other.Source = b.Source - } return other } @@ -661,7 +652,6 @@ func (b *Bitmap) IntersectionCount(other *Bitmap) uint64 { // Intersect returns the intersection of b and other. func (b *Bitmap) Intersect(other *Bitmap) *Bitmap { output := NewBitmap() - usedB, usedOther := false, false iiter, _ := b.Containers.Iterator(0) jiter, _ := other.Containers.Iterator(0) i, j := iiter.Next(), jiter.Next() @@ -676,26 +666,12 @@ func (b *Bitmap) Intersect(other *Bitmap) *Bitmap { kj, cj = jiter.Value() } else { // ki == kj newC := intersect(ci, cj) - if newC == ci { - usedB = true - } - if newC == cj { - usedOther = true - } output.Containers.Put(ki, newC) i, j = iiter.Next(), jiter.Next() ki, ci = iiter.Value() kj, cj = jiter.Value() } } - switch { - case usedB && usedOther: - output.Source = MergeSources(b.Source, other.Source) - case usedB: - output.Source = b.Source - case usedOther: - output.Source = other.Source - } return output } @@ -1192,43 +1168,26 @@ func (b *Bitmap) UnionInPlace(others ...*Bitmap) { func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) { iiter, _ := b.Containers.Iterator(0) jiter, _ := other.Containers.Iterator(0) - usedB, usedOther := false, false i, j := iiter.Next(), jiter.Next() ki, ci := iiter.Value() kj, cj := jiter.Value() for i || j { if i && (!j || ki < kj) { target.Containers.Put(ki, ci.Freeze()) - usedB = true i = iiter.Next() ki, ci = iiter.Value() } else if j && (!i || ki > kj) { target.Containers.Put(kj, cj.Freeze()) - usedOther = true j = jiter.Next() kj, cj = jiter.Value() } else { // ki == kj newC := union(ci, cj) target.Containers.Put(ki, newC) - if newC == ci { - usedB = true - } - if newC == cj { - usedOther = true - } i, j = iiter.Next(), jiter.Next() ki, ci = iiter.Value() kj, cj = jiter.Value() } } - switch { - case usedB && usedOther: - target.Source = MergeSources(b.Source, other.Source) - case usedB: - target.Source = b.Source - case usedOther: - target.Source = other.Source - } } // unionInPlace stores the union of b and others into b. The others will @@ -1324,14 +1283,7 @@ func (b *Bitmap) unionInPlace(others ...*Bitmap) { bitmapIters = make(handledIters, 0, requiredSliceSize) } - var sources []Source - if b.Source != nil { - sources = append(sources, b.Source) - } for _, other := range others { - if other.Source != nil { - sources = append(sources, other.Source) - } otherIter, _ := other.Containers.Iterator(0) if otherIter.Next() { bitmapIters = append(bitmapIters, handledIter{ @@ -1341,8 +1293,6 @@ func (b *Bitmap) unionInPlace(others ...*Bitmap) { }) } } - // new bitmap might have containers from any of those bitmaps in it - b.Source = MergeSources(sources...) // Loop until we've exhausted every iter. hasNext := true @@ -1505,9 +1455,6 @@ func (b *Bitmap) singleDifference(other *Bitmap) *Bitmap { // Xor returns the bitwise exclusive or of b and other. func (b *Bitmap) Xor(other *Bitmap) *Bitmap { output := NewBitmap() - // Xor can end up with containers from either parent if the other - // had no container or an empty container. - output.Source = MergeSources(b.Source, other.Source) iiter, _ := b.Containers.Iterator(0) jiter, _ := other.Containers.Iterator(0) diff --git a/roaring/source.go b/roaring/source.go deleted file mode 100644 index e2be583e2..000000000 --- a/roaring/source.go +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright 2021 Molecula Corp. All rights reserved. -package roaring - -import ( - "strings" -) - -// A Source represents the source a given bitmap gets its data from, -// such as a memory-mapped file. When combining bitmaps, we might -// track them together in a single combined-source of some sort. -type Source interface { - ID() string - Dead() bool -} - -// MergeSources combines sources. If you have two bitmaps, and you're -// combining them, then the combination's source is a combination of -// those two sources. -func MergeSources(sources ...Source) Source { - sourceCount := 0 - totalCount := 0 - var lastSource Source - for _, s := range sources { - if s == nil { - continue - } - lastSource = s - if s, ok := s.(combinedSource); ok { - sourceCount++ - totalCount += len(s) - } else { - sourceCount++ - totalCount++ - } - } - // if there's no sources (this includes all sources being - // empty combinedSources), we don't have a source. - if totalCount == 0 { - return nil - } - // if there's exactly one source, combined or otherwise, that's - // fine, we'll just return it. - if sourceCount == 1 { - return lastSource - } - // make a new combinedSource, flattening any combinedSources - // already present. - newSources := make([]Source, 0, totalCount) - for _, s := range sources { - if s == nil { - continue - } - if s, ok := s.(combinedSource); ok { - newSources = append(newSources, s...) - } else { - newSources = append(newSources, s) - } - } - return combinedSource(newSources) -} - -// SetSource tells the bitmap what source to associate with new things it -// creates. This is possibly logically incorrect. -func (b *Bitmap) SetSource(s Source) { - b.Source = s -} - -type combinedSource []Source - -func (c combinedSource) ID() string { - ids := make([]string, len(c)) - for i := range c { - ids[i] = c[i].ID() - } - return strings.Join(ids, ",") -} - -func (c combinedSource) Dead() bool { - for i := range c { - if c[i].Dead() { - return true - } - } - return false -}