Skip to content

Commit

Permalink
lib/bytesutil: make sure that the cleanup code is performed only by a…
Browse files Browse the repository at this point in the history
… single goroutine out of many concurrently running goroutines

Updates #3466
  • Loading branch information
valyala committed Dec 21, 2022
1 parent 0f8ffc7 commit 3300546
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 12 deletions.
17 changes: 13 additions & 4 deletions lib/bytesutil/fast_string_matcher.go
Expand Up @@ -62,10 +62,8 @@ func (fsm *FastStringMatcher) Match(s string) bool {
s = strings.Clone(s)
fsm.m.Store(s, e)

if atomic.LoadUint64(&fsm.lastCleanupTime)+61 < ct {
// Perform a global cleanup for fsm.m by removing items, which weren't accessed
// during the last 5 minutes.
atomic.StoreUint64(&fsm.lastCleanupTime, ct)
if needCleanup(&fsm.lastCleanupTime, ct) {
// Perform a global cleanup for fsm.m by removing items, which weren't accessed during the last 5 minutes.
m := &fsm.m
m.Range(func(k, v interface{}) bool {
e := v.(*fsmEntry)
Expand All @@ -78,3 +76,14 @@ func (fsm *FastStringMatcher) Match(s string) bool {

return b
}

func needCleanup(lastCleanupTime *uint64, currentTime uint64) bool {
lct := atomic.LoadUint64(lastCleanupTime)
if lct+61 >= currentTime {
return false
}
// Atomically compare and swap the current time with the lastCleanupTime
// in order to guarantee that only a single goroutine out of multiple
// concurrently executing goroutines gets true from the call.
return atomic.CompareAndSwapUint64(lastCleanupTime, lct, currentTime)
}
24 changes: 24 additions & 0 deletions lib/bytesutil/fast_string_matcher_test.go
Expand Up @@ -23,3 +23,27 @@ func TestFastStringMatcher(t *testing.T) {
f("a_b-C", false)
f("foobar", true)
}

func TestNeedCleanup(t *testing.T) {
f := func(lastCleanupTime, currentTime uint64, resultExpected bool) {
t.Helper()
lct := lastCleanupTime
result := needCleanup(&lct, currentTime)
if result != resultExpected {
t.Fatalf("unexpected result for needCleanup(%d, %d); got %v; want %v", lastCleanupTime, currentTime, result, resultExpected)
}
if result {
if lct != currentTime {
t.Fatalf("unexpected value for lct; got %d; want currentTime=%d", lct, currentTime)
}
} else {
if lct != lastCleanupTime {
t.Fatalf("unexpected value for lct; got %d; want lastCleanupTime=%d", lct, lastCleanupTime)
}
}
}
f(0, 0, false)
f(0, 61, false)
f(0, 62, true)
f(10, 100, true)
}
6 changes: 2 additions & 4 deletions lib/bytesutil/fast_string_transformer.go
Expand Up @@ -67,10 +67,8 @@ func (fst *FastStringTransformer) Transform(s string) string {
}
fst.m.Store(s, e)

if atomic.LoadUint64(&fst.lastCleanupTime)+61 < ct {
// Perform a global cleanup for fst.m by removing items, which weren't accessed
// during the last 5 minutes.
atomic.StoreUint64(&fst.lastCleanupTime, ct)
if needCleanup(&fst.lastCleanupTime, ct) {
// Perform a global cleanup for fst.m by removing items, which weren't accessed during the last 5 minutes.
m := &fst.m
m.Range(func(k, v interface{}) bool {
e := v.(*fstEntry)
Expand Down
6 changes: 2 additions & 4 deletions lib/bytesutil/internstring.go
Expand Up @@ -30,10 +30,8 @@ func InternString(s string) string {
}
internStringsMap.Store(sCopy, e)

if atomic.LoadUint64(&internStringsMapLastCleanupTime)+61 < ct {
// Perform a global cleanup for internStringsMap by removing items, which weren't accessed
// during the last 5 minutes.
atomic.StoreUint64(&internStringsMapLastCleanupTime, ct)
if needCleanup(&internStringsMapLastCleanupTime, ct) {
// Perform a global cleanup for internStringsMap by removing items, which weren't accessed during the last 5 minutes.
m := &internStringsMap
m.Range(func(k, v interface{}) bool {
e := v.(*ismEntry)
Expand Down

0 comments on commit 3300546

Please sign in to comment.