fishy commented on pull request #2307:
URL: https://github.com/apache/thrift/pull/2307#issuecomment-764147874
I believe currently there's some bug in your implementation. The result is
different from `reflect.DeepEqual`, Here's the code I used:
`test.thrift`:
```thrift
namespace go test
enum Enum {
One = 1,
Two = 2,
}
struct Struct {
1: optional string Str,
2: optional i32 I32,
3: optional set<Enum> Enums,
}
```
`bench_test.go`:
```go
package thriftsetbench
import (
"reflect"
"testing"
"github.com/apache/thrift/lib/go/thrift"
"github.com/fishy/thriftsetbench/gen-go/test"
)
func BenchmarkEqual(b *testing.B) {
equalGetter := func() (*test.Struct, *test.Struct) {
s1 := &test.Struct{
Str: thrift.StringPtr("foobar"),
Enums: []test.Enum{
test.Enum_One,
test.Enum_Two,
},
}
s2 := &test.Struct{
Str: thrift.StringPtr("foobar"),
Enums: []test.Enum{
test.Enum_One,
test.Enum_Two,
},
}
return s1, s2
}
nonEqualGetter := func() (*test.Struct, *test.Struct) {
s1 := &test.Struct{
Str: thrift.StringPtr("foobar"),
Enums: []test.Enum{
test.Enum_One,
},
}
s2 := &test.Struct{
Str: thrift.StringPtr("foobar"),
Enums: []test.Enum{
test.Enum_Two,
},
}
return s1, s2
}
for label, getter := range map[string]func() (*test.Struct,
*test.Struct){
"equal": equalGetter,
"nonequal": nonEqualGetter,
} {
b.Run(label, func(b *testing.B) {
s1, s2 := getter()
reflectResult := reflect.DeepEqual(s1, s2)
equalsResult := s1.Equals(s2)
if reflectResult != equalsResult {
b.Errorf("Reflect result: %v, Equals result:
%v", reflectResult, equalsResult)
}
b.Run("reflect", func(b *testing.B) {
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
reflect.DeepEqual(s1, s2)
}
})
})
b.Run("equals", func(b *testing.B) {
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
s1.Equals(s2)
}
})
})
})
}
}
```
result:
```
$ go test -bench . -benchmem
goos: linux
goarch: amd64
pkg: github.com/fishy/thriftsetbench
BenchmarkEqual/equal/reflect-12 10597189 113 ns/op
64 B/op 6 allocs/op
BenchmarkEqual/equal/equals-12 1000000000 0.680 ns/op
0 B/op 0 allocs/op
BenchmarkEqual/nonequal/reflect-12 12201334 99.9
ns/op 48 B/op 4 allocs/op
BenchmarkEqual/nonequal/equals-12 1000000000
0.712 ns/op 0 B/op 0 allocs/op
--- FAIL: BenchmarkEqual/nonequal
bench_test.go:54: Reflect result: false, Equals result: true
--- FAIL: BenchmarkEqual
FAIL
exit status 1
FAIL github.com/fishy/thriftsetbench 4.183s
```
We likely would need to have some thorough test for the `Equals`
implementation with this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
