From ab73819baa8537d3fdca2b9be6cd657b4a631037 Mon Sep 17 00:00:00 2001 From: Tim Bray Date: Fri, 19 Jun 2026 17:34:09 -0700 Subject: [PATCH 1/2] fix: teach Printer about MatcherBuildMode addresses #544 Signed-off-by: Tim Bray --- live_pattern_state.go | 93 +++++++++++++++++++------------------- live_pattern_state_test.go | 2 +- pruner.go | 4 +- pruner_test.go | 5 +- 4 files changed, 54 insertions(+), 50 deletions(-) diff --git a/live_pattern_state.go b/live_pattern_state.go index d132c89c..3ffe84db 100644 --- a/live_pattern_state.go +++ b/live_pattern_state.go @@ -17,76 +17,77 @@ type LivePatternsState interface { 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{} + // SetMatcherBuildMode - See the method of the same name in quamina.go + SetMatcherBuildMode(mode MatcherBuildMode) +} -// 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 + builderMode MatcherBuildMode + entries []memStateEntry } func newMemState() *memState { - // Accept initial size as a parameter? - return &memState{ - m: make(map[X]stringSet), - } + return &memState{builderMode: BuiltForComfort} +} +func (s *memState) SetMatcherBuildMode(mode MatcherBuildMode) { + s.builderMode = mode } - func (s *memState) Add(x X, pattern string) 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, s.builderMode}) 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, s.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 f957fea6..71ee44ad 100644 --- a/live_pattern_state_test.go +++ b/live_pattern_state_test.go @@ -7,7 +7,7 @@ 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 { diff --git a/pruner.go b/pruner.go index fd49df2c..644db256 100644 --- a/pruner.go +++ b/pruner.go @@ -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 28f0a528..4ab1881e 100644 --- a/pruner_test.go +++ b/pruner_test.go @@ -274,6 +274,9 @@ 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 { @@ -288,7 +291,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 } From de47632c40c3c2a50a4179c02d3d350171050f1b Mon Sep 17 00:00:00 2001 From: Tim Bray Date: Sun, 21 Jun 2026 12:39:44 -0700 Subject: [PATCH 2/2] beef up testing Signed-off-by: Tim Bray --- README.md | 3 --- live_pattern_state.go | 21 +++++++------------- live_pattern_state_test.go | 6 +++--- pruner.go | 2 +- pruner_test.go | 39 +++++++++++++++++++++++++++++++++++++- quamina.go | 4 ---- quamina_test.go | 6 +++--- 7 files changed, 52 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index c8db1aa1..84780fa1 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 3ffe84db..dd35a04b 100644 --- a/live_pattern_state.go +++ b/live_pattern_state.go @@ -10,7 +10,7 @@ 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. @@ -21,9 +21,6 @@ type LivePatternsState interface { // Contains returns true if x is in the live set; false otherwise. Contains(x X) (bool, error) - - // SetMatcherBuildMode - See the method of the same name in quamina.go - SetMatcherBuildMode(mode MatcherBuildMode) } // memState is a LivePatternsState that is just a slice of buildMode/pattern pairs @@ -36,21 +33,17 @@ type memStateEntry struct { builderMode MatcherBuildMode } type memState struct { - lock sync.RWMutex - builderMode MatcherBuildMode - entries []memStateEntry + lock sync.RWMutex + entries []memStateEntry } func newMemState() *memState { - return &memState{builderMode: BuiltForComfort} -} -func (s *memState) SetMatcherBuildMode(mode MatcherBuildMode) { - s.builderMode = mode + 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() defer s.lock.Unlock() - s.entries = append(s.entries, memStateEntry{x, pattern, s.builderMode}) + s.entries = append(s.entries, memStateEntry{x, pattern, buildMode}) return nil } func (s *memState) Delete(x X) (int, error) { @@ -84,7 +77,7 @@ func (s *memState) Iterate(f func(x X, pattern string, buildMode MatcherBuildMod defer s.lock.Unlock() var err error for _, entry := range s.entries { - err = f(entry.x, entry.pattern, s.builderMode) + err = f(entry.x, entry.pattern, entry.builderMode) if err != nil { break } diff --git a/live_pattern_state_test.go b/live_pattern_state_test.go index 71ee44ad..0871bcec 100644 --- a/live_pattern_state_test.go +++ b/live_pattern_state_test.go @@ -10,7 +10,7 @@ func TestMemIterateFerr(t *testing.T) { 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 644db256..dd2d2bbd 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. diff --git a/pruner_test.go b/pruner_test.go index 4ab1881e..4390d78c 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"]}` @@ -279,7 +316,7 @@ 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 } diff --git a/quamina.go b/quamina.go index 712ab5ed..74a8a0e7 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 fc007815..2ecb16ef 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") } }