From 071f56d5fc209c2fc416385a980175f55364b3ce Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Sat, 3 Apr 2021 22:12:21 -0500 Subject: [PATCH] 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 --- runtime/interpreter/decode.go | 106 +++---- runtime/interpreter/encode.go | 34 ++- runtime/interpreter/encoding_test.go | 399 ++++++++++++++++++--------- 3 files changed, 337 insertions(+), 202 deletions(-) diff --git a/runtime/interpreter/decode.go b/runtime/interpreter/decode.go index ab3d1350b8..b2f9a5f64b 100644 --- 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 index 216140ac72..82be78096e 100644 --- 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,11 +808,25 @@ 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) @@ -813,7 +834,8 @@ func (e *Encoder) prepareCompositeValue( 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, diff --git a/runtime/interpreter/encoding_test.go b/runtime/interpreter/encoding_test.go index 1609d2a3c1..2b5d11a9d7 100644 --- a/runtime/interpreter/encoding_test.go +++ b/runtime/interpreter/encoding_test.go @@ -508,41 +508,79 @@ func TestEncodeDecodeComposite(t *testing.T) { ) expected.modified = false + encoded := []byte{ + // tag + 0xd8, cborTagCompositeValue, + // array, 5 items follow + 0x85, + + // tag + 0xd8, cborTagStringLocation, + // UTF-8 string, length 4 + 0x64, + // t, e, s, t + 0x74, 0x65, 0x73, 0x74, + + // nil + 0xf6, + + // positive integer 1 + 0x1, + + // array, 0 items follow + 0x80, + + // UTF-8 string, length 10 + 0x6a, + 0x54, 0x65, 0x73, 0x74, 0x53, 0x74, 0x72, 0x75, 0x63, 0x74, + } + + version3Encoded := []byte{ + // tag + 0xd8, cborTagCompositeValue, + // map, 4 pairs of items follow + 0xa4, + // key 0 + 0x0, + // tag + 0xd8, cborTagStringLocation, + // UTF-8 string, length 4 + 0x64, + // t, e, s, t + 0x74, 0x65, 0x73, 0x74, + // key 2 + 0x2, + // positive integer 1 + 0x1, + // key 3 + 0x3, + // map, 0 pairs of items follow + 0xa0, + // key 4 + 0x4, + // UTF-8 string, length 10 + 0x6a, + 0x54, 0x65, 0x73, 0x74, 0x53, 0x74, 0x72, 0x75, 0x63, 0x74, + } + testEncodeDecode(t, encodeDecodeTest{ - value: expected, - encoded: []byte{ - // tag - 0xd8, cborTagCompositeValue, - // map, 4 pairs of items follow - 0xa4, - // key 0 - 0x0, - // tag - 0xd8, cborTagStringLocation, - // UTF-8 string, length 4 - 0x64, - // t, e, s, t - 0x74, 0x65, 0x73, 0x74, - // key 2 - 0x2, - // positive integer 1 - 0x1, - // key 3 - 0x3, - // map, 0 pairs of items follow - 0xa0, - // key 4 - 0x4, - // UTF-8 string, length 10 - 0x6a, - 0x54, 0x65, 0x73, 0x74, 0x53, 0x74, 0x72, 0x75, 0x63, 0x74, - }, + value: expected, + encoded: encoded, }, ) + + testEncodeDecodeOldFormat(t, + encodeDecodeTest{ + value: expected, + encoded: encoded, + }, + 3, + version3Encoded, + ) }) - t.Run("empty structure, string location, type ID", func(t *testing.T) { + t.Run("empty structure, string location, type ID, version <= 3", func(t *testing.T) { expected := NewCompositeValue( utils.TestLocation, "TestStruct", @@ -554,8 +592,10 @@ func TestEncodeDecodeComposite(t *testing.T) { testEncodeDecode(t, encodeDecodeTest{ - decodeOnly: true, - decodedValue: expected, + decodeVersionOverride: true, + decodeVersion: 3, + decodeOnly: true, + decodedValue: expected, encoded: []byte{ // tag 0xd8, cborTagCompositeValue, @@ -591,7 +631,7 @@ func TestEncodeDecodeComposite(t *testing.T) { ) }) - t.Run("empty structure, address location without name", func(t *testing.T) { + t.Run("empty structure, address location without name, version <= 3", func(t *testing.T) { expected := NewCompositeValue( common.AddressLocation{ Address: common.BytesToAddress([]byte{0x1}), @@ -606,8 +646,10 @@ func TestEncodeDecodeComposite(t *testing.T) { testEncodeDecode(t, encodeDecodeTest{ - decodeOnly: true, - decodedValue: expected, + decodeOnly: true, + decodedValue: expected, + decodeVersionOverride: true, + decodeVersion: 3, encoded: []byte{ // tag 0xd8, cborTagCompositeValue, @@ -662,55 +704,107 @@ func TestEncodeDecodeComposite(t *testing.T) { ) expected.modified = false + encoded := []byte{ + // tag + 0xd8, cborTagCompositeValue, + // array, 5 items follow + 0x85, + + // tag + 0xd8, cborTagStringLocation, + // UTF-8 string, length 4 + 0x64, + // t, e, s, t + 0x74, 0x65, 0x73, 0x74, + + // nil + 0xf6, + + // positive integer 2 + 0x2, + + // array, 4 items follow + 0x84, + // UTF-8 string, length 6 + 0x66, + // s, t, r, i, n, g + 0x73, 0x74, 0x72, 0x69, 0x6e, 0x67, + // UTF-8 string, length 4 + 0x64, + // t, e, s, t + 0x74, 0x65, 0x73, 0x74, + // UTF-8 string, length 4 + 0x64, + // t, r, u, e + 0x74, 0x72, 0x75, 0x65, + // true + 0xf5, + + // UTF-8 string, length 12 + 0x6c, + 0x54, 0x65, 0x73, 0x74, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, + } + + version3Encoded := []byte{ + // tag + 0xd8, cborTagCompositeValue, + // map, 4 pairs of items follow + 0xa4, + // key 0 + 0x0, + // tag + 0xd8, cborTagStringLocation, + // UTF-8 string, length 4 + 0x64, + // t, e, s, t + 0x74, 0x65, 0x73, 0x74, + // key 2 + 0x2, + // positive integer 2 + 0x2, + // key 3 + 0x3, + // map, 2 pairs of items follow + 0xa2, + // UTF-8 string, length 4 + 0x64, + // t, r, u, e + 0x74, 0x72, 0x75, 0x65, + // true + 0xf5, + // UTF-8 string, length 6 + 0x66, + // s, t, r, i, n, g + 0x73, 0x74, 0x72, 0x69, 0x6e, 0x67, + // UTF-8 string, length 4 + 0x64, + // t, e, s, t + 0x74, 0x65, 0x73, 0x74, + // key 4 + 0x4, + // UTF-8 string, length 12 + 0x6c, + 0x54, 0x65, 0x73, 0x74, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, + } + testEncodeDecode(t, encodeDecodeTest{ - value: expected, - encoded: []byte{ - // tag - 0xd8, cborTagCompositeValue, - // map, 4 pairs of items follow - 0xa4, - // key 0 - 0x0, - // tag - 0xd8, cborTagStringLocation, - // UTF-8 string, length 4 - 0x64, - // t, e, s, t - 0x74, 0x65, 0x73, 0x74, - // key 2 - 0x2, - // positive integer 2 - 0x2, - // key 3 - 0x3, - // map, 2 pairs of items follow - 0xa2, - // UTF-8 string, length 4 - 0x64, - // t, r, u, e - 0x74, 0x72, 0x75, 0x65, - // true - 0xf5, - // UTF-8 string, length 6 - 0x66, - // s, t, r, i, n, g - 0x73, 0x74, 0x72, 0x69, 0x6e, 0x67, - // UTF-8 string, length 4 - 0x64, - // t, e, s, t - 0x74, 0x65, 0x73, 0x74, - // key 4 - 0x4, - // UTF-8 string, length 12 - 0x6c, - 0x54, 0x65, 0x73, 0x74, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, - }, + value: expected, + encoded: encoded, + }, + ) + + testEncodeDecodeOldFormat(t, + encodeDecodeTest{ + value: expected, + encoded: encoded, }, + 3, + version3Encoded, ) }) - t.Run("non-empty resource, type ID", func(t *testing.T) { + t.Run("non-empty resource, type ID, version <= 3", func(t *testing.T) { stringValue := NewStringValue("test") stringValue.modified = false @@ -729,8 +823,10 @@ func TestEncodeDecodeComposite(t *testing.T) { testEncodeDecode(t, encodeDecodeTest{ - decodeOnly: true, - decodedValue: expected, + decodeVersionOverride: true, + decodeVersion: 3, + decodeOnly: true, + decodedValue: expected, encoded: []byte{ // tag 0xd8, cborTagCompositeValue, @@ -778,7 +874,7 @@ func TestEncodeDecodeComposite(t *testing.T) { ) }) - t.Run("empty, address location, nested", func(t *testing.T) { + t.Run("empty, address location, nested, version <= 3", func(t *testing.T) { expected := NewCompositeValue( common.AddressLocation{ @@ -795,7 +891,9 @@ func TestEncodeDecodeComposite(t *testing.T) { testEncodeDecode(t, encodeDecodeTest{ - decodedValue: expected, + decodeVersionOverride: true, + decodeVersion: 3, + decodedValue: expected, encoded: []byte{ // tag 0xd8, cborTagCompositeValue, @@ -835,9 +933,11 @@ func TestEncodeDecodeComposite(t *testing.T) { ) }) - t.Run("empty, address location, address too long", func(t *testing.T) { + t.Run("empty, address location, address too long, version <= 3", func(t *testing.T) { testEncodeDecode(t, encodeDecodeTest{ + decodeVersionOverride: true, + decodeVersion: 3, encoded: []byte{ // tag 0xd8, cborTagCompositeValue, @@ -886,52 +986,101 @@ func TestEncodeDecodeComposite(t *testing.T) { ) expected.modified = false + encoded := []byte{ + // tag + 0xd8, cborTagCompositeValue, + // array, 5 items follow + 0x85, + + // tag + 0xd8, cborTagAddressLocation, + // map, 2 pairs of items follow + 0xa2, + // key 0 + 0x0, + // byte sequence, length 1 + 0x41, + // positive integer 1 + 0x1, + // key 1 + 0x1, + // UTF-8 string, length 10 + 0x6a, + 0x54, 0x65, 0x73, 0x74, 0x53, 0x74, 0x72, 0x75, + 0x63, 0x74, + + // nil + 0xf6, + + // positive integer 1 + 0x1, + + // array, 0 items follow + 0x80, + + // UTF-8 string, length 10 + 0x6a, + 0x54, 0x65, 0x73, 0x74, 0x53, 0x74, 0x72, 0x75, + 0x63, 0x74, + } + + version3Encoded := []byte{ + // tag + 0xd8, cborTagCompositeValue, + // map, 4 pairs of items follow + 0xa4, + // key 0 + 0x0, + // tag + 0xd8, cborTagAddressLocation, + // map, 4 pairs of items follow + 0xa2, + // key 0 + 0x0, + // byte sequence, length 1 + 0x41, + // positive integer 1 + 0x1, + // key 1 + 0x1, + // UTF-8 string, length 10 + 0x6a, + 0x54, 0x65, 0x73, 0x74, 0x53, 0x74, 0x72, 0x75, + 0x63, 0x74, + // key 2 + 0x2, + // positive integer 1 + 0x1, + // key 3 + 0x3, + // map, 0 pairs of items follow + 0xa0, + // key 4 + 0x4, + // UTF-8 string, length 10 + 0x6a, + 0x54, 0x65, 0x73, 0x74, 0x53, 0x74, 0x72, 0x75, + 0x63, 0x74, + } + testEncodeDecode(t, encodeDecodeTest{ - value: expected, - encoded: []byte{ - // tag - 0xd8, cborTagCompositeValue, - // map, 4 pairs of items follow - 0xa4, - // key 0 - 0x0, - // tag - 0xd8, cborTagAddressLocation, - // map, 4 pairs of items follow - 0xa2, - // key 0 - 0x0, - // byte sequence, length 1 - 0x41, - // positive integer 1 - 0x1, - // key 1 - 0x1, - // UTF-8 string, length 10 - 0x6a, - 0x54, 0x65, 0x73, 0x74, 0x53, 0x74, 0x72, 0x75, - 0x63, 0x74, - // key 2 - 0x2, - // positive integer 1 - 0x1, - // key 3 - 0x3, - // map, 0 pairs of items follow - 0xa0, - // key 4 - 0x4, - // UTF-8 string, length 10 - 0x6a, - 0x54, 0x65, 0x73, 0x74, 0x53, 0x74, 0x72, 0x75, - 0x63, 0x74, - }, + value: expected, + encoded: encoded, + }, + ) + + testEncodeDecodeOldFormat(t, + encodeDecodeTest{ + value: expected, + encoded: encoded, }, + 3, + version3Encoded, ) }) - t.Run("empty, address location, address too long", func(t *testing.T) { + t.Run("empty, address location, address too long, version <= 3", func(t *testing.T) { testEncodeDecode(t, encodeDecodeTest{ encoded: []byte{ @@ -943,7 +1092,7 @@ func TestEncodeDecodeComposite(t *testing.T) { 0x0, // tag 0xd8, cborTagAddressLocation, - // map, 4 pairs of items follow + // map, 2 pairs of items follow 0xa2, // key 0 0x0, @@ -974,7 +1123,9 @@ func TestEncodeDecodeComposite(t *testing.T) { // map, 0 pairs of items follow 0xa0, }, - invalid: true, + invalid: true, + decodeVersionOverride: true, + decodeVersion: 3, }, ) })