From ff7900070c1b76c533d15312260c3a44b7d9a32b Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 8 Jun 2022 16:28:35 -0500 Subject: [PATCH] drop the bool from a map that's only using it to check presence We only actually check presence/absence in this map, we never set the value stored to false, and we assume in some places that any value present is equivalent to true, so we might as well use a map of struct{} and save the several whole bytes of memory. --- dbshard.go | 20 ++++++++++---------- dbshard_internal_test.go | 21 ++++++++++----------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/dbshard.go b/dbshard.go index 6c1945fdb..8e7d5e7bf 100644 --- a/dbshard.go +++ b/dbshard.go @@ -153,12 +153,12 @@ func newIndex2Shards() (r map[string]*shardSet) { } type shardSet struct { - shardsMap map[uint64]bool + shardsMap map[uint64]struct{} shardsVer int64 // increment with each change. // give out readonly to repeated consumers if // readonlyVer == shardsVer - readonly map[uint64]bool + readonly map[uint64]struct{} readonlyVer int64 } @@ -203,7 +203,7 @@ func (ss *shardSet) String() (r string) { func (ss *shardSet) add(shard uint64) { _, already := ss.shardsMap[shard] if !already { - ss.shardsMap[shard] = true + ss.shardsMap[shard] = struct{}{} ss.shardsVer++ } } @@ -212,7 +212,7 @@ func (ss *shardSet) add(shard uint64) { // ss.shards that can be returned to multiple goroutine // reads as it will never change. A copy is only made // once for each change in the shard set. -func (ss *shardSet) CloneMaybe() map[uint64]bool { +func (ss *shardSet) CloneMaybe() map[uint64]struct{} { if ss.readonlyVer == ss.shardsVer { return ss.readonly @@ -222,10 +222,10 @@ func (ss *shardSet) CloneMaybe() map[uint64]bool { // readonly needs update. We cannot // modify the readonly map in place; // must make a fully new copy here. - ss.readonly = make(map[uint64]bool) + ss.readonly = make(map[uint64]struct{}) - for k, v := range ss.shardsMap { - ss.readonly[k] = v + for k := range ss.shardsMap { + ss.readonly[k] = struct{}{} } ss.readonlyVer = ss.shardsVer return ss.readonly @@ -233,7 +233,7 @@ func (ss *shardSet) CloneMaybe() map[uint64]bool { func newShardSet() *shardSet { return &shardSet{ - shardsMap: make(map[uint64]bool), + shardsMap: make(map[uint64]struct{}), } } @@ -446,7 +446,7 @@ func (per *DBPerShard) Close() (err error) { // DBPerShardGetShardsForIndex returns the shards for idx. // If requireData, we open the database and see that it has a key, rather // than assume that the database file presence is enough. -func (f *TxFactory) GetShardsForIndex(idx *Index, roaringViewPath string, requireData bool) (map[uint64]bool, error) { +func (f *TxFactory) GetShardsForIndex(idx *Index, roaringViewPath string, requireData bool) (map[uint64]struct{}, error) { return f.dbPerShard.TypedDBPerShardGetShardsForIndex(f.typ, idx, roaringViewPath, requireData) } @@ -457,7 +457,7 @@ func (f *TxFactory) GetShardsForIndex(idx *Index, roaringViewPath string, requir // when a new DBShard is made, we will update the list of shards then. Thus // the per.index2shard should always be up to date AFTER the first call here. // -func (per *DBPerShard) TypedDBPerShardGetShardsForIndex(ty txtype, idx *Index, roaringViewPath string, requireData bool) (shardMap map[uint64]bool, err error) { +func (per *DBPerShard) TypedDBPerShardGetShardsForIndex(ty txtype, idx *Index, roaringViewPath string, requireData bool) (shardMap map[uint64]struct{}, err error) { // use the cache, always per.Mu.Lock() diff --git a/dbshard_internal_test.go b/dbshard_internal_test.go index 38f752425..0e4dcb9fa 100644 --- a/dbshard_internal_test.go +++ b/dbshard_internal_test.go @@ -2,7 +2,6 @@ package pilosa import ( - "fmt" "os" "path/filepath" "strings" @@ -88,8 +87,8 @@ func Test_DBPerShard_GetShardsForIndex_LocalOnly(t *testing.T) { PanicOn(err) for _, shard := range []uint64{93, 223, 221, 215, 219, 217} { - if !shards[shard] { - panic(fmt.Sprintf("missing shard=%v from shards='%#v'", shard, shards)) + if _, ok := shards[shard]; !ok { + t.Fatalf("missing shard=%v from shards='%#v'", shard, shards) } } for _, shard := range []uint64{93, 223, 221, 215, 219, 217} { @@ -100,13 +99,13 @@ func Test_DBPerShard_GetShardsForIndex_LocalOnly(t *testing.T) { expect0 := txkey.FieldView{Field: "_exists", View: "standard"} expect1 := txkey.FieldView{Field: "f", View: "standard"} if len(fvs) != 2 { - panic(fmt.Sprintf("fvs should be len 2, got '%#v' (%s)", fvs, src)) + t.Fatalf("fvs should be len 2, got '%#v' (%s)", fvs, src) } if fvs[0] != expect0 { - panic(fmt.Sprintf("expected fvs[0]='%#v', but got '%#v'", expect0, fvs[0])) + t.Fatalf("expected fvs[0]='%#v', but got '%#v'", expect0, fvs[0]) } if fvs[1] != expect1 { - panic(fmt.Sprintf("expected fvs[1]='%#v', but got '%#v'", expect1, fvs[1])) + t.Fatalf("expected fvs[1]='%#v', but got '%#v'", expect1, fvs[1]) } tx.Rollback() } @@ -170,7 +169,7 @@ func makeSampleRoaringDir(t *testing.T, root, index, backend string, minBytes in // view2shards has them all anyway. if !firstDone { firstDone = true - makeTxTestDBWithViewsShards(h, idx, view2shards) + makeTxTestDBWithViewsShards(t, h, idx, view2shards) } continue case "roaring": @@ -225,7 +224,7 @@ func makeRBFtestDB(path string, h *Holder, shard uint64) { PanicOn(err) } -func makeTxTestDBWithViewsShards(holder *Holder, idx *Index, exp *FieldView2Shards) { +func makeTxTestDBWithViewsShards(tb testing.TB, holder *Holder, idx *Index, exp *FieldView2Shards) { // TODO(jea): need date time quantum views!! for field, viewmap := range exp.m { @@ -240,7 +239,7 @@ func makeTxTestDBWithViewsShards(holder *Holder, idx *Index, exp *FieldView2Shar changeCount, err := tx.Add(idx.name, field, view, shard, bits...) PanicOn(err) if changeCount != len(bits) { - panic(fmt.Sprintf("writing field '%v', view '%v' shard '%v', expected changeCount to equal len bits = %v but was %v", field, view, shard, len(bits), changeCount)) + tb.Fatalf("writing field '%v', view '%v' shard '%v', expected changeCount to equal len bits = %v but was %v", field, view, shard, len(bits), changeCount) } PanicOn(tx.Commit()) @@ -285,7 +284,7 @@ func Test_DBPerShard_GetFieldView2Shards_map_from_RBF(t *testing.T) { hrShardSet.add(7) exp.addViewShardSet(txkey.FieldView{Field: field, View: "standard_2019092416"}, hrShardSet) - makeTxTestDBWithViewsShards(holder, idx, exp) + makeTxTestDBWithViewsShards(t, holder, idx, exp) // setup is done view2shard, err := holder.txf.GetFieldView2ShardsMapForIndex(idx) @@ -293,6 +292,6 @@ func Test_DBPerShard_GetFieldView2Shards_map_from_RBF(t *testing.T) { // compare against setup if !view2shard.equals(exp) { - panic(fmt.Sprintf("expected '%v' but got view2shard '%v'", exp, view2shard)) + t.Fatalf("expected '%v' but got view2shard '%v'", exp, view2shard) } }