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

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 $ benchstat before.txt after.txt 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
gnolang · Jan 9, 2025 · a87a2fd · a87a2fd
1 parent 60acdf1
commit a87a2fd
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 33 deletions.
diff --git a/contribs/gnomigrate/internal/txs/txs.go b/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)
 			}
 		}

diff --git a/gnovm/pkg/gnolang/gno_test.go b/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()

diff --git a/gnovm/pkg/gnolang/machine.go b/gnovm/pkg/gnolang/machine.go
@@ -2131,25 +2131,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 +2167,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 +2188,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 +2207,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()

diff --git a/misc/docs-linter/jsx.go b/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)
 		}
 	}
 

diff --git a/misc/docs-linter/links.go b/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)
 			}
 		}
 	}

diff --git a/misc/docs-linter/main.go b/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)

diff --git a/misc/docs-linter/urls.go b/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()
 				}
 

diff --git a/tm2/pkg/bft/rpc/lib/server/handlers.go b/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")

diff --git a/tm2/pkg/sdk/auth/params.go b/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()
 }
-Original file line number
+Diff line change
@@ Expand Up @@
     				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)
     		}
     	}
@@ Expand Down @@