Skip to content

Commit

Permalink
gen/FromWire: Allocate pointer fields once
Browse files Browse the repository at this point in the history
In FromWire, when we decode pointer fields, our code takes the form,

```
name := value.GetString()  // Given (value wire.Value)
user.Name = &name
```

This results in an allocation for every optional field present over the
wire.

    name := value.GetString()
    user.Name = &name  // alloc

    // ...

    email := value.GetString()
    user.Email = &email  // alloc

    // ...

    age := value.GetInt8()
    user.Age = &age  // alloc

This changes how we generate FromWire for structs to instead allocate
space for all pointer fields once, and then re-use the same block.

    var ptrFields struct {
        Name string
        Email string
        Age int8
    }  // alloc

    ptrFields.Name = value.GetString()
    user.Name = &ptrFields.Name  // no alloc

    ptrFields.Email = value.GetString()
    user.Email = &ptrFields.Email  // no alloc

    ptrFields.Age = value.GetInt8()
    user.Age = &ptrFields.Age  // no alloc

Note that the pre-allocated struct also includes nested struct fields,
so we can avoid that allocation too.

    user.Comment = _Comment_Read(value)
        // func _Comment_Read(value wire.Value) *Comment {
        //   var c Comment
        //   c.FromWire(value)
        //   return &c  // alloc
        // }

    // Becomes,

    var ptrFields struct {
        // ...
        Comment Comment
    }

    ptrFields.Comment.FromWire(value)
    user.Comment = &ptrFields.Comment  // no alloc

This will have the following effects:

Structs with optional primitive fields, or other structs inside them
will go from N small allocations to one big allocation. For most
structs, the total amount of space allocated will remain largely the
same. However, for sparse structs with lots of optional fields where
only a handful of them are set, this will allocate more bytes and
therefore, hold onto more memory than they need.

We think that since the majority case is going to be structs with most
of their fields filled in, reducing the number of allocations takes
precedence.

Performance:

```
$ git co master
$ go test -run '^$' -bench ././Decode -benchmem -count 5 > before.txt
$ git co -
$ go test -run '^$' -bench ././Decode -benchmem -count 5 > after.txt
$ benchstat before.txt after.txt
name                                        old time/op    new time/op    delta
RoundTrip/PrimitiveOptionalStruct/Decode-4    2.93µs ± 5%    2.81µs ± 6%     ~     (p=0.095 n=5+5)
RoundTrip/Graph/Decode-4                      7.30µs ±17%    6.16µs ± 8%  -15.60%  (p=0.032 n=5+5)
RoundTrip/ContainersOfContainers/Decode-4     52.7µs ± 1%    57.7µs ±12%     ~     (p=0.730 n=4+5)

name                                        old alloc/op   new alloc/op   delta
RoundTrip/PrimitiveOptionalStruct/Decode-4    1.40kB ± 0%    1.41kB ± 0%   +0.43%  (p=0.008 n=5+5)
RoundTrip/Graph/Decode-4                      2.78kB ± 0%    2.78kB ± 0%     ~     (all equal)
RoundTrip/ContainersOfContainers/Decode-4     12.3kB ± 0%    12.3kB ± 0%     ~     (p=0.881 n=5+5)

name                                        old allocs/op  new allocs/op  delta
RoundTrip/PrimitiveOptionalStruct/Decode-4      14.0 ± 0%       8.0 ± 0%  -42.86%  (p=0.008 n=5+5)
RoundTrip/Graph/Decode-4                        32.0 ± 0%      29.0 ± 0%   -9.38%  (p=0.008 n=5+5)
RoundTrip/ContainersOfContainers/Decode-4        164 ± 0%       164 ± 0%     ~     (p=0.556 n=5+4)
```
  • Loading branch information
abhinav committed Jul 8, 2021
1 parent 14808ba commit f1e6392
Show file tree
Hide file tree
Showing 17 changed files with 547 additions and 276 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,4 @@ cover:
.PHONY: clean
clean:
go clean
rm $(GOBIN)/thriftrw
17 changes: 16 additions & 1 deletion gen/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,15 @@ func (f fieldGroupGenerator) FromWire(g Generator) error {
// }
// return &<$v>, nil
func (<$v> *<.Name>) FromWire(<$w> <$wire>.Value) error {
var ptrFields struct {
<range .Fields>
<- if or (isStructType .Type) (and (not .Required) (isPrimitiveType .Type)) ->
<goName .> <typeName .Type>
<- end>
<end>
}
_ = ptrFields
<if len .Fields> var err error <end>
<$f := newVar "field">
Expand All @@ -410,8 +419,14 @@ func (f fieldGroupGenerator) FromWire(g Generator) error {
if <$f>.Value.Type() == <typeCode .Type> {
<- $lhs := printf "%s.%s" $v (goName .) ->
<- $value := printf "%s.Value" $f ->
<- if .Required ->
<- if isStructType .Type ->
err = ptrFields.<goName .>.FromWire(<$value>)
<$lhs> = &ptrFields.<goName .>
<- else if .Required ->
<$lhs>, err = <fromWire .Type $value>
<- else if isPrimitiveType .Type ->
ptrFields.<goName .>, err = <fromWire .Type $value>
<$lhs> = &ptrFields.<goName .>
<- else ->
<fromWirePtr .Type $lhs $value>
<- end>
Expand Down
96 changes: 62 additions & 34 deletions gen/internal/tests/collision/collision.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions gen/internal/tests/containers/containers.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 10 additions & 6 deletions gen/internal/tests/enum_conflict/enum_conflict.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit f1e6392

Please sign in to comment.