Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move generic decoding to codec level #336

Merged
merged 2 commits into from
Dec 21, 2023

Conversation

redaLaanait
Copy link
Contributor

This change will make it easier to keep evolving decoders without the need to duplicate a similar change at reader_generic.

@redaLaanait
Copy link
Contributor Author

genericDecode

goos: darwin
goarch: amd64
pkg: github.com/hamba/avro/v2
cpu: Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
BenchmarkDecoder_Interface/Empty_Interface-8               40282         29478 ns/op       16581 B/op        262 allocs/op
BenchmarkDecoder_Interface/Interface_Non-Ptr-8             39390         29453 ns/op       16581 B/op        262 allocs/op
BenchmarkDecoder_Interface/Interface_Nil_Ptr-8             42998         27922 ns/op       16021 B/op        250 allocs/op
BenchmarkDecoder_Interface/Interface_Ptr-8                 43202         28280 ns/op       16021 B/op        250 allocs/op


ReadNext

goos: darwin
goarch: amd64
pkg: github.com/hamba/avro/v2
cpu: Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
BenchmarkDecoder_Interface/Empty_Interface-8               37956         28762 ns/op       16373 B/op        253 allocs/op
BenchmarkDecoder_Interface/Interface_Non-Ptr-8             42888         27599 ns/op       16373 B/op        253 allocs/op
BenchmarkDecoder_Interface/Interface_Nil_Ptr-8             42120         27684 ns/op       16021 B/op        250 allocs/op
BenchmarkDecoder_Interface/Interface_Ptr-8                 42163         28508 ns/op       16021 B/op        250 allocs/op


Benchmark code

func BenchmarkDecoder_Interface(b *testing.B) {
	tests := []struct {
		name   string
		data   []byte
		schema string
		got    any
		want   any
	}{
		{
			name:   "Empty Interface",
			data:   []byte{0x36, 0x06, 0x66, 0x6f, 0x6f},
			schema: `{"type": "record", "name": "test", "fields" : [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}`,
			got:    nil,
			want:   map[string]any{"a": int64(27), "b": "foo"},
		},
		{
			name:   "Interface Non-Ptr",
			data:   []byte{0x36, 0x06, 0x66, 0x6f, 0x6f},
			schema: `{"type": "record", "name": "test", "fields": [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}`,
			got:    TestRecord{},
			want:   map[string]any{"a": int64(27), "b": "foo"},
		},
		{
			name:   "Interface Nil Ptr",
			data:   []byte{0x36, 0x06, 0x66, 0x6f, 0x6f},
			schema: `{"type": "record", "name": "test", "fields" : [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}`,
			got:    (*TestRecord)(nil),
			want:   &TestRecord{A: 27, B: "foo"},
		},
		{
			name:   "Interface Ptr",
			data:   []byte{0x36, 0x06, 0x66, 0x6f, 0x6f},
			schema: `{"type": "record", "name": "test", "fields": [{"name": "a", "type": "long"}, {"name": "b", "type": "string"}]}`,
			got:    &TestRecord{},
			want:   &TestRecord{A: 27, B: "foo"},
		},
	}

	for _, test := range tests {
		test := test
		b.Run(test.name, func(b *testing.B) {
			defer ConfigTeardown()
			b.ResetTimer()

			for n := 0; n < b.N; n++ {
				dec, _ := avro.NewDecoder(test.schema, bytes.NewReader(test.data))
				_ = dec.Decode(&test.got)
			}
		})
	}
}

Copy link
Member

@nrwiersma nrwiersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one minor comment.

codec_dynamic.go Outdated
@@ -15,13 +15,15 @@ func (d *efaceDecoder) Decode(ptr unsafe.Pointer, r *Reader) {
pObj := (*any)(ptr)
obj := *pObj
if obj == nil {
*pObj = r.ReadNext(d.schema)
// *pObj = r.ReadNext(d.schema)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the commented out code.

@nrwiersma nrwiersma merged commit 35f90ee into hamba:main Dec 21, 2023
2 of 3 checks passed
RyougiNevermore added a commit to RyougiNevermore/avro that referenced this pull request Dec 21, 2023
* origin/main:
  feat: move generic decoding to codec level (hamba#336)
  fix: linter issues

# Conflicts:
#	config.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants