From ff8f2c3944d172b99f0c384936c4f551f7eb2077 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Wed, 1 Jan 2025 15:21:42 -0800 Subject: [PATCH] perf: use fmt.Fprintf to avoid unnecessary string+args + WriteString MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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.79ms ± 1% 8.32ms ± 2% -5.36% (p=0.000 n=18+18) name old alloc/op new alloc/op delta StringLargeData-8 8.34MB ± 0% 7.78MB ± 0% -6.71% (p=0.000 n=20+20) name old allocs/op new allocs/op delta StringLargeData-8 90.1k ± 0% 70.1k ± 0% -22.20% (p=0.000 n=20+20) ``` for heavily used code this is going to reduce on garbage collection cycles too. Fixes #3433 --- contribs/gnomigrate/internal/txs/txs.go | 2 +- gnovm/pkg/gnolang/gno_test.go | 2 ++ gnovm/pkg/gnolang/machine.go | 34 ++++++++++++------------- misc/docs-linter/jsx.go | 2 +- misc/docs-linter/links.go | 2 +- misc/docs-linter/main.go | 4 +-- misc/docs-linter/urls.go | 2 +- tm2/pkg/bft/rpc/lib/server/handlers.go | 4 +-- tm2/pkg/sdk/auth/params.go | 16 ++++++------ 9 files changed, 35 insertions(+), 33 deletions(-) diff --git a/contribs/gnomigrate/internal/txs/txs.go b/contribs/gnomigrate/internal/txs/txs.go index 4c65ca6ef0b..231428d5064 100644 --- 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 index 89458667997..5a8c6faf315 100644 --- 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 index 4480a89d16f..4207ba53b3e 100644 --- a/gnovm/pkg/gnolang/machine.go +++ b/gnovm/pkg/gnolang/machine.go @@ -2131,25 +2131,25 @@ func (m *Machine) String() string { totalLength = vsLength + ssLength + xsLength + bsLength + obsLength + fsLength + exceptionsLength ) - var builder strings.Builder + builder := new(strings.Builder) 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 +2166,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 +2187,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,12 +2206,12 @@ 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(" ")) } } } @@ -2219,17 +2219,17 @@ func (m *Machine) String() string { 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 index d0307680a0c..eb041a78386 100644 --- 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 index 744917d8dfb..e34d35d9f58 100644 --- 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 index 97d80316108..bc77086c0a3 100644 --- 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)) + output := new(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 index 093e624d81e..098d0a05524 100644 --- 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 index 88ee26da4a9..9e10596a975 100644 --- 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("%s
", link, link)) + fmt.Fprintf(buf, "%s
", link, link) } buf.WriteString("
Endpoints that require arguments:
") @@ -952,7 +952,7 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st link += "&" } } - buf.WriteString(fmt.Sprintf("%s
", link, link)) + fmt.Fprintf(buf, "%s
", link, link) } buf.WriteString("") w.Header().Set("Content-Type", "text/html") diff --git a/tm2/pkg/sdk/auth/params.go b/tm2/pkg/sdk/auth/params.go index 3fe08ed444d..b9cab0bec82 100644 --- a/tm2/pkg/sdk/auth/params.go +++ b/tm2/pkg/sdk/auth/params.go @@ -69,15 +69,15 @@ func DefaultParams() Params { // String implements the stringer interface. func (p Params) String() string { - var sb strings.Builder + sb := new(strings.Builder) 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() }