From b4d93379c440f7311448028e31fa4efba8b8ede1 Mon Sep 17 00:00:00 2001 From: Dominik Honnef Date: Sat, 2 Sep 2023 16:44:42 +0200 Subject: [PATCH] op: don't allocate for each string reference When storing a string in an interface value that escapes, Go has to heap allocate space for the string header, as interface values can only store pointers. In text-heavy applications, this can lead to hundreds of allocations per frame due to semantic.LabelOp, the primary user of string-typed references in ops. Instead of allocating each string header individually, provide a slice of strings to store string-typed references in, and store pointers into this slice as the actual references. This only allocates when resizing the slice's backing array, and averages out to no allocations, as the backing array gets reused between calls to Ops.Reset. We introduce two new functions, Write1String and Write2String, which make use of this new slice for their last argument. We could've automated this in the existing Write1 and Write2 methods, but that would require type assertions on each call, and the vast majority of ops do not make use of strings. Signed-off-by: Dominik Honnef --- internal/ops/ops.go | 29 +++++++++++++++++++++++++++++ io/clipboard/clipboard.go | 2 +- io/key/key.go | 5 ++--- io/router/router.go | 8 ++++---- io/semantic/semantic.go | 4 ++-- 5 files changed, 38 insertions(+), 10 deletions(-) diff --git a/internal/ops/ops.go b/internal/ops/ops.go index fff43ad3..328fa184 100644 --- a/internal/ops/ops.go +++ b/internal/ops/ops.go @@ -19,6 +19,17 @@ type Ops struct { data []byte // refs hold external references for operations. refs []interface{} + // stringRefs provides space for string references, pointers to which will + // be stored in refs. Storing a string directly in refs would cause a heap + // allocation, to store the string header in an interface value. The backing + // array of stringRefs, on the other hand, gets reused between calls to + // reset, making string references free on average. + // + // Appending to stringRefs might reallocate the backing array, which will + // leave pointers to the old array in refs. This temporarily causes a slight + // increase in memory usage, but this, too, amortizes away as the capacity + // of stringRefs approaches its stable maximum. + stringRefs []string // nextStateID is the id allocated for the next // StateOp. nextStateID int @@ -183,8 +194,12 @@ func Reset(o *Ops) { for i := range o.refs { o.refs[i] = nil } + for i := range o.stringRefs { + o.stringRefs[i] = "" + } o.data = o.data[:0] o.refs = o.refs[:0] + o.stringRefs = o.stringRefs[:0] o.nextStateID = 0 o.version++ } @@ -265,12 +280,26 @@ func Write1(o *Ops, n int, ref1 interface{}) []byte { return o.data[len(o.data)-n:] } +func Write1String(o *Ops, n int, ref1 string) []byte { + o.data = append(o.data, make([]byte, n)...) + o.stringRefs = append(o.stringRefs, ref1) + o.refs = append(o.refs, &o.stringRefs[len(o.stringRefs)-1]) + return o.data[len(o.data)-n:] +} + func Write2(o *Ops, n int, ref1, ref2 interface{}) []byte { o.data = append(o.data, make([]byte, n)...) o.refs = append(o.refs, ref1, ref2) return o.data[len(o.data)-n:] } +func Write2String(o *Ops, n int, ref1 interface{}, ref2 string) []byte { + o.data = append(o.data, make([]byte, n)...) + o.stringRefs = append(o.stringRefs, ref2) + o.refs = append(o.refs, ref1, &o.stringRefs[len(o.stringRefs)-1]) + return o.data[len(o.data)-n:] +} + func Write3(o *Ops, n int, ref1, ref2, ref3 interface{}) []byte { o.data = append(o.data, make([]byte, n)...) o.refs = append(o.refs, ref1, ref2, ref3) diff --git a/io/clipboard/clipboard.go b/io/clipboard/clipboard.go index ae4a4359..474ae9bb 100644 --- a/io/clipboard/clipboard.go +++ b/io/clipboard/clipboard.go @@ -30,7 +30,7 @@ func (h ReadOp) Add(o *op.Ops) { } func (h WriteOp) Add(o *op.Ops) { - data := ops.Write1(&o.Internal, ops.TypeClipboardWriteLen, &h.Text) + data := ops.Write1String(&o.Internal, ops.TypeClipboardWriteLen, h.Text) data[0] = byte(ops.TypeClipboardWrite) } diff --git a/io/key/key.go b/io/key/key.go index 53cdf926..18d27bc8 100644 --- a/io/key/key.go +++ b/io/key/key.go @@ -323,8 +323,7 @@ func (h InputOp) Add(o *op.Ops) { if h.Tag == nil { panic("Tag must be non-nil") } - filter := h.Keys - data := ops.Write2(&o.Internal, ops.TypeKeyInputLen, h.Tag, &filter) + data := ops.Write2String(&o.Internal, ops.TypeKeyInputLen, h.Tag, string(h.Keys)) data[0] = byte(ops.TypeKeyInput) data[1] = byte(h.Hint) } @@ -343,7 +342,7 @@ func (h FocusOp) Add(o *op.Ops) { } func (s SnippetOp) Add(o *op.Ops) { - data := ops.Write2(&o.Internal, ops.TypeSnippetLen, s.Tag, &s.Text) + data := ops.Write2String(&o.Internal, ops.TypeSnippetLen, s.Tag, s.Text) data[0] = byte(ops.TypeSnippet) bo := binary.LittleEndian bo.PutUint32(data[1:], uint32(s.Range.Start)) diff --git a/io/router/router.go b/io/router/router.go index c7fad915..0c691c7c 100644 --- a/io/router/router.go +++ b/io/router/router.go @@ -490,11 +490,11 @@ func (q *Router) collect() { } kc.softKeyboard(op.Show) case ops.TypeKeyInput: - filter := encOp.Refs[1].(*key.Set) + filter := key.Set(*encOp.Refs[1].(*string)) op := key.InputOp{ Tag: encOp.Refs[0].(event.Tag), Hint: key.InputHint(encOp.Data[1]), - Keys: *filter, + Keys: filter, } a := pc.currentArea() b := pc.currentAreaBounds() @@ -532,10 +532,10 @@ func (q *Router) collect() { // Semantic ops. case ops.TypeSemanticLabel: - lbl := encOp.Refs[0].(string) + lbl := *encOp.Refs[0].(*string) pc.semanticLabel(lbl) case ops.TypeSemanticDesc: - desc := encOp.Refs[0].(string) + desc := *encOp.Refs[0].(*string) pc.semanticDesc(desc) case ops.TypeSemanticClass: class := semantic.ClassOp(encOp.Data[1]) diff --git a/io/semantic/semantic.go b/io/semantic/semantic.go index 86e2b20e..569446a2 100644 --- a/io/semantic/semantic.go +++ b/io/semantic/semantic.go @@ -40,12 +40,12 @@ type SelectedOp bool type DisabledOp bool func (l LabelOp) Add(o *op.Ops) { - data := ops.Write1(&o.Internal, ops.TypeSemanticLabelLen, string(l)) + data := ops.Write1String(&o.Internal, ops.TypeSemanticLabelLen, string(l)) data[0] = byte(ops.TypeSemanticLabel) } func (d DescriptionOp) Add(o *op.Ops) { - data := ops.Write1(&o.Internal, ops.TypeSemanticDescLen, string(d)) + data := ops.Write1String(&o.Internal, ops.TypeSemanticDescLen, string(d)) data[0] = byte(ops.TypeSemanticDesc) }