Ensure fixed-opaque types (arrays) implement EncodeTo() by pointer

[2opremio]

This PR switches the EncodeTo() implementation for fixed opaque types (Go arrays) to a pointer received.

Otherwise (if called by value), Go will make a heap allocation for every by-value call since the copy required by the call tends to escape the stack due to the large array sizes.

This finally reduces marhsalling allocations to a bare minimum and speeds up marshaling between 20 and 40%.

Using the benchmarks from stellar/go:

Before:

goos: darwin
goarch: amd64
pkg: github.com/stellar/go/benchmarks
cpu: Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
BenchmarkXDRMarshalWithReflection
BenchmarkXDRMarshalWithReflection-8                   	   95192	     13507 ns/op	    5128 B/op	     156 allocs/op
BenchmarkXDRMarshal
BenchmarkXDRMarshal-8                                 	 1081450	      1048 ns/op	    1352 B/op	      14 allocs/op
BenchmarkXDRMarshalWithEncodingBuffer
BenchmarkXDRMarshalWithEncodingBuffer-8               	 1927026	       707.4 ns/op	     176 B/op	       9 allocs/op
BenchmarkGXDRMarshal
BenchmarkGXDRMarshal-8                                	  132522	      7787 ns/op	    2152 B/op	     157 allocs/op
BenchmarkXDRMarshalHex
BenchmarkXDRMarshalHex-8                              	  481605	      2461 ns/op	    3640 B/op	      19 allocs/op
BenchmarkXDRMarshalHexWithEncodingBuffer
BenchmarkXDRMarshalHexWithEncodingBuffer-8            	  848468	      1620 ns/op	    1072 B/op	      10 allocs/op
BenchmarkXDRUnsafeMarshalHexWithEncodingBuffer
BenchmarkXDRUnsafeMarshalHexWithEncodingBuffer-8      	  997700	      1067 ns/op	     176 B/op	       9 allocs/op
BenchmarkXDRMarshalBase64
BenchmarkXDRMarshalBase64-8                           	  529298	      1968 ns/op	    3000 B/op	      19 allocs/op
BenchmarkXDRMarshalBase64WithEncodingBuffer
BenchmarkXDRMarshalBase64WithEncodingBuffer-8         	  998764	      1349 ns/op	     752 B/op	      10 allocs/op
BenchmarkXDRUnsafeMarshalBase64WithEncodingBuffer
BenchmarkXDRUnsafeMarshalBase64WithEncodingBuffer-8   	 1000000	      1013 ns/op	     176 B/op	       9 allocs/op
PASS

After:

goos: darwin
goarch: amd64
pkg: github.com/stellar/go/benchmarks
cpu: Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
BenchmarkXDRMarshalWithReflection
BenchmarkXDRMarshalWithReflection-8                   	  100161	     12249 ns/op	    5128 B/op	     156 allocs/op
BenchmarkXDRMarshal
BenchmarkXDRMarshal-8                                 	 1259341	       954.3 ns/op	    1208 B/op	       6 allocs/op
BenchmarkXDRMarshalWithEncodingBuffer
BenchmarkXDRMarshalWithEncodingBuffer-8               	 2368460	       488.1 ns/op	      32 B/op	       1 allocs/op
BenchmarkGXDRMarshal
BenchmarkGXDRMarshal-8                                	  151405	      7731 ns/op	    2152 B/op	     157 allocs/op
BenchmarkXDRMarshalHex
BenchmarkXDRMarshalHex-8                              	  538134	      1944 ns/op	    3496 B/op	      11 allocs/op
BenchmarkXDRMarshalHexWithEncodingBuffer
BenchmarkXDRMarshalHexWithEncodingBuffer-8            	  965180	      1130 ns/op	     928 B/op	       2 allocs/op
BenchmarkXDRUnsafeMarshalHexWithEncodingBuffer
BenchmarkXDRUnsafeMarshalHexWithEncodingBuffer-8      	 1364654	       879.6 ns/op	      32 B/op	       1 allocs/op
BenchmarkXDRMarshalBase64
BenchmarkXDRMarshalBase64-8                           	  597410	      1833 ns/op	    2856 B/op	      11 allocs/op
BenchmarkXDRMarshalBase64WithEncodingBuffer
BenchmarkXDRMarshalBase64WithEncodingBuffer-8         	 1000000	      1035 ns/op	     608 B/op	       2 allocs/op
BenchmarkXDRUnsafeMarshalBase64WithEncodingBuffer
BenchmarkXDRUnsafeMarshalBase64WithEncodingBuffer-8   	 1393238	       846.7 ns/op	      32 B/op	       1 allocs/op
PASS

[leighmcculloch]

Looks good. One request to change (❗) below, and a suggestion (💡).

I recommend testing the stellar/go tests before merging. Also last time we made a change @bartekn recommended running verify range on Horizon for a few days.

[leighmcculloch]

💡 Suggested rewording of this explanation:

Suggested change

	# Implement EncodeTo by pointer in fixed opaque types (arrays)
	# otherwise (if called by value), Go will make a heap allocation
	# Make the receiver of EncodeTo a pointer in fixed opaque types (arrays)
	# otherwise (if called by value), Go will make a heap allocation

[2opremio]

recommend testing the stellar/go tests before merging.

I run the tests and they work

Also last time we made a change @bartekn recommended running verify range on Horizon for a few days.

I don't think it will be necessary this time since the changes to xdr_generated.go are minimal:

diff --git a/xdr/xdr_generated.go b/xdr/xdr_generated.go
index 0add7f20..8dc1e82c 100644
--- a/xdr/xdr_generated.go
+++ b/xdr/xdr_generated.go
@@ -1064,7 +1064,7 @@ func (e Thresholds) XDRMaxSize() int {
 }
 
 // EncodeTo encodes this value using the Encoder.
-func (s Thresholds) EncodeTo(e *xdr.Encoder) error {
+func (s *Thresholds) EncodeTo(e *xdr.Encoder) error {
        var err error
        _, err = e.EncodeFixedOpaque(s[:])
        if err != nil {
@@ -1327,7 +1327,7 @@ type PoolId Hash
 // EncodeTo encodes this value using the Encoder.
 func (s PoolId) EncodeTo(e *xdr.Encoder) error {
        var err error
-       err = Hash(s).EncodeTo(e)
+       err = (*Hash)(&s).EncodeTo(e)
        if err != nil {
                return err
        }
@@ -1371,7 +1371,7 @@ func (e AssetCode4) XDRMaxSize() int {
 }
 
 // EncodeTo encodes this value using the Encoder.
-func (s AssetCode4) EncodeTo(e *xdr.Encoder) error {
+func (s *AssetCode4) EncodeTo(e *xdr.Encoder) error {
        var err error
        _, err = e.EncodeFixedOpaque(s[:])
        if err != nil {
@@ -1417,7 +1417,7 @@ func (e AssetCode12) XDRMaxSize() int {
 }
 
 // EncodeTo encodes this value using the Encoder.
-func (s AssetCode12) EncodeTo(e *xdr.Encoder) error {
+func (s *AssetCode12) EncodeTo(e *xdr.Encoder) error {
        var err error
        _, err = e.EncodeFixedOpaque(s[:])
        if err != nil {
@@ -25927,7 +25927,7 @@ func (e Hash) XDRMaxSize() int {
 }
 
 // EncodeTo encodes this value using the Encoder.
-func (s Hash) EncodeTo(e *xdr.Encoder) error {
+func (s *Hash) EncodeTo(e *xdr.Encoder) error {
        var err error
        _, err = e.EncodeFixedOpaque(s[:])
        if err != nil {
@@ -25973,7 +25973,7 @@ func (e Uint256) XDRMaxSize() int {
 }
 
 // EncodeTo encodes this value using the Encoder.
-func (s Uint256) EncodeTo(e *xdr.Encoder) error {
+func (s *Uint256) EncodeTo(e *xdr.Encoder) error {
        var err error
        _, err = e.EncodeFixedOpaque(s[:])
        if err != nil {
@@ -26747,7 +26747,7 @@ func (e SignatureHint) XDRMaxSize() int {
 }
 
 // EncodeTo encodes this value using the Encoder.
-func (s SignatureHint) EncodeTo(e *xdr.Encoder) error {
+func (s *SignatureHint) EncodeTo(e *xdr.Encoder) error {
        var err error
        _, err = e.EncodeFixedOpaque(s[:])
        if err != nil {

[leighmcculloch]

The diff you posted was so useful in understanding the change that I think we should add a CI run that runs against stellar/go and prints out the diff. I created #69.

[leighmcculloch]

I'm surprised this causes such a huge gain, because I would have thought the pointer receivers increase the change something gets moved to the stack. Happy to be wrong about this. Nice find!