Skip to content

Commit

Permalink
Use concrete types for path steps
Browse files Browse the repository at this point in the history
Rather than representing each path step as an interface,
use concrete types instead. This provides some performance benefits as
it reduces the amount of virtual function calls and also provides the
ability for the compiler to inline method calls.

This is technically a breaking change, but since each of the path step
interfaces were explicitly implemented in such a way that they couldn't
be implemented directly (due to the presence of an unexported method),
the only way someone could have been depending on these as interfaces is
if they embedded the interface into another interface. Static analysis of
all code at Google and publicly available on GitHub shows that this is
not a problem.

The performance benefits of this change is significant:

benchmark                               old ns/op     new ns/op     delta
BenchmarkBytes/64KiB/EqualFilter0-4     80551394      46592605      -42.16%
BenchmarkBytes/64KiB/EqualFilter1-4     102922132     69974509      -32.01%
BenchmarkBytes/64KiB/EqualFilter2-4     159009935     94474812      -40.59%
BenchmarkBytes/64KiB/EqualFilter3-4     181231264     124601102     -31.25%
BenchmarkBytes/64KiB/EqualFilter4-4     189775228     148864070     -21.56%
BenchmarkBytes/64KiB/EqualFilter5-4     285065469     175198907     -38.54%

benchmark                               old MB/s     new MB/s     speedup
BenchmarkBytes/64KiB/EqualFilter0-4     1.63         2.81         1.72x
BenchmarkBytes/64KiB/EqualFilter1-4     1.27         1.87         1.47x
BenchmarkBytes/64KiB/EqualFilter2-4     0.82         1.39         1.70x
BenchmarkBytes/64KiB/EqualFilter3-4     0.72         1.05         1.46x
BenchmarkBytes/64KiB/EqualFilter4-4     0.69         0.88         1.28x
BenchmarkBytes/64KiB/EqualFilter5-4     0.46         0.75         1.63x

benchmark                               old allocs     new allocs     delta
BenchmarkBytes/64KiB/EqualFilter0-4     133            134            +0.75%
BenchmarkBytes/64KiB/EqualFilter1-4     134            134            +0.00%
BenchmarkBytes/64KiB/EqualFilter2-4     135            135            +0.00%
BenchmarkBytes/64KiB/EqualFilter3-4     135            135            +0.00%
BenchmarkBytes/64KiB/EqualFilter4-4     136            136            +0.00%
BenchmarkBytes/64KiB/EqualFilter5-4     136            136            +0.00%

benchmark                               old bytes     new bytes     delta
BenchmarkBytes/64KiB/EqualFilter0-4     6632417       6632523       +0.00%
BenchmarkBytes/64KiB/EqualFilter1-4     6632416       6632464       +0.00%
BenchmarkBytes/64KiB/EqualFilter2-4     6632464       6632507       +0.00%
BenchmarkBytes/64KiB/EqualFilter3-4     6632502       6632483       -0.00%
BenchmarkBytes/64KiB/EqualFilter4-4     6632652       6632668       +0.00%
BenchmarkBytes/64KiB/EqualFilter5-4     6632604       6632659       +0.00%
  • Loading branch information
dsnet committed Mar 1, 2019
1 parent c812816 commit f9489fa
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 186 deletions.
14 changes: 7 additions & 7 deletions cmp/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func (s *state) compareAny(step PathStep) {
return
}
vx, vy = vx.Elem(), vy.Elem()
s.compareAny(&indirect{pathStep{t.Elem(), vx, vy}})
s.compareAny(Indirect{&indirect{pathStep{t.Elem(), vx, vy}}})
return
case reflect.Interface:
if vx.IsNil() || vy.IsNil() {
Expand All @@ -286,7 +286,7 @@ func (s *state) compareAny(step PathStep) {
s.report(false, 0)
return
}
s.compareAny(&typeAssertion{pathStep{vx.Type(), vx, vy}})
s.compareAny(TypeAssertion{&typeAssertion{pathStep{vx.Type(), vx, vy}}})
return
default:
panic(fmt.Sprintf("%v kind not handled", t.Kind()))
Expand Down Expand Up @@ -314,7 +314,7 @@ func (s *state) tryMethod(t reflect.Type, vx, vy reflect.Value) bool {
return true
}

func (s *state) callTRFunc(f, v reflect.Value, step *transform) reflect.Value {
func (s *state) callTRFunc(f, v reflect.Value, step Transform) reflect.Value {
v = sanitizeValue(v, f.Type().In(0))
if !s.dynChecker.Next() {
return f.Call([]reflect.Value{v})[0]
Expand Down Expand Up @@ -384,7 +384,7 @@ func sanitizeValue(v reflect.Value, t reflect.Type) reflect.Value {
func (s *state) compareStruct(t reflect.Type, vx, vy reflect.Value) {
var vax, vay reflect.Value // Addressable versions of vx and vy

step := &structField{}
step := StructField{&structField{}}
for i := 0; i < t.NumField(); i++ {
step.typ = t.Field(i).Type
step.vx = vx.Field(i)
Expand Down Expand Up @@ -415,8 +415,8 @@ func (s *state) compareStruct(t reflect.Type, vx, vy reflect.Value) {
}

func (s *state) compareSlice(t reflect.Type, vx, vy reflect.Value) {
step := &sliceIndex{pathStep: pathStep{typ: t.Elem()}}
withIndexes := func(ix, iy int) *sliceIndex {
step := SliceIndex{&sliceIndex{pathStep: pathStep{typ: t.Elem()}}}
withIndexes := func(ix, iy int) SliceIndex {
if ix >= 0 {
step.vx, step.xkey = vx.Index(ix), ix
} else {
Expand Down Expand Up @@ -495,7 +495,7 @@ func (s *state) compareMap(t reflect.Type, vx, vy reflect.Value) {

// We combine and sort the two map keys so that we can perform the
// comparisons in a deterministic order.
step := &mapIndex{pathStep: pathStep{typ: t.Elem()}}
step := MapIndex{&mapIndex{pathStep: pathStep{typ: t.Elem()}}}
for _, k := range value.SortKeys(append(vx.MapKeys(), vy.MapKeys()...)) {
step.vx = vx.MapIndex(k)
step.vy = vy.MapIndex(k)
Expand Down
4 changes: 2 additions & 2 deletions cmp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func (tr *transformer) isFiltered() bool { return tr.typ != nil }

func (tr *transformer) filter(s *state, t reflect.Type, _, _ reflect.Value) applicableOption {
for i := len(s.curPath) - 1; i >= 0; i-- {
if t, ok := s.curPath[i].(*transform); !ok {
if t, ok := s.curPath[i].(Transform); !ok {
break // Hit most recent non-Transform step
} else if tr == t.trans {
return nil // Cannot directly use same Transform
Expand All @@ -301,7 +301,7 @@ func (tr *transformer) filter(s *state, t reflect.Type, _, _ reflect.Value) appl
}

func (tr *transformer) apply(s *state, vx, vy reflect.Value) {
step := &transform{pathStep{typ: tr.fnc.Type().Out(0)}, tr}
step := Transform{&transform{pathStep{typ: tr.fnc.Type().Out(0)}, tr}}
vvx := s.callTRFunc(tr.fnc, vx, step)
vvy := s.callTRFunc(tr.fnc, vy, step)
step.vx, step.vy = vvx, vvy
Expand Down
Loading

0 comments on commit f9489fa

Please sign in to comment.