From dc586c25206a4b41a7d8c505e3700f5704134228 Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Thu, 5 Jan 2023 19:20:25 -0700 Subject: containers: Add SyncValue and SyncPool types --- lib/textui/progress.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'lib/textui') diff --git a/lib/textui/progress.go b/lib/textui/progress.go index 68d986f..1a5e7d8 100644 --- a/lib/textui/progress.go +++ b/lib/textui/progress.go @@ -7,10 +7,11 @@ package textui import ( "context" "fmt" - "sync/atomic" "time" "github.com/datawire/dlib/dlog" + + "git.lukeshu.com/btrfs-progs-ng/lib/containers" ) type Stats interface { @@ -26,7 +27,7 @@ type Progress[T Stats] struct { cancel context.CancelFunc done chan struct{} - cur atomic.Value // Value[T] + cur containers.SyncValue[T] oldStat T oldLine string } @@ -45,7 +46,7 @@ func NewProgress[T Stats](ctx context.Context, lvl dlog.LogLevel, interval time. } func (p *Progress[T]) Set(val T) { - if p.cur.Swap(val) == nil { + if _, hadOld := p.cur.Swap(val); !hadOld { go p.run() } } @@ -56,8 +57,10 @@ func (p *Progress[T]) Done() { } func (p *Progress[T]) flush(force bool) { - //nolint:forcetypeassert // It wasn't worth it to me (yet?) to make a typed wrapper around atomic.Value. - cur := p.cur.Load().(T) + cur, ok := p.cur.Load() + if !ok { + panic("should not happen") + } if !force && cur == p.oldStat { return } -- cgit v1.2.3-54-g00ecf From 21fc68264ae07da11ed90d1f44b9a24225c799dc Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Thu, 5 Jan 2023 19:40:32 -0700 Subject: textui: Go ahead and implement that logBufPool --- lib/textui/log.go | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) (limited to 'lib/textui') diff --git a/lib/textui/log.go b/lib/textui/log.go index f73b271..41c874f 100644 --- a/lib/textui/log.go +++ b/lib/textui/log.go @@ -26,6 +26,8 @@ import ( "github.com/datawire/dlib/dlog" "github.com/spf13/pflag" + + "git.lukeshu.com/btrfs-progs-ng/lib/containers" ) type LogLevelFlag struct { @@ -153,7 +155,11 @@ func (l *logger) UnformattedLogf(lvl dlog.LogLevel, format string, args ...any) } var ( - logBuf bytes.Buffer + logBufPool = containers.SyncPool[*bytes.Buffer]{ + New: func() *bytes.Buffer { + return new(bytes.Buffer) + }, + } logMu sync.Mutex thisModDir string ) @@ -169,13 +175,8 @@ func (l *logger) log(lvl dlog.LogLevel, writeMsg func(io.Writer)) { if lvl > l.lvl { return } - // This is optimized for mostly-single-threaded usage. If I cared more - // about multi-threaded performance, I'd trade in some - // memory-use/allocations and (1) instead of using a static `logBuf`, - // I'd have a `logBufPool` `sync.Pool`, and (2) have the final call to - // `l.out.Write()` be the only thing protected by `logMu`. - logMu.Lock() - defer logMu.Unlock() + logBuf, _ := logBufPool.Get() + defer logBufPool.Put(logBuf) defer logBuf.Reset() // time //////////////////////////////////////////////////////////////// @@ -222,19 +223,19 @@ func (l *logger) log(lvl dlog.LogLevel, writeMsg func(io.Writer)) { nextField = i break } - writeField(&logBuf, fieldKey, fields[fieldKey]) + writeField(logBuf, fieldKey, fields[fieldKey]) } // message ///////////////////////////////////////////////////////////// logBuf.WriteString(" : ") - writeMsg(&logBuf) + writeMsg(logBuf) // fields (late) /////////////////////////////////////////////////////// if nextField < len(fieldKeys) { logBuf.WriteString(" :") } for _, fieldKey := range fieldKeys[nextField:] { - writeField(&logBuf, fieldKey, fields[fieldKey]) + writeField(logBuf, fieldKey, fields[fieldKey]) } // caller ////////////////////////////////////////////////////////////// @@ -258,13 +259,16 @@ func (l *logger) log(lvl dlog.LogLevel, writeMsg func(io.Writer)) { logBuf.WriteString(" :") } file := f.File[strings.LastIndex(f.File, thisModDir+"/")+len(thisModDir+"/"):] - fmt.Fprintf(&logBuf, " (from %s:%d)", file, f.Line) + fmt.Fprintf(logBuf, " (from %s:%d)", file, f.Line) break } // boilerplate ///////////////////////////////////////////////////////// logBuf.WriteByte('\n') + + logMu.Lock() _, _ = l.out.Write(logBuf.Bytes()) + logMu.Unlock() } // fieldOrd returns the sort-position for a given log-field-key. Lower return -- cgit v1.2.3-54-g00ecf From 9c5d61133af93ddf279e355a06d4c1ae78c568b3 Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Thu, 5 Jan 2023 19:41:12 -0700 Subject: textui: Avoid allocating strings when logging --- lib/textui/log.go | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) (limited to 'lib/textui') diff --git a/lib/textui/log.go b/lib/textui/log.go index 41c874f..605755d 100644 --- a/lib/textui/log.go +++ b/lib/textui/log.go @@ -18,7 +18,6 @@ import ( "path/filepath" "runtime" "sort" - "strconv" "strings" "sync" "time" @@ -348,35 +347,49 @@ func fieldOrd(key string) int { } func writeField(w io.Writer, key string, val any) { - valStr := printer.Sprint(val) + valBuf, _ := logBufPool.Get() + defer func() { + // The wrapper `func()` is important to defer + // evaluating `valBuf`, since we might re-assign it + // below. + valBuf.Reset() + logBufPool.Put(valBuf) + }() + _, _ = printer.Fprint(valBuf, val) needsQuote := false - if strings.HasPrefix(valStr, `"`) { + if bytes.HasPrefix(valBuf.Bytes(), []byte(`"`)) { needsQuote = true } else { - for _, r := range valStr { - if !(unicode.IsPrint(r) && r != ' ') { + for _, r := range valBuf.Bytes() { + if !(unicode.IsPrint(rune(r)) && r != ' ') { needsQuote = true break } } } if needsQuote { - valStr = strconv.Quote(valStr) + valBuf2, _ := logBufPool.Get() + fmt.Fprintf(valBuf2, "%q", valBuf.Bytes()) + valBuf.Reset() + logBufPool.Put(valBuf) + valBuf = valBuf2 } + valStr := valBuf.Bytes() name := key switch { case name == "THREAD": name = "thread" - switch valStr { - case "", "/main": + switch { + case len(valStr) == 0 || bytes.Equal(valStr, []byte("/main")): return default: - if strings.HasPrefix(valStr, "/main/") { - valStr = strings.TrimPrefix(valStr, "/main") + if bytes.HasPrefix(valStr, []byte("/main/")) { + valStr = valStr[len("/main/"):] + } else if bytes.HasPrefix(valStr, []byte("/")) { + valStr = valStr[len("/"):] } - valStr = strings.TrimPrefix(valStr, "/") } case strings.HasSuffix(name, ".pass"): fmt.Fprintf(w, "/pass-%s", valStr) -- cgit v1.2.3-54-g00ecf From 86063b41fc1a235930d6c79e6b7cd38ae8d8c147 Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Thu, 26 Jan 2023 14:53:37 -0700 Subject: Split lib/containers.Sync* to git.lukeshu.com/go/typedsync --- go.mod | 1 + go.sum | 2 + lib/btrfsprogs/btrfsinspect/mount.go | 5 +- .../btrfsinspect/rebuildnodes/btrees/forrest.go | 3 +- lib/containers/syncmap.go | 53 ----------------- lib/containers/syncpool.go | 33 ----------- lib/containers/syncvalue.go | 67 ---------------------- lib/textui/log.go | 5 +- lib/textui/progress.go | 5 +- 9 files changed, 12 insertions(+), 162 deletions(-) delete mode 100644 lib/containers/syncmap.go delete mode 100644 lib/containers/syncpool.go delete mode 100644 lib/containers/syncvalue.go (limited to 'lib/textui') diff --git a/go.mod b/go.mod index bd3f702..d14ff26 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ go 1.19 require ( git.lukeshu.com/go/lowmemjson v0.2.0 + git.lukeshu.com/go/typedsync v0.0.0-20230126205501-1e8afc0ceb1e github.com/datawire/dlib v1.3.0 github.com/datawire/ocibuild v0.0.3-0.20220423003204-fc6a4e9f90dc github.com/davecgh/go-spew v1.1.1 diff --git a/go.sum b/go.sum index c6e06c5..ee0b036 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ git.lukeshu.com/go/lowmemjson v0.2.0 h1:ybTArT2jmTJ1QVFGycnGX20zFDKBaqQp4S+dI3vOkTI= git.lukeshu.com/go/lowmemjson v0.2.0/go.mod h1:7StdaFpmZNKYJmQ67fGbzcIcnrGjmD54f/2WbeHLaBw= +git.lukeshu.com/go/typedsync v0.0.0-20230126205501-1e8afc0ceb1e h1:ZAzzElMx7aMgJXC9QXOxIPyoZrWxX00eP2sR4UHYP+4= +git.lukeshu.com/go/typedsync v0.0.0-20230126205501-1e8afc0ceb1e/go.mod h1:EAn7NcfoGeGMv3DWxKQnifcT/rYPAIEqp9Rsz//oYqY= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/datawire/dlib v1.3.0 h1:KkmyXU1kwm3oPBk1ypR70YbcOlEXWzEbx5RE0iRXTGk= github.com/datawire/dlib v1.3.0/go.mod h1:NiGDmetmbkBvtznpWSx6C0vA0s0LK9aHna3LJDqjruk= diff --git a/lib/btrfsprogs/btrfsinspect/mount.go b/lib/btrfsprogs/btrfsinspect/mount.go index ee3c0ec..0ac8497 100644 --- a/lib/btrfsprogs/btrfsinspect/mount.go +++ b/lib/btrfsprogs/btrfsinspect/mount.go @@ -14,6 +14,7 @@ import ( "sync/atomic" "syscall" + "git.lukeshu.com/go/typedsync" "github.com/datawire/dlib/dcontext" "github.com/datawire/dlib/dgroup" "github.com/datawire/dlib/dlog" @@ -109,8 +110,8 @@ type subvolume struct { fuseutil.NotImplementedFileSystem lastHandle uint64 - dirHandles containers.SyncMap[fuseops.HandleID, *dirState] - fileHandles containers.SyncMap[fuseops.HandleID, *fileState] + dirHandles typedsync.Map[fuseops.HandleID, *dirState] + fileHandles typedsync.Map[fuseops.HandleID, *fileState] subvolMu sync.Mutex subvols containers.Set[string] diff --git a/lib/btrfsprogs/btrfsinspect/rebuildnodes/btrees/forrest.go b/lib/btrfsprogs/btrfsinspect/rebuildnodes/btrees/forrest.go index ff6b1c5..45a5bb2 100644 --- a/lib/btrfsprogs/btrfsinspect/rebuildnodes/btrees/forrest.go +++ b/lib/btrfsprogs/btrfsinspect/rebuildnodes/btrees/forrest.go @@ -7,6 +7,7 @@ package btrees import ( "context" + "git.lukeshu.com/go/typedsync" "github.com/datawire/dlib/dlog" "git.lukeshu.com/btrfs-progs-ng/lib/btrfs/btrfsitem" @@ -62,7 +63,7 @@ type RebuiltForrest struct { cbLookupUUID func(ctx context.Context, uuid btrfsprim.UUID) (id btrfsprim.ObjID, ok bool) // mutable - trees containers.SyncMap[btrfsprim.ObjID, *RebuiltTree] + trees typedsync.Map[btrfsprim.ObjID, *RebuiltTree] leafs *containers.LRUCache[btrfsprim.ObjID, map[btrfsvol.LogicalAddr]containers.Set[btrfsvol.LogicalAddr]] incItems *containers.LRUCache[btrfsprim.ObjID, *itemIndex] excItems *containers.LRUCache[btrfsprim.ObjID, *itemIndex] diff --git a/lib/containers/syncmap.go b/lib/containers/syncmap.go deleted file mode 100644 index 74da4b3..0000000 --- a/lib/containers/syncmap.go +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (C) 2022-2023 Luke Shumaker -// -// SPDX-License-Identifier: GPL-2.0-or-later - -package containers - -import ( - "sync" -) - -type SyncMap[K comparable, V any] struct { - inner sync.Map -} - -func (m *SyncMap[K, V]) Delete(key K) { - m.inner.Delete(key) -} - -func (m *SyncMap[K, V]) Load(key K) (value V, ok bool) { - _value, ok := m.inner.Load(key) - if ok { - //nolint:forcetypeassert // Typed wrapper around untyped lib. - value = _value.(V) - } - return value, ok -} - -func (m *SyncMap[K, V]) LoadAndDelete(key K) (value V, loaded bool) { - _value, ok := m.inner.LoadAndDelete(key) - if ok { - //nolint:forcetypeassert // Typed wrapper around untyped lib. - value = _value.(V) - } - return value, ok -} - -func (m *SyncMap[K, V]) LoadOrStore(key K, value V) (actual V, loaded bool) { - _actual, loaded := m.inner.LoadOrStore(key, value) - //nolint:forcetypeassert // Typed wrapper around untyped lib. - actual = _actual.(V) - return actual, loaded -} - -func (m *SyncMap[K, V]) Range(f func(key K, value V) bool) { - m.inner.Range(func(key, value any) bool { - //nolint:forcetypeassert // Typed wrapper around untyped lib. - return f(key.(K), value.(V)) - }) -} - -func (m *SyncMap[K, V]) Store(key K, value V) { - m.inner.Store(key, value) -} diff --git a/lib/containers/syncpool.go b/lib/containers/syncpool.go deleted file mode 100644 index cb5398d..0000000 --- a/lib/containers/syncpool.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (C) 2022-2023 Luke Shumaker -// -// SPDX-License-Identifier: GPL-2.0-or-later - -package containers - -import ( - "sync" -) - -type SyncPool[T any] struct { - New func() T - - inner sync.Pool -} - -func (p *SyncPool[T]) Get() (val T, ok bool) { - _val := p.inner.Get() - switch { - case _val != nil: - //nolint:forcetypeassert // Typed wrapper around untyped lib. - return _val.(T), true - case p.New != nil: - return p.New(), true - default: - var zero T - return zero, false - } -} - -func (p *SyncPool[T]) Put(val T) { - p.inner.Put(val) -} diff --git a/lib/containers/syncvalue.go b/lib/containers/syncvalue.go deleted file mode 100644 index 160db3c..0000000 --- a/lib/containers/syncvalue.go +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright (C) 2022-2023 Luke Shumaker -// -// SPDX-License-Identifier: GPL-2.0-or-later - -package containers - -import ( - "sync" -) - -// SyncValue is a typed equivalent of sync/atomic.Value. -// -// It is not actually a wrapper around sync/atomic.Value for -// allocation-performance reasons. -type SyncValue[T comparable] struct { - mu sync.Mutex - ok bool - val T -} - -// This uses a dumb mutex-based solution because -// -// 1. Performance is good enough, because in the fast-path mutexes -// use the same compare-and-swap as sync/atomic.Value; and because -// all of these methods are short we're unlikely to hit the -// mutex's slow path. -// -// 2. We could use sync/atomic.Pointer[T], which by itself would have -// the same performance characteristics as sync/atomic.Value but -// without the benefit of runtime_procPin()/runtime_procUnpin(). -// We want to avoid that because it means we're doing an -// allocation for every store/swap; avoiding that is our whole -// reason for not just wraping sync/atomic.Value. So then we'd -// want to use a SyncPool to reuse allocations; but (1) that adds -// more sync-overhead, and (2) it also gets trickier because we'd -// have to be careful about not adding a pointer back to the pool -// when load has grabbed the pointer but not yet dereferenced it. - -func (v *SyncValue[T]) Load() (val T, ok bool) { - v.mu.Lock() - defer v.mu.Unlock() - return v.val, v.ok -} - -func (v *SyncValue[T]) Store(val T) { - v.mu.Lock() - defer v.mu.Unlock() - v.val, v.ok = val, true -} - -func (v *SyncValue[T]) Swap(newV T) (oldV T, oldOK bool) { - v.mu.Lock() - defer v.mu.Unlock() - oldV, oldOK = v.val, v.ok - v.val, v.ok = newV, true - return -} - -func (v *SyncValue[T]) CompareAndSwap(oldV, newV T) (swapped bool) { - v.mu.Lock() - defer v.mu.Unlock() - if !v.ok || v.val != oldV { - return false - } - v.val = newV - return true -} diff --git a/lib/textui/log.go b/lib/textui/log.go index 605755d..2bcd9af 100644 --- a/lib/textui/log.go +++ b/lib/textui/log.go @@ -23,10 +23,9 @@ import ( "time" "unicode" + "git.lukeshu.com/go/typedsync" "github.com/datawire/dlib/dlog" "github.com/spf13/pflag" - - "git.lukeshu.com/btrfs-progs-ng/lib/containers" ) type LogLevelFlag struct { @@ -154,7 +153,7 @@ func (l *logger) UnformattedLogf(lvl dlog.LogLevel, format string, args ...any) } var ( - logBufPool = containers.SyncPool[*bytes.Buffer]{ + logBufPool = typedsync.Pool[*bytes.Buffer]{ New: func() *bytes.Buffer { return new(bytes.Buffer) }, diff --git a/lib/textui/progress.go b/lib/textui/progress.go index 1a5e7d8..48a3901 100644 --- a/lib/textui/progress.go +++ b/lib/textui/progress.go @@ -9,9 +9,8 @@ import ( "fmt" "time" + "git.lukeshu.com/go/typedsync" "github.com/datawire/dlib/dlog" - - "git.lukeshu.com/btrfs-progs-ng/lib/containers" ) type Stats interface { @@ -27,7 +26,7 @@ type Progress[T Stats] struct { cancel context.CancelFunc done chan struct{} - cur containers.SyncValue[T] + cur typedsync.Value[T] oldStat T oldLine string } -- cgit v1.2.3-54-g00ecf