From dc0a215596cc0ea9dd2f020f9cb3644e20c6fefc Mon Sep 17 00:00:00 2001 From: Or Hayat Date: Wed, 20 Mar 2024 15:56:50 +0200 Subject: [PATCH 1/2] remove redundent tryMarshaler calls in encodeString and encodeNumber both encodeString and encodeNumber get called from encode after tryMarshaller failled added benchmark for list of int and struct of int and string fields --- feature/dynamodb/attributevalue/encode.go | 10 -- .../dynamodb/attributevalue/encode_test.go | 91 +++++++++++++++++++ .../dynamodb/attributevalue/marshaler_test.go | 54 +++++++++++ 3 files changed, 145 insertions(+), 10 deletions(-) diff --git a/feature/dynamodb/attributevalue/encode.go b/feature/dynamodb/attributevalue/encode.go index f8b2246c894..f62000a68f0 100644 --- a/feature/dynamodb/attributevalue/encode.go +++ b/feature/dynamodb/attributevalue/encode.go @@ -714,11 +714,6 @@ func (e *Encoder) encodeScalar(v reflect.Value, fieldTag tag) (types.AttributeVa } func (e *Encoder) encodeNumber(v reflect.Value) (types.AttributeValue, error) { - if av, err := tryMarshaler(v); err != nil { - return nil, err - } else if av != nil { - return av, nil - } var out string switch v.Kind() { @@ -742,11 +737,6 @@ func (e *Encoder) encodeNumber(v reflect.Value) (types.AttributeValue, error) { } func (e *Encoder) encodeString(v reflect.Value) (types.AttributeValue, error) { - if av, err := tryMarshaler(v); err != nil { - return nil, err - } else if av != nil { - return av, nil - } switch v.Kind() { case reflect.String: diff --git a/feature/dynamodb/attributevalue/encode_test.go b/feature/dynamodb/attributevalue/encode_test.go index e536a42030d..342550a6c03 100644 --- a/feature/dynamodb/attributevalue/encode_test.go +++ b/feature/dynamodb/attributevalue/encode_test.go @@ -83,6 +83,97 @@ func TestMarshalMashaler(t *testing.T) { } } +type customBoolStringMarshaler string + +func (m customBoolStringMarshaler) MarshalDynamoDBAttributeValue() (types.AttributeValue, error) { + + if b, err := strconv.ParseBool(string(m)); err == nil { + return &types.AttributeValueMemberBOOL{Value: b}, nil + } + + return &types.AttributeValueMemberS{Value: string(m)}, nil +} + +func TestCustomStringMarshaler(t *testing.T) { + cases := []struct { + expected types.AttributeValue + input string + }{ + { + expected: &types.AttributeValueMemberBOOL{Value: false}, + input: "false", + }, + { + expected: &types.AttributeValueMemberBOOL{Value: true}, + input: "true", + }, + { + expected: &types.AttributeValueMemberS{Value: "ABC"}, + input: "ABC", + }, + } + + for _, testCase := range cases { + input := customBoolStringMarshaler(testCase.input) + actual, err := Marshal(input) + if err != nil { + t.Errorf("got unexpected error %v for input %v", err, testCase.input) + } + if diff := cmpDiff(testCase.expected, actual); len(diff) != 0 { + t.Errorf("expected match but got:%s", diff) + } + } +} + +type customGradeMarshaler uint + +func (m customGradeMarshaler) MarshalDynamoDBAttributeValue() (types.AttributeValue, error) { + if int(m) > 100 { + return nil, fmt.Errorf("grade cant be larger then 100") + } + return &types.AttributeValueMemberN{Value: strconv.FormatUint(uint64(m), 10)}, nil +} + +func TestCustomNumberMarshaler(t *testing.T) { + cases := []struct { + expectedErr bool + input uint + expected types.AttributeValue + }{ + { + expectedErr: false, + input: 50, + expected: &types.AttributeValueMemberN{Value: "50"}, + }, + { + expectedErr: false, + input: 90, + expected: &types.AttributeValueMemberN{Value: "90"}, + }, + { + expectedErr: true, + input: 150, + expected: nil, + }, + } + + for _, testCase := range cases { + input := customGradeMarshaler(testCase.input) + actual, err := Marshal(customGradeMarshaler(input)) + if testCase.expectedErr && err == nil { + t.Errorf("expected error but got nil for input %v", testCase.input) + continue + } + if !testCase.expectedErr && err != nil { + t.Errorf("got unexpected error %v for input %v", err, testCase.input) + continue + } + if diff := cmpDiff(testCase.expected, actual); len(diff) != 0 { + t.Errorf("expected match but got:%s", diff) + } + } +} + type testOmitEmptyElemListStruct struct { Values []string `dynamodbav:",omitemptyelem"` } diff --git a/feature/dynamodb/attributevalue/marshaler_test.go b/feature/dynamodb/attributevalue/marshaler_test.go index 93630415a5b..9c324632994 100644 --- a/feature/dynamodb/attributevalue/marshaler_test.go +++ b/feature/dynamodb/attributevalue/marshaler_test.go @@ -546,6 +546,60 @@ func BenchmarkMarshalOneMember(b *testing.B) { }) } +func BenchmarkList20Ints(b *testing.B) { + input := []int{} + for i := 0; i < 20; i++ { + input = append(input, i) + } + + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _, err := Marshal(input) + if err != nil { + b.Fatal(err) + } + } + }) +} + +func BenchmarkStruct10Fields(b *testing.B) { + + type struct10Fields struct { + Field1 int + Field2 string + Field3 int + Field4 string + Field5 string + Field6 string + Field7 int + Field8 string + Field9 int + Field10 int + } + + input := struct10Fields{ + Field1: 10, + Field2: "ASD", + Field3: 70, + Field4: "qqqqq", + Field5: "AAA", + Field6: "bbb", + Field7: 63, + Field8: "aa", + Field9: 10, + Field10: 63, + } + + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _, err := Marshal(input) + if err != nil { + b.Fatal(err) + } + } + }) +} + func BenchmarkMarshalTwoMembers(b *testing.B) { fieldCache = &fieldCacher{} From 544f25e1c372e33dcd32bcf1eb46efeaa8e745db Mon Sep 17 00:00:00 2001 From: Luc Talatinian Date: Fri, 22 Mar 2024 14:15:16 -0400 Subject: [PATCH 2/2] add changelog --- .changelog/b35ddd7e9ee644818481e7729b0f749d.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changelog/b35ddd7e9ee644818481e7729b0f749d.json diff --git a/.changelog/b35ddd7e9ee644818481e7729b0f749d.json b/.changelog/b35ddd7e9ee644818481e7729b0f749d.json new file mode 100644 index 00000000000..de492164271 --- /dev/null +++ b/.changelog/b35ddd7e9ee644818481e7729b0f749d.json @@ -0,0 +1,8 @@ +{ + "id": "b35ddd7e-9ee6-4481-8481-e7729b0f749d", + "type": "bugfix", + "description": "Removes some duplicated reflection-based calls in the marshaler.", + "modules": [ + "feature/dynamodb/attributevalue" + ] +}