perf: use fmt.Fprintf to avoid unnecessary string+args + WriteString (#…

…3434)

In a bid to remove unnecessary CPU and memory bloat for the gnovm which
takes the order of minutes to run certain code, I noticed the pattern:

    io.StringWriter.WriteString(fmt.Sprintf(...))

in which fmt.Sprintf(...) has to create a string by inserting all
arguments into the format specifiers then pass that into .WriteString
which defeats the entire purpose of io.StringWriter.WriteString that
*bytes.Buffer and *strings.Builder implement.

Just from picking a single benchmark that already exists results in
improvements in all dimensions:

```shell
name               old time/op    new time/op    delta
StringLargeData-8    8.87ms ± 1%    8.28ms ± 3%   -6.68%  (p=0.000 n=17+19)

name               old alloc/op   new alloc/op   delta
StringLargeData-8    8.44MB ± 0%    7.78MB ± 0%   -7.75%  (p=0.000 n=20+19)

name               old allocs/op  new allocs/op  delta
StringLargeData-8     94.1k ± 0%     70.1k ± 0%  -25.51%  (p=0.000 n=15+20)
```

for heavily used code this is going to reduce on garbage collection
cycles too.

Fixes #3433

---------

Co-authored-by: Nathan Toups <[email protected]>
Co-authored-by: Morgan <[email protected]>

contribs/gnomigrate/internal/txs/txs.go

@@ -184,7 +184,7 @@ func processFile(ctx context.Context, io commands.IO, source, destination string
 				continue
 			}
 
-			if _, err = outputFile.WriteString(fmt.Sprintf("%s\n", string(marshaledData))); err != nil {
+			if _, err = fmt.Fprintf(outputFile, "%s\n", marshaledData); err != nil {
 				io.ErrPrintfln("unable to save to output file, %s", err)
 			}
 		}

gnovm/pkg/gnolang/gno_test.go

@@ -36,6 +36,8 @@ func setupMachine(b *testing.B, numValues, numStmts, numExprs, numBlocks, numFra
 
 func BenchmarkStringLargeData(b *testing.B) {
 	m := setupMachine(b, 10000, 5000, 5000, 2000, 3000, 1000)
+	b.ResetTimer()
+	b.ReportAllocs()
 
 	for i := 0; i < b.N; i++ {
 		_ = m.String()

gnovm/pkg/gnolang/machine.go

@@ -2117,6 +2117,10 @@ func (m *Machine) Printf(format string, args ...interface{}) {
 }
 
 func (m *Machine) String() string {
+	if m == nil {
+		return "Machine:nil"
+	}
+
 	// Calculate some reasonable total length to avoid reallocation
 	// Assuming an average length of 32 characters per string
 	var (
@@ -2131,25 +2135,26 @@ func (m *Machine) String() string {
 		totalLength = vsLength + ssLength + xsLength + bsLength + obsLength + fsLength + exceptionsLength
 	)
 
-	var builder strings.Builder
+	var sb strings.Builder
+	builder := &sb // Pointer for use in fmt.Fprintf.
 	builder.Grow(totalLength)
 
-	builder.WriteString(fmt.Sprintf("Machine:\n    PreprocessorMode: %v\n    Op: %v\n    Values: (len: %d)\n", m.PreprocessorMode, m.Ops[:m.NumOps], m.NumValues))
+	fmt.Fprintf(builder, "Machine:\n    PreprocessorMode: %v\n    Op: %v\n    Values: (len: %d)\n", m.PreprocessorMode, m.Ops[:m.NumOps], m.NumValues)
 
 	for i := m.NumValues - 1; i >= 0; i-- {
-		builder.WriteString(fmt.Sprintf("          #%d %v\n", i, m.Values[i]))
+		fmt.Fprintf(builder, "          #%d %v\n", i, m.Values[i])
 	}
 
 	builder.WriteString("    Exprs:\n")
 
 	for i := len(m.Exprs) - 1; i >= 0; i-- {
-		builder.WriteString(fmt.Sprintf("          #%d %v\n", i, m.Exprs[i]))
+		fmt.Fprintf(builder, "          #%d %v\n", i, m.Exprs[i])
 	}
 
 	builder.WriteString("    Stmts:\n")
 
 	for i := len(m.Stmts) - 1; i >= 0; i-- {
-		builder.WriteString(fmt.Sprintf("          #%d %v\n", i, m.Stmts[i]))
+		fmt.Fprintf(builder, "          #%d %v\n", i, m.Stmts[i])
 	}
 
 	builder.WriteString("    Blocks:\n")
@@ -2166,17 +2171,17 @@ func (m *Machine) String() string {
 		if pv, ok := b.Source.(*PackageNode); ok {
 			// package blocks have too much, so just
 			// print the pkgpath.
-			builder.WriteString(fmt.Sprintf("          %s(%d) %s\n", gens, gen, pv.PkgPath))
+			fmt.Fprintf(builder, "          %s(%d) %s\n", gens, gen, pv.PkgPath)
 		} else {
 			bsi := b.StringIndented("            ")
-			builder.WriteString(fmt.Sprintf("          %s(%d) %s\n", gens, gen, bsi))
+			fmt.Fprintf(builder, "          %s(%d) %s\n", gens, gen, bsi)
 
 			if b.Source != nil {
 				sb := b.GetSource(m.Store).GetStaticBlock().GetBlock()
-				builder.WriteString(fmt.Sprintf(" (s vals) %s(%d) %s\n", gens, gen, sb.StringIndented("            ")))
+				fmt.Fprintf(builder, " (s vals) %s(%d) %s\n", gens, gen, sb.StringIndented("            "))
 
 				sts := b.GetSource(m.Store).GetStaticBlock().Types
-				builder.WriteString(fmt.Sprintf(" (s typs) %s(%d) %s\n", gens, gen, sts))
+				fmt.Fprintf(builder, " (s typs) %s(%d) %s\n", gens, gen, sts)
 			}
 		}
 
@@ -2187,7 +2192,7 @@ func (m *Machine) String() string {
 		case *Block:
 			b = bp
 		case RefValue:
-			builder.WriteString(fmt.Sprintf("            (block ref %v)\n", bp.ObjectID))
+			fmt.Fprintf(builder, "            (block ref %v)\n", bp.ObjectID)
 			b = nil
 		default:
 			panic("should not happen")
@@ -2206,30 +2211,30 @@ func (m *Machine) String() string {
 		if _, ok := b.Source.(*PackageNode); ok {
 			break // done, skip *PackageNode.
 		} else {
-			builder.WriteString(fmt.Sprintf("          #%d %s\n", i,
-				b.StringIndented("            ")))
+			fmt.Fprintf(builder, "          #%d %s\n", i,
+				b.StringIndented("            "))
 			if b.Source != nil {
 				sb := b.GetSource(m.Store).GetStaticBlock().GetBlock()
-				builder.WriteString(fmt.Sprintf(" (static) #%d %s\n", i,
-					sb.StringIndented("            ")))
+				fmt.Fprintf(builder, " (static) #%d %s\n", i,
+					sb.StringIndented("            "))
 			}
 		}
 	}
 
 	builder.WriteString("    Frames:\n")
 
 	for i := len(m.Frames) - 1; i >= 0; i-- {
-		builder.WriteString(fmt.Sprintf("          #%d %s\n", i, m.Frames[i]))
+		fmt.Fprintf(builder, "          #%d %s\n", i, m.Frames[i])
 	}
 
 	if m.Realm != nil {
-		builder.WriteString(fmt.Sprintf("    Realm:\n      %s\n", m.Realm.Path))
+		fmt.Fprintf(builder, "    Realm:\n      %s\n", m.Realm.Path)
 	}
 
 	builder.WriteString("    Exceptions:\n")
 
 	for _, ex := range m.Exceptions {
-		builder.WriteString(fmt.Sprintf("      %s\n", ex.Sprint(m)))
+		fmt.Fprintf(builder, "      %s\n", ex.Sprint(m))
 	}
 
 	return builder.String()

gnovm/pkg/gnolang/machine_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/gnolang/gno/tm2/pkg/store/dbadapter"
 	"github.com/gnolang/gno/tm2/pkg/store/iavl"
 	stypes "github.com/gnolang/gno/tm2/pkg/store/types"
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -56,3 +57,61 @@ func TestRunMemPackageWithOverrides_revertToOld(t *testing.T) {
 	assert.Equal(t, StringKind, v.T.Kind())
 	assert.Equal(t, StringValue("1"), v.V)
 }
+
+func TestMachineString(t *testing.T) {
+	cases := []struct {
+		name string
+		in   *Machine
+		want string
+	}{
+		{
+			"nil Machine",
+			nil,
+			"Machine:nil",
+		},
+		{
+			"created with defaults",
+			NewMachineWithOptions(MachineOptions{}),
+			"Machine:\n    PreprocessorMode: false\n    Op: []\n    Values: (len: 0)\n    Exprs:\n    Stmts:\n    Blocks:\n    Blocks (other):\n    Frames:\n    Exceptions:\n",
+		},
+		{
+			"created with store and defaults",
+			func() *Machine {
+				db := memdb.NewMemDB()
+				baseStore := dbadapter.StoreConstructor(db, stypes.StoreOptions{})
+				iavlStore := iavl.StoreConstructor(db, stypes.StoreOptions{})
+				store := NewStore(nil, baseStore, iavlStore)
+				return NewMachine("std", store)
+			}(),
+			"Machine:\n    PreprocessorMode: false\n    Op: []\n    Values: (len: 0)\n    Exprs:\n    Stmts:\n    Blocks:\n    Blocks (other):\n    Frames:\n    Exceptions:\n",
+		},
+		{
+			"filled in",
+			func() *Machine {
+				db := memdb.NewMemDB()
+				baseStore := dbadapter.StoreConstructor(db, stypes.StoreOptions{})
+				iavlStore := iavl.StoreConstructor(db, stypes.StoreOptions{})
+				store := NewStore(nil, baseStore, iavlStore)
+				m := NewMachine("std", store)
+				m.PushOp(OpHalt)
+				m.PushExpr(&BasicLitExpr{
+					Kind:  INT,
+					Value: "100",
+				})
+				m.Blocks = make([]*Block, 1, 1)
+				m.PushStmts(S(Call(X("Redecl"), 11)))
+				return m
+			}(),
+			"Machine:\n    PreprocessorMode: false\n    Op: [OpHalt]\n    Values: (len: 0)\n    Exprs:\n          #0 100\n    Stmts:\n          #0 Redecl<VPUverse(0)>(11)\n    Blocks:\n    Blocks (other):\n    Frames:\n    Exceptions:\n",
+		},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.in.String()
+			if diff := cmp.Diff(got, tt.want); diff != "" {
+				t.Fatalf("Mismatch: got - want +\n%s", diff)
+			}
+		})
+	}
+}

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/davecgh/go-spew v1.1.1
 	github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0
 	github.com/fortytw2/leaktest v1.3.0
+	github.com/google/go-cmp v0.6.0
 	github.com/google/gofuzz v1.2.0
 	github.com/gorilla/websocket v1.5.3
 	github.com/libp2p/go-buffer-pool v0.1.0

misc/docs-linter/jsx.go

@@ -50,7 +50,7 @@ func lintJSX(filepathToJSX map[string][]string) (string, error) {
 				found = true
 			}
 
-			output.WriteString(fmt.Sprintf(">>> %s (found in file: %s)\n", tag, filePath))
+			fmt.Fprintf(&output, ">>> %s (found in file: %s)\n", tag, filePath)
 		}
 	}
 

misc/docs-linter/links.go

@@ -80,7 +80,7 @@ func lintLocalLinks(filepathToLinks map[string][]string, docsPath string) (strin
 					found = true
 				}
 
-				output.WriteString(fmt.Sprintf(">>> %s (found in file: %s)\n", link, filePath))
+				fmt.Fprintf(&output, ">>> %s (found in file: %s)\n", link, filePath)
 			}
 		}
 	}

misc/docs-linter/main.go

@@ -61,8 +61,8 @@ func execLint(cfg *cfg, ctx context.Context) (string, error) {
 	}
 
 	// Main buffer to write to the end user after linting
-	var output bytes.Buffer
-	output.WriteString(fmt.Sprintf("Linting %s...\n", absPath))
+        var output bytes.Buffer
+	fmt.Fprintf(&output, "Linting %s...\n", absPath)
 
 	// Find docs files to lint
 	mdFiles, err := findFilePaths(cfg.docsPath)

misc/docs-linter/urls.go

@@ -66,7 +66,7 @@ func lintURLs(filepathToURLs map[string][]string, ctx context.Context) (string,
 						found = true
 					}
 
-					output.WriteString(fmt.Sprintf(">>> %s (found in file: %s)\n", url, filePath))
+					fmt.Fprintf(&output, ">>> %s (found in file: %s)\n", url, filePath)
 					lock.Unlock()
 				}
 

tm2/pkg/bft/rpc/lib/server/handlers.go

@@ -939,7 +939,7 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st
 
 	for _, name := range noArgNames {
 		link := fmt.Sprintf("//%s/%s", r.Host, name)
-		buf.WriteString(fmt.Sprintf("<a href=\"%s\">%s</a></br>", link, link))
+		fmt.Fprintf(buf, "<a href=\"%s\">%s</a></br>", link, link)
 	}
 
 	buf.WriteString("<br>Endpoints that require arguments:<br>")
@@ -952,7 +952,7 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st
 				link += "&"
 			}
 		}
-		buf.WriteString(fmt.Sprintf("<a href=\"%s\">%s</a></br>", link, link))
+		fmt.Fprintf(buf, "<a href=\"%s\">%s</a></br>", link, link)
 	}
 	buf.WriteString("</body></html>")
 	w.Header().Set("Content-Type", "text/html")

tm2/pkg/bft/rpc/lib/server/write_endpoints_test.go

@@ -0,0 +1,33 @@
+package rpcserver
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
+
+	types "github.com/gnolang/gno/tm2/pkg/bft/rpc/lib/types"
+)
+
+func TestWriteListOfEndpoints(t *testing.T) {
+	funcMap := map[string]*RPCFunc{
+		"c": NewWSRPCFunc(func(ctx *types.Context, s string, i int) (string, error) { return "foo", nil }, "s,i"),
+		"d": {},
+	}
+
+	req, _ := http.NewRequest("GET", "http://localhost/", nil)
+	rec := httptest.NewRecorder()
+	writeListOfEndpoints(rec, req, funcMap)
+	res := rec.Result()
+	assert.Equal(t, res.StatusCode, 200, "Should always return 200")
+	blob, err := io.ReadAll(res.Body)
+	assert.NoError(t, err)
+	gotResp := string(blob)
+	wantResp := `<html><body><br>Available endpoints:<br><a href="//localhost/d">//localhost/d</a></br><br>Endpoints that require arguments:<br><a href="//localhost/c?s=_&i=_">//localhost/c?s=_&i=_</a></br></body></html>`
+	if diff := cmp.Diff(gotResp, wantResp); diff != "" {
+		t.Fatalf("Mismatch response: got - want +\n%s", diff)
+	}
+}

tm2/pkg/sdk/auth/params.go

@@ -69,15 +69,16 @@ func DefaultParams() Params {
 
 // String implements the stringer interface.
 func (p Params) String() string {
-	var sb strings.Builder
+	var builder strings.Builder
+	sb := &builder // Pointer for use with fmt.Fprintf
 	sb.WriteString("Params: \n")
-	sb.WriteString(fmt.Sprintf("MaxMemoBytes: %d\n", p.MaxMemoBytes))
-	sb.WriteString(fmt.Sprintf("TxSigLimit: %d\n", p.TxSigLimit))
-	sb.WriteString(fmt.Sprintf("TxSizeCostPerByte: %d\n", p.TxSizeCostPerByte))
-	sb.WriteString(fmt.Sprintf("SigVerifyCostED25519: %d\n", p.SigVerifyCostED25519))
-	sb.WriteString(fmt.Sprintf("SigVerifyCostSecp256k1: %d\n", p.SigVerifyCostSecp256k1))
-	sb.WriteString(fmt.Sprintf("GasPricesChangeCompressor: %d\n", p.GasPricesChangeCompressor))
-	sb.WriteString(fmt.Sprintf("TargetGasRatio: %d\n", p.TargetGasRatio))
+	fmt.Fprintf(sb, "MaxMemoBytes: %d\n", p.MaxMemoBytes)
+	fmt.Fprintf(sb, "TxSigLimit: %d\n", p.TxSigLimit)
+	fmt.Fprintf(sb, "TxSizeCostPerByte: %d\n", p.TxSizeCostPerByte)
+	fmt.Fprintf(sb, "SigVerifyCostED25519: %d\n", p.SigVerifyCostED25519)
+	fmt.Fprintf(sb, "SigVerifyCostSecp256k1: %d\n", p.SigVerifyCostSecp256k1)
+	fmt.Fprintf(sb, "GasPricesChangeCompressor: %d\n", p.GasPricesChangeCompressor)
+	fmt.Fprintf(sb, "TargetGasRatio: %d\n", p.TargetGasRatio)
 	return sb.String()
 }
 

tm2/pkg/sdk/auth/params_test.go

@@ -4,6 +4,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/require"
 )
 
@@ -105,3 +106,26 @@ func TestNewParams(t *testing.T) {
 		t.Errorf("NewParams() = %+v, want %+v", params, expectedParams)
 	}
 }
+
+func TestParamsString(t *testing.T) {
+	cases := []struct {
+		name   string
+		params Params
+		want   string
+	}{
+		{"blank params", Params{}, "Params: \nMaxMemoBytes: 0\nTxSigLimit: 0\nTxSizeCostPerByte: 0\nSigVerifyCostED25519: 0\nSigVerifyCostSecp256k1: 0\nGasPricesChangeCompressor: 0\nTargetGasRatio: 0\n"},
+		{"some values", Params{
+			MaxMemoBytes:      1_000_000,
+			TxSizeCostPerByte: 8192,
+		}, "Params: \nMaxMemoBytes: 1000000\nTxSigLimit: 0\nTxSizeCostPerByte: 8192\nSigVerifyCostED25519: 0\nSigVerifyCostSecp256k1: 0\nGasPricesChangeCompressor: 0\nTargetGasRatio: 0\n"},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.params.String()
+			if diff := cmp.Diff(got, tt.want); diff != "" {
+				t.Fatalf("Mismatch: got - want +\n%s", diff)
+			}
+		})
+	}
+}