perf: fix: tx/textual/valuerender: use io.WriteString to skip str->byteslice + fix negative sign dropping (#12815)

Noticed in an audit that the differeent value renderers perform an
expensive and unnecessary string->byteslice in cases where the output
write implements io.StringWriter. This change instead invokes

   io.WriteString(w, formatted)

instead of:

   w.Write([]byte(formatted))

and added benchmarks that show an improvement from just the 1 line change:

```shell
$ benchstat before.txt after.txt
name                        old time/op    new time/op    delta
IntValueRendererFormat-8      4.13µs ± 3%    3.95µs ± 6%   -4.55%  (p=0.000 n=15+14)
BytesValueRendererFormat-8    5.22ms ± 3%    4.77ms ± 5%   -8.60%  (p=0.000 n=15+14)

name                        old alloc/op   new alloc/op   delta
IntValueRendererFormat-8      3.64kB ± 0%    3.31kB ± 0%   -9.01%  (p=0.000 n=15+15)
BytesValueRendererFormat-8    12.6MB ± 0%     8.4MB ± 0%  -33.22%  (p=0.000 n=15+15)

name                        old allocs/op  new allocs/op  delta
IntValueRendererFormat-8        76.0 ± 0%      67.0 ± 0%  -11.84%  (p=0.000 n=15+15)
BytesValueRendererFormat-8      27.0 ± 0%      18.0 ± 0%  -33.33%  (p=0.000 n=15+15)
```

While here, implemented negative sign preservation because previously
the code wasn't tested for negative values so passing in negative values
such as:

  "-10000000.11"

would produce:

  "10'000'000.11"

instead of the proper value with the negative sign preserved:

  "-10'000'000.11"

Fixes #12810
Fixes #12812
This commit is contained in:
Emmanuel T Odeke 2022-08-04 01:27:54 -07:00 committed by GitHub
parent 970582d183
commit 8c23f6f957
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 90 additions and 14 deletions

View File

@ -39,5 +39,9 @@
["0.000000000000001000", "0.000000000000001"],
["0.000000000000000100", "0.0000000000000001"],
["0.000000000000000010", "0.00000000000000001"],
["0.000000000000000001", "0.000000000000000001"]
["0.000000000000000001", "0.000000000000000001"],
["-10.0", "-10"],
["-10000", "-10'000"],
["-9999", "-9'999"],
["-999999999999", "-999'999'999'999"]
]

View File

@ -0,0 +1,67 @@
package valuerenderer
import (
"bytes"
"context"
"testing"
"google.golang.org/protobuf/reflect/protoreflect"
)
var intValues = []protoreflect.Value{
protoreflect.ValueOfString("10.00"),
protoreflect.ValueOfString("999.00"),
protoreflect.ValueOfString("999.9999"),
protoreflect.ValueOfString("99999999.9999"),
protoreflect.ValueOfString("9999999999999999999"),
protoreflect.ValueOfString("1000000000000000000000000000000000000000000000000000000.00"),
protoreflect.ValueOfString("77777777777.777777777777777777777700"),
protoreflect.ValueOfString("-77777777777.777777777777777777777700"),
protoreflect.ValueOfString("777777777777777777777777.77777777700"),
}
func BenchmarkIntValueRendererFormat(b *testing.B) {
ctx := context.Background()
ivr := new(intValueRenderer)
buf := new(bytes.Buffer)
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
for _, value := range intValues {
if err := ivr.Format(ctx, value, buf); err != nil {
b.Fatal(err)
}
}
buf.Reset()
}
}
var byteValues = []protoreflect.Value{
protoreflect.ValueOfBytes(bytes.Repeat([]byte("abc"), 1<<20)),
protoreflect.ValueOfBytes([]byte("999.00")),
protoreflect.ValueOfBytes([]byte("999.9999")),
protoreflect.ValueOfBytes([]byte("99999999.9999")),
protoreflect.ValueOfBytes([]byte("9999999999999999999")),
protoreflect.ValueOfBytes([]byte("1000000000000000000000000000000000000000000000000000000.00")),
protoreflect.ValueOfBytes([]byte("77777777777.777777777777777777777700")),
protoreflect.ValueOfBytes([]byte("-77777777777.777777777777777777777700")),
protoreflect.ValueOfBytes([]byte("777777777777777777777777.77777777700")),
}
func BenchmarkBytesValueRendererFormat(b *testing.B) {
ctx := context.Background()
bvr := new(bytesValueRenderer)
buf := new(bytes.Buffer)
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
for _, value := range byteValues {
if err := bvr.Format(ctx, value, buf); err != nil {
b.Fatal(err)
}
}
buf.Reset()
}
}

View File

@ -8,14 +8,14 @@ import (
"google.golang.org/protobuf/reflect/protoreflect"
)
//bytesValueRenderer implements ValueRenderer for bytes
// bytesValueRenderer implements ValueRenderer for bytes
type bytesValueRenderer struct {
}
var _ ValueRenderer = bytesValueRenderer{}
func (vr bytesValueRenderer) Format(ctx context.Context, v protoreflect.Value, w io.Writer) error {
_, err := w.Write([]byte(base64.StdEncoding.EncodeToString(v.Bytes())))
_, err := io.WriteString(w, base64.StdEncoding.EncodeToString(v.Bytes()))
return err
}

View File

@ -21,7 +21,7 @@ func (vr decValueRenderer) Format(_ context.Context, v protoreflect.Value, w io.
return err
}
_, err = w.Write([]byte(formatted))
_, err = io.WriteString(w, formatted)
return err
}

View File

@ -18,7 +18,7 @@ func (vr intValueRenderer) Format(_ context.Context, v protoreflect.Value, w io.
return err
}
_, err = w.Write([]byte(formatted))
_, err = io.WriteString(w, formatted)
return err
}
@ -30,7 +30,9 @@ func (vr intValueRenderer) Parse(_ context.Context, r io.Reader) (protoreflect.V
// operates with string manipulation (instead of manipulating the int or sdk.Int
// object).
func formatInteger(v string) (string, error) {
sign := ""
if v[0] == '-' {
sign = "-"
v = v[1:]
}
if len(v) > 1 {
@ -44,5 +46,5 @@ func formatInteger(v string) (string, error) {
v = v[:outputIndex] + thousandSeparator + v[outputIndex:]
}
return v, nil
return sign + v, nil
}

View File

@ -73,15 +73,18 @@ func TestFormatDecimal(t *testing.T) {
require.NoError(t, err)
for _, tc := range testcases {
d, err := math.LegacyNewDecFromStr(tc[0])
require.NoError(t, err)
r, err := valueRendererOf(d)
require.NoError(t, err)
b := new(strings.Builder)
err = r.Format(context.Background(), protoreflect.ValueOf(tc[0]), b)
require.NoError(t, err)
tc := tc
t.Run(tc[0], func(t *testing.T) {
d, err := math.LegacyNewDecFromStr(tc[0])
require.NoError(t, err)
r, err := valueRendererOf(d)
require.NoError(t, err)
b := new(strings.Builder)
err = r.Format(context.Background(), protoreflect.ValueOf(tc[0]), b)
require.NoError(t, err)
require.Equal(t, tc[1], b.String())
require.Equal(t, tc[1], b.String())
})
}
}