Skip to content

Commit

Permalink
op: don't allocate for each string reference
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
dominikh authored and eliasnaur committed Sep 2, 2023
1 parent b9654eb commit b4d9337
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 10 deletions.
29 changes: 29 additions & 0 deletions internal/ops/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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++
}
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion io/clipboard/clipboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
5 changes: 2 additions & 3 deletions io/key/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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))
Expand Down
8 changes: 4 additions & 4 deletions io/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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])
Expand Down
4 changes: 2 additions & 2 deletions io/semantic/semantic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit b4d9337

Please sign in to comment.