Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion contribs/gnomigrate/internal/txs/txs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
2 changes: 2 additions & 0 deletions gnovm/pkg/gnolang/gno_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
39 changes: 22 additions & 17 deletions gnovm/pkg/gnolang/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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)
odeke-em marked this conversation as resolved.
Show resolved Hide resolved

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")
Expand All @@ -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)
}
}

Expand All @@ -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")
Expand All @@ -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()
Expand Down
59 changes: 59 additions & 0 deletions gnovm/pkg/gnolang/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
})
}
}
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion misc/docs-linter/jsx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
2 changes: 1 addition & 1 deletion misc/docs-linter/links.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions misc/docs-linter/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion misc/docs-linter/urls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
4 changes: 2 additions & 2 deletions tm2/pkg/bft/rpc/lib/server/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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>")
Expand All @@ -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")
Expand Down
33 changes: 33 additions & 0 deletions tm2/pkg/bft/rpc/lib/server/write_endpoints_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
17 changes: 9 additions & 8 deletions tm2/pkg/sdk/auth/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
24 changes: 24 additions & 0 deletions tm2/pkg/sdk/auth/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"reflect"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -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)
}
})
}
}
Loading