Optimize encoding of composites

Switch from CBOR map to CBOR array to improve speed and preserve ordering. Remove unnecessary field sorting when decoding. Remove old backwards-compatibility decoding code (decoding type ID). Add tests, including round-trip for decoding old format and encoding new format. Closes #744
onflow · Apr 4, 2021 · 071f56d · 071f56d
1 parent cede38e
commit 071f56d
Show file tree

Hide file tree

Showing 3 changed files with 337 additions and 202 deletions.
diff --git a/runtime/interpreter/decode.go b/runtime/interpreter/decode.go
@@ -25,7 +25,6 @@ import (
 	"io"
 	"math"
 	"math/big"
-	"sort"
 	"strconv"
 	"strings"
 
@@ -528,8 +527,7 @@ func (d *DecoderV4) decodeAddressLocation(v interface{}) (common.Location, error
 }
 
 func (d *DecoderV4) decodeComposite(v interface{}, path []string) (*CompositeValue, error) {
-
-	encoded, ok := v.(map[interface{}]interface{})
+	encoded, ok := v.(cborArray)
 	if !ok {
 		return nil, fmt.Errorf(
 			"invalid composite encoding (@ %s): %T",
@@ -538,6 +536,15 @@ func (d *DecoderV4) decodeComposite(v interface{}, path []string) (*CompositeVal
 		)
 	}
 
+	if len(encoded) != encodedCompositeValueLength {
+		return nil, fmt.Errorf(
+			"invalid composite encoding (@ %s): invalid size: expected %d, got %d",
+			strings.Join(path, "."),
+			encodedCompositeValueLength,
+			len(encoded),
+		)
+	}
+
 	// Location
 
 	location, err := d.decodeLocation(encoded[encodedCompositeValueLocationFieldKey])
@@ -549,55 +556,16 @@ func (d *DecoderV4) decodeComposite(v interface{}, path []string) (*CompositeVal
 		)
 	}
 
-	// Qualified identifier or Type ID.
-	//
-	// An earlier version of the format stored the whole type ID.
-	// However, the composite already stores the location,
-	// so the current version of the format only stores the qualified identifier.
-
-	var qualifiedIdentifier string
+	// Qualified identifier
 
 	qualifiedIdentifierField := encoded[encodedCompositeValueQualifiedIdentifierFieldKey]
-	if qualifiedIdentifierField != nil {
-		qualifiedIdentifier, ok = qualifiedIdentifierField.(string)
-		if !ok {
-			return nil, fmt.Errorf(
-				"invalid composite qualified identifier encoding (@ %s): %T",
-				strings.Join(path, "."),
-				qualifiedIdentifierField,
-			)
-		}
-	} else {
-		typeIDField := encoded[encodedCompositeValueTypeIDFieldKey]
-		if typeIDField != nil {
-
-			encodedTypeID, ok := typeIDField.(string)
-			if !ok {
-				return nil, fmt.Errorf(
-					"invalid composite type ID encoding (@ %s): %T",
-					strings.Join(path, "."),
-					typeIDField,
-				)
-			}
-
-			_, qualifiedIdentifier, err = common.DecodeTypeID(encodedTypeID)
-			if err != nil {
-				return nil, fmt.Errorf(
-					"invalid composite type ID (@ %s): %w",
-					strings.Join(path, "."),
-					err,
-				)
-			}
-
-			// Special case: The decoded location might be an address location which has no name
-
-			location = d.inferAddressLocationName(location, qualifiedIdentifier)
-		} else {
-			return nil, fmt.Errorf(
-				"missing composite qualified identifier or type ID (@ %s)",
-				strings.Join(path, "."),
-			)
-		}
+	qualifiedIdentifier, ok := qualifiedIdentifierField.(string)
+	if !ok {
+		return nil, fmt.Errorf(
+			"invalid composite qualified identifier encoding (@ %s): %T",
+			strings.Join(path, "."),
+			qualifiedIdentifierField,
+		)
 	}
 
 	// Kind
@@ -616,7 +584,7 @@ func (d *DecoderV4) decodeComposite(v interface{}, path []string) (*CompositeVal
 	// Fields
 
 	fieldsField := encoded[encodedCompositeValueFieldsFieldKey]
-	encodedFields, ok := fieldsField.(map[interface{}]interface{})
+	encodedFields, ok := fieldsField.(cborArray)
 	if !ok {
 		return nil, fmt.Errorf(
 			"invalid composite fields encoding (@ %s): %T",
@@ -625,37 +593,31 @@ func (d *DecoderV4) decodeComposite(v interface{}, path []string) (*CompositeVal
 		)
 	}
 
-	// Gather all field names and sort them lexicographically
+	if len(encodedFields)%2 == 1 {
+		return nil, fmt.Errorf(
+			"invalid composite fields encoding (@ %s): fields should have even number of elements: got %d",
+			strings.Join(path, "."),
+			len(encodedFields),
+		)
+	}
 
-	var fieldNames []string
+	fields := NewStringValueOrderedMap()
 
-	index := 0
+	for i := 0; i < len(encodedFields); i += 2 {
 
-	for fieldName := range encodedFields { //nolint:maprangecheck
-		nameString, ok := fieldName.(string)
+		// field name
+		fieldName, ok := encodedFields[i].(string)
 		if !ok {
 			return nil, fmt.Errorf(
 				"invalid composite field name encoding (@ %s, %d): %T",
 				strings.Join(path, "."),
-				index,
-				fieldName,
+				i/2,
+				encodedFields[i],
 			)
 		}
 
-		fieldNames = append(fieldNames, nameString)
-
-		index++
-	}
-
-	// Decode all fields in lexicographic order
-
-	sort.Strings(fieldNames)
-
-	fields := NewStringValueOrderedMap()
-
-	for _, fieldName := range fieldNames {
-
-		value := encodedFields[fieldName]
+		// field value
+		value := encodedFields[i+1]
 
 		valuePath := append(path[:], fieldName)
 		decodedValue, err := d.decodeValue(value, valuePath)

diff --git a/runtime/interpreter/encode.go b/runtime/interpreter/encode.go
@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"io"
 	"math/big"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -791,6 +792,12 @@ const (
 	encodedCompositeValueKindFieldKey                uint64 = 2
 	encodedCompositeValueFieldsFieldKey              uint64 = 3
 	encodedCompositeValueQualifiedIdentifierFieldKey uint64 = 4
+
+	// !!! *WARNING* !!!
+	//
+	// encodedCompositeValueLength MUST be updated when new element is added.
+	// It is used to verify encoded composites length during decoding.
+	encodedCompositeValueLength int = 5
 )
 
 func (e *Encoder) prepareCompositeValue(
@@ -801,19 +808,34 @@ func (e *Encoder) prepareCompositeValue(
 	interface{},
 	error,
 ) {
-	fields := make(map[string]interface{}, v.Fields.Len())
+	fieldPairs := v.Fields.pairs
+	fieldLen := len(fieldPairs)
+
+	// Sort field names lexicographically.
+	fieldNames := make([]string, fieldLen)
+
+	index := 0
+	for name := range fieldPairs {
+		fieldNames[index] = name
+		index++
+	}
 
-	for pair := v.Fields.Oldest(); pair != nil; pair = pair.Next() {
-		fieldName := pair.Key
-		value := pair.Value
+	sort.Strings(fieldNames)
+
+	// Create fields (key and value) array, sorted by field name.
+	fields := make(cborArray, fieldLen*2)
+
+	for i, fieldName := range fieldNames {
+		value := fieldPairs[fieldName].Value
 
 		valuePath := append(path[:], fieldName)
 
 		prepared, err := e.prepare(value, valuePath, deferrals)
 		if err != nil {
 			return nil, err
 		}
-		fields[fieldName] = prepared
+
+		fields[i*2], fields[i*2+1] = fieldName, prepared
 	}
 
 	location, err := e.prepareLocation(v.Location)
@@ -823,7 +845,7 @@ func (e *Encoder) prepareCompositeValue(
 
 	return cbor.Tag{
 		Number: cborTagCompositeValue,
-		Content: cborMap{
+		Content: cborArray{
 			encodedCompositeValueLocationFieldKey:            location,
 			encodedCompositeValueKindFieldKey:                uint(v.Kind),
 			encodedCompositeValueFieldsFieldKey:              fields,