Skip to content

Commit

Permalink
Optimisation of diff performace (#277)
Browse files Browse the repository at this point in the history
* optimize performace of diff

* Suggested changes

* Added a Benchmarktest for ygot.Diff

* changed errorhandling and import path

* resolve syntax errors
  • Loading branch information
idefixcert authored and robshakir committed Apr 27, 2019
1 parent 5fe05c9 commit 68346f9
Show file tree
Hide file tree
Showing 4 changed files with 9,141 additions and 17 deletions.
38 changes: 21 additions & 17 deletions ygot/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package ygot
import (
"fmt"
"reflect"
"sort"
"strings"

"github.com/golang/protobuf/proto"
"github.com/kylelemons/godebug/pretty"
Expand Down Expand Up @@ -210,6 +212,7 @@ func getPathSpec(ni *util.NodeInfo) (*pathSpec, error) {
// the walk.
func findSetLeaves(s GoStruct, opts ...DiffOpt) (map[*pathSpec]interface{}, error) {
pathOpt := hasDiffPathOpt(opts)
processedPaths := map[string]bool{}

findSetIterFunc := func(ni *util.NodeInfo, in, out interface{}) (errs util.Errors) {
if reflect.DeepEqual(ni.StructField, reflect.StructField{}) {
Expand Down Expand Up @@ -242,6 +245,23 @@ func findSetLeaves(s GoStruct, opts ...DiffOpt) (map[*pathSpec]interface{}, erro
if err != nil {
return util.NewErrs(err)
}

//prevent duplicate key
keys := make([]string, len(vp.gNMIPaths))
for i, paths := range vp.gNMIPaths {
s, err := PathToString(paths)
if err != nil {
return util.NewErrs(err)
}
keys[i] = s
}
sort.Strings(keys)
key := strings.Join(keys, "/")
if _, ok := processedPaths[key]; ok == true {
return
}
processedPaths[key] = true

ni.Annotation = []interface{}{vp}

if util.IsNilOrInvalidValue(ni.FieldValue) || util.IsValueStructPtr(ni.FieldValue) || util.IsValueMap(ni.FieldValue) {
Expand Down Expand Up @@ -269,23 +289,7 @@ func findSetLeaves(s GoStruct, opts ...DiffOpt) (map[*pathSpec]interface{}, erro
return nil, fmt.Errorf("error from ForEachDataField iteration: %v", errs)
}

uOut := map[*pathSpec]interface{}{}
// Deduplicate the list, since the iteration function will be called
// multiple times for path tags that have >1 element.
for ok, ov := range out {
var skip bool
for uk := range uOut {
if ok.Equal(uk) {
// This is a duplicate path, so we do not need to append it to the list.
skip = true
}
}
if !skip {
uOut[ok] = ov
}
}

return uOut, nil
return out, nil
}

// hasDiffPathOpt extracts a DiffPathOpt from the opts slice provided. In
Expand Down
42 changes: 42 additions & 0 deletions ytypes/schema_tests/diff_benchmark_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package validate

import (
"io/ioutil"
"path/filepath"
"testing"

oc "github.com/openconfig/ygot/exampleoc"
"github.com/openconfig/ygot/ygot"
)

func BenchmarkDiff(b *testing.B) {
jsonFileA := "interfaceBenchmarkA.json"
jsonFileB := "interfaceBenchmarkB.json"

jsonA, err := ioutil.ReadFile(filepath.Join(testRoot, "testdata", jsonFileA))
if err != nil {
b.Errorf("ioutil.ReadFile(%s): could not open file: %v", jsonFileA, err)
return
}
deviceA := &oc.Device{}
if err := oc.Unmarshal(jsonA, deviceA); err != nil {
b.Errorf("ioutil.ReadFile(%s): could unmarschal: %v", jsonFileA, err)
return
}

jsonB, err := ioutil.ReadFile(filepath.Join(testRoot, "testdata", jsonFileB))
if err != nil {
b.Errorf("ioutil.ReadFile(%s): could not open file: %v", jsonFileB, err)
return
}
deviceB := &oc.Device{}
if err := oc.Unmarshal(jsonB, deviceB); err != nil {
b.Errorf("ioutil.ReadFile(%s): could unmarschal: %v", jsonFileB, err)
return
}
_, err = ygot.Diff(deviceA, deviceB)
if err != nil {
b.Errorf("Error in diff %v", err)
return
}
}
Loading

0 comments on commit 68346f9

Please sign in to comment.