diff --git a/README.md b/README.md index c8db1aa..84780fa 100644 --- a/README.md +++ b/README.md @@ -188,9 +188,6 @@ not free; it can incur extra costs in memory and occasional stop-the-world Quamina rebuilds. (We plan to improve this.) -Note that if you choose this option, the `SetMatcherBuildMode` -API will be disabled. This is a bug and will be fixed. - `WithPatternStorage`: If you provide an argument that supports the `LivePatternsState` API, Quamina will use it to maintain a list of which Patterns have currently diff --git a/live_pattern_state.go b/live_pattern_state.go index d132c89..dd35a04 100644 --- a/live_pattern_state.go +++ b/live_pattern_state.go @@ -10,83 +10,77 @@ type LivePatternsState interface { // Add adds a new pattern or updates an old pattern. // // Note that multiple patterns can be associated with the same X. - Add(x X, pattern string) error + Add(x X, pattern string, buildMode MatcherBuildMode) error // Delete removes all patterns associated with the given X and returns the // number of removed patterns. Delete(x X) (int, error) // Iterate calls the given function for every stored pattern. - Iterate(func(x X, pattern string) error) error + Iterate(func(x X, pattern string, buildMode MatcherBuildMode) error) error // Contains returns true if x is in the live set; false otherwise. Contains(x X) (bool, error) } -type ( - stringSet map[string]nothing - nothing struct{} -) - -var na = nothing{} - -// memState is a LivePatternsState that is just a map (with a RWMutex). +// memState is a LivePatternsState that is just a slice of buildMode/pattern pairs // // Since the LivePatternsState implementation can be provided to the // application, we're keeping things simple here initially. +type memStateEntry struct { + x X + pattern string + builderMode MatcherBuildMode +} type memState struct { - lock sync.RWMutex - m map[X]stringSet + lock sync.RWMutex + entries []memStateEntry } func newMemState() *memState { - // Accept initial size as a parameter? - return &memState{ - m: make(map[X]stringSet), - } + return &memState{} } - -func (s *memState) Add(x X, pattern string) error { +func (s *memState) Add(x X, pattern string, buildMode MatcherBuildMode) error { s.lock.Lock() - ps, have := s.m[x] - if !have { - ps = make(stringSet) - s.m[x] = ps - } - ps[pattern] = na - s.lock.Unlock() + defer s.lock.Unlock() + s.entries = append(s.entries, memStateEntry{x, pattern, buildMode}) return nil } - -func (s *memState) Contains(x X) (bool, error) { - s.lock.RLock() - _, have := s.m[x] - s.lock.RUnlock() - return have, nil -} - func (s *memState) Delete(x X) (int, error) { s.lock.Lock() - cardinality := 0 - if xs, have := s.m[x]; have { - cardinality = len(xs) - delete(s.m, x) + defer s.lock.Unlock() + howMany := 0 + var newEntries []memStateEntry + for _, entry := range s.entries { + if entry.x == x { + howMany++ + } else { + newEntries = append(newEntries, entry) + } } - s.lock.Unlock() - - return cardinality, nil + s.entries = newEntries + return howMany, nil } -func (s *memState) Iterate(f func(x X, pattern string) error) error { - s.lock.RLock() +func (s *memState) Contains(x X) (bool, error) { + s.lock.Lock() + defer s.lock.Unlock() + for _, entry := range s.entries { + if entry.x == x { + return true, nil + } + } + return false, nil +} +func (s *memState) Iterate(f func(x X, pattern string, buildMode MatcherBuildMode) error) error { + s.lock.Lock() + defer s.lock.Unlock() var err error - for x, ps := range s.m { - for p := range ps { - if err = f(x, p); err != nil { - break - } + for _, entry := range s.entries { + err = f(entry.x, entry.pattern, entry.builderMode) + if err != nil { + break } } - s.lock.RUnlock() return err } diff --git a/live_pattern_state_test.go b/live_pattern_state_test.go index f957fea..0871bce 100644 --- a/live_pattern_state_test.go +++ b/live_pattern_state_test.go @@ -7,10 +7,10 @@ import ( func TestMemIterateFerr(t *testing.T) { s := newMemState() - f := func(x X, pattern string) error { + f := func(x X, pattern string, buildMode MatcherBuildMode) error { return fmt.Errorf("broken") } - if err := s.Add(1, "{}"); err != nil { + if err := s.Add(1, "{}", BuiltForComfort); err != nil { t.Fatal(err) } if err := s.Iterate(f); err == nil { @@ -21,11 +21,11 @@ func TestMemIterateFerr(t *testing.T) { func TestStateDelete(t *testing.T) { s := newMemState() - if err := s.Add(1, `{"likes":"queso"}`); err != nil { + if err := s.Add(1, `{"likes":"queso"}`, BuiltForComfort); err != nil { t.Fatal(err) } - if err := s.Add(1, `{"likes":"tacos"}`); err != nil { + if err := s.Add(1, `{"likes":"tacos"}`, BuiltForComfort); err != nil { t.Fatal(err) } diff --git a/pruner.go b/pruner.go index fd49df2..dd2d2bb 100644 --- a/pruner.go +++ b/pruner.go @@ -206,7 +206,7 @@ func (m *prunerMatcher) addPattern(x X, pat string, buildMode MatcherBuildMode) m.stats.Live++ _ = m.maybeRebuild(true) m.lock.Unlock() - err = m.live.Add(x, pat) + err = m.live.Add(x, pat, buildMode) // ToDo: Contemplate what do to about an error here // (or if we got an error from addPattern after we did // live.Add. @@ -311,8 +311,8 @@ func (m *prunerMatcher) rebuildWhileLocked(fearlessly bool) error { } count := 0 - err := m.live.Iterate(func(x X, p string) error { - err := m1.addPattern(x, p, BuiltForComfort) + err := m.live.Iterate(func(x X, p string, buildMode MatcherBuildMode) error { + err := m1.addPattern(x, p, buildMode) if err == nil { count++ } diff --git a/pruner_test.go b/pruner_test.go index 28f0a52..4390d78 100644 --- a/pruner_test.go +++ b/pruner_test.go @@ -20,6 +20,43 @@ func (m *prunerMatcher) printStats() { logf("%#v", m.getPrunerStats()) } +func TestPrunerBuildMode(t *testing.T) { + entries := []memStateEntry{ + {1, `{"x": [{"wildcard": "t*ortilla"}]}`, BuiltForComfort}, + {2, `{"x": [{"wildcard": "tortilla*"}]}`, BuiltForSpeed}, + {3, `{"x": [{"wildcard": "*tortilla"}]}`, BuiltForComfort}, + {4, `{"x": [{"wildcard": "tortil*la"}]}`, BuiltForSpeed}, + } + q, err := New(WithPatternDeletion(true)) + if err != nil { + t.Error(err) + } + for _, entry := range entries { + err = q.SetMatcherBuildMode(entry.builderMode) + if err != nil { + t.Error(err) + } + err = q.AddPattern(entry.x, entry.pattern) + if err != nil { + t.Error(err) + } + } + bytes1 := int(q.GetMatcherStats()["bytes"]) + pr, ok := q.matcher.(*prunerMatcher) + if !ok { + t.Error("Not pruner") + } + // trigger := pr.rebuildTrigger + err = pr.rebuildWhileLocked(true) + if err != nil { + t.Error(err) + } + bytes2 := int(q.GetMatcherStats()["bytes"]) + if bytes1 != bytes2 { + t.Errorf("b1 %d, b2 %d", bytes1, bytes2) + } +} + func TestBasic(t *testing.T) { var ( pat = `{"likes":["tacos"]}` @@ -274,9 +311,12 @@ type badState struct { err error } +func (s *badState) SetMatcherBuildMode(mode MatcherBuildMode) { +} + var errBadState = fmt.Errorf("BadState can't do anything right") -func (s *badState) Add(x X, pattern string) error { +func (s *badState) Add(x X, pattern string, buildMode MatcherBuildMode) error { return s.err } @@ -288,7 +328,7 @@ func (s *badState) Delete(x X) (int, error) { return 0, s.err } -func (s *badState) Iterate(f func(x X, pattern string) error) error { +func (s *badState) Iterate(f func(x X, pattern string, buildMode MatcherBuildMode) error) error { return s.err } diff --git a/quamina.go b/quamina.go index 712ab5e..74a8a0e 100644 --- a/quamina.go +++ b/quamina.go @@ -188,10 +188,6 @@ const ( // SetMatcherBuildMode puts a Quamina instance into the provided mode func (q *Quamina) SetMatcherBuildMode(mode MatcherBuildMode) error { - _, ok := q.matcher.(*coreMatcher) - if !ok { - return errors.New("this API not available if WithPatternDeletion enabled") - } q.buildMode = mode return nil } diff --git a/quamina_test.go b/quamina_test.go index fc00781..2ecb16e 100644 --- a/quamina_test.go +++ b/quamina_test.go @@ -20,11 +20,11 @@ func TestCopy(t *testing.T) { func TestSetBuildModeDisabled(t *testing.T) { q, err := New(WithPatternDeletion(true)) if err != nil { - t.Error(err) + t.Error("Can't build for deletion") } err = q.SetMatcherBuildMode(BuiltForSpeed) - if err == nil { - t.Error("SetMatcherBuildMode allowed with deletion enabled") + if err != nil { + t.Error("SetMatcherBuildMode not allowed with deletion enabled") } }