From d443dc83040f59072b86f0d5530db5d40057f18d Mon Sep 17 00:00:00 2001 From: Jay Zhu Date: Tue, 6 Dec 2022 18:38:19 -0700 Subject: [PATCH 1/5] perf(logging): prevent unnecessary processing for debug logging The inline processing (while calling debug logging functions) can consume significant amount of CPU resources at scale even when debug logging is disabled. This commit prevents such unnecessary processing. This commit also includes performance improvements from reducing expensive string split calls. --- util/debug.go | 17 +++++- util/path.go | 42 +++++++------ util/reflect.go | 130 ++++++++++++++++++++++++++++++---------- util/yang.go | 2 +- ygot/render.go | 23 ++++++- ytypes/choice.go | 5 +- ytypes/common.go | 6 +- ytypes/container.go | 21 +++++-- ytypes/int_type.go | 8 ++- ytypes/int_type_test.go | 48 +++++++-------- ytypes/leaf.go | 67 ++++++++++++++++----- ytypes/leaf_list.go | 12 +++- ytypes/leafref.go | 53 ++++++++++++---- ytypes/list.go | 30 ++++++++-- ytypes/node.go | 6 +- ytypes/unmarshal.go | 11 +++- ytypes/util_schema.go | 25 +++++--- ytypes/util_types.go | 43 ++++++++++--- ytypes/validate.go | 4 +- 19 files changed, 405 insertions(+), 148 deletions(-) diff --git a/util/debug.go b/util/debug.go index cb5929b21..e2322e6c8 100644 --- a/util/debug.go +++ b/util/debug.go @@ -40,6 +40,18 @@ var ( maxValueStrLen = 150 ) +// DebugLibraryEnabled returns the value of debugLibrary. +// This function is used to prevent unnecessary inline processing of the DbgPrint calls. +func DebugLibraryEnabled() bool { + return debugLibrary +} + +// DebugSchemaEnabled returns the value of debugSchema. +// This function is used to prevent unnecessary inline processing of the DbgSchema calls. +func DebugSchemaEnabled() bool { + return debugSchema +} + // DbgPrint prints v if the package global variable debugLibrary is set. // v has the same format as Printf. A trailing newline is added to the output. func DbgPrint(v ...interface{}) { @@ -63,7 +75,10 @@ func DbgSchema(v ...interface{}) { // DbgErr DbgPrints err and returns it. func DbgErr(err error) error { - DbgPrint("ERR: " + err.Error()) + if debugLibrary { + DbgPrint("ERR: " + err.Error()) + } + return err } diff --git a/util/path.go b/util/path.go index 11616f260..1ddc5d362 100644 --- a/util/path.go +++ b/util/path.go @@ -25,15 +25,20 @@ import ( // SchemaPaths returns all the paths in the path tag. func SchemaPaths(f reflect.StructField) ([][]string, error) { - var out [][]string pathTag, ok := f.Tag.Lookup("path") if !ok || pathTag == "" { return nil, fmt.Errorf("field %s did not specify a path", f.Name) } + // Early return for performance. + if strings.Index(pathTag, "|") == -1 { + return [][]string{stripModulePrefixes(strings.Split(pathTag, "/"))}, nil + } + ps := strings.Split(pathTag, "|") - for _, p := range ps { - out = append(out, stripModulePrefixes(strings.Split(p, "/"))) + out := make([][]string, len(ps)) + for i, p := range ps { + out[i] = stripModulePrefixes(strings.Split(p, "/")) } return out, nil } @@ -41,15 +46,15 @@ func SchemaPaths(f reflect.StructField) ([][]string, error) { // ShadowSchemaPaths returns all the paths in the shadow-path tag. If the tag // doesn't exist, a nil slice is returned. func ShadowSchemaPaths(f reflect.StructField) [][]string { - var out [][]string pathTag, ok := f.Tag.Lookup("shadow-path") if !ok || pathTag == "" { return nil } ps := strings.Split(pathTag, "|") - for _, p := range ps { - out = append(out, stripModulePrefixes(strings.Split(p, "/"))) + out := make([][]string, len(ps)) + for i, p := range ps { + out[i] = stripModulePrefixes(strings.Split(p, "/")) } return out } @@ -126,11 +131,12 @@ func relativeSchemaPath(f reflect.StructField, preferShadowPath bool) ([]string, return nil, fmt.Errorf("field %s did not specify a shadow-path", f.Name) } - paths := strings.Split(pathTag, "|") - if len(paths) == 1 { + if strings.Index(pathTag, "|") == -1 { pathTag = strings.TrimPrefix(pathTag, "/") return strings.Split(pathTag, "/"), nil } + + paths := strings.Split(pathTag, "|") for _, pv := range paths { pv = strings.TrimPrefix(pv, "/") pe := strings.Split(pv, "/") @@ -314,9 +320,9 @@ func StripModulePrefixesStr(in string) string { // stripModulePrefixes returns "in" with each element with the format "A:B" // changed to "B". func stripModulePrefixes(in []string) []string { - var out []string - for _, v := range in { - out = append(out, StripModulePrefix(v)) + out := make([]string, len(in)) + for i, v := range in { + out[i] = StripModulePrefix(v) } return out } @@ -341,15 +347,13 @@ func stripModulePrefixWithCheck(name string) (string, error) { // removing foo from "foo:bar". Such qualified paths are used in YANG modules // where remote paths are referenced. func StripModulePrefix(name string) string { - ps := strings.Split(name, ":") - switch len(ps) { - case 1: - return name - case 2: - return ps[1] - default: - return name + for i, r := range name { + if r == ':' && len(name) > i+1 { + return name[i+1:] + } } + + return name } // ReplacePathSuffix replaces the non-prefix part of a prefixed path name, or diff --git a/util/reflect.go b/util/reflect.go index 3ad849386..2ba1a37fc 100644 --- a/util/reflect.go +++ b/util/reflect.go @@ -172,7 +172,9 @@ func IsStructValueWithNFields(v reflect.Value, n int) bool { // InsertIntoSlice inserts value into parent which must be a slice ptr. func InsertIntoSlice(parentSlice interface{}, value interface{}) error { - DbgPrint("InsertIntoSlice into parent type %T with value %v, type %T", parentSlice, ValueStrDebug(value), value) + if DebugLibraryEnabled() { + DbgPrint("InsertIntoSlice into parent type %T with value %v, type %T", parentSlice, ValueStrDebug(value), value) + } pv := reflect.ValueOf(parentSlice) t := reflect.TypeOf(parentSlice) @@ -183,15 +185,19 @@ func InsertIntoSlice(parentSlice interface{}, value interface{}) error { } pv.Elem().Set(reflect.Append(pv.Elem(), v)) - DbgPrint("new list: %v\n", pv.Elem().Interface()) + if DebugLibraryEnabled() { + DbgPrint("new list: %v\n", pv.Elem().Interface()) + } return nil } // InsertIntoMap inserts value with key into parent which must be a map. func InsertIntoMap(parentMap interface{}, key interface{}, value interface{}) error { - DbgPrint("InsertIntoMap into parent type %T with key %v(%T) value \n%s\n (%T)", - parentMap, ValueStrDebug(key), key, pretty.Sprint(value), value) + if DebugLibraryEnabled() { + DbgPrint("InsertIntoMap into parent type %T with key %v(%T) value \n%s\n (%T)", + parentMap, ValueStrDebug(key), key, pretty.Sprint(value), value) + } v := reflect.ValueOf(parentMap) t := reflect.TypeOf(parentMap) @@ -211,7 +217,9 @@ func InsertIntoMap(parentMap interface{}, key interface{}, value interface{}) er // nil) in parentStruct, with value fieldValue. If the field is a slice, // fieldValue is appended. func UpdateField(parentStruct interface{}, fieldName string, fieldValue interface{}) error { - DbgPrint("UpdateField field %s of parent type %T with value %v", fieldName, parentStruct, ValueStrDebug(fieldValue)) + if DebugLibraryEnabled() { + DbgPrint("UpdateField field %s of parent type %T with value %v", fieldName, parentStruct, ValueStrDebug(fieldValue)) + } if IsValueNil(parentStruct) { return fmt.Errorf("parent is nil in UpdateField for field %s", fieldName) @@ -239,7 +247,9 @@ func UpdateField(parentStruct interface{}, fieldName string, fieldValue interfac // If the struct field type is a ptr and the value is non-ptr, the field is // populated with the corresponding ptr type. func InsertIntoStruct(parentStruct interface{}, fieldName string, fieldValue interface{}) error { - DbgPrint("InsertIntoStruct field %s of parent type %T with value %v", fieldName, parentStruct, ValueStrDebug(fieldValue)) + if DebugLibraryEnabled() { + DbgPrint("InsertIntoStruct field %s of parent type %T with value %v", fieldName, parentStruct, ValueStrDebug(fieldValue)) + } v, t := reflect.ValueOf(fieldValue), reflect.TypeOf(fieldValue) pv, pt := reflect.ValueOf(parentStruct), reflect.TypeOf(parentStruct) @@ -294,7 +304,9 @@ func InsertIntoStruct(parentStruct interface{}, fieldName string, fieldValue int // InsertIntoSliceStructField inserts fieldValue into a field of type slice in // parentStruct called fieldName (which must exist, but may be nil). func InsertIntoSliceStructField(parentStruct interface{}, fieldName string, fieldValue interface{}) error { - DbgPrint("InsertIntoSliceStructField field %s of parent type %T with value %v", fieldName, parentStruct, ValueStrDebug(fieldValue)) + if DebugLibraryEnabled() { + DbgPrint("InsertIntoSliceStructField field %s of parent type %T with value %v", fieldName, parentStruct, ValueStrDebug(fieldValue)) + } v, t := reflect.ValueOf(fieldValue), reflect.TypeOf(fieldValue) pv, pt := reflect.ValueOf(parentStruct), reflect.TypeOf(parentStruct) @@ -334,7 +346,9 @@ func InsertIntoSliceStructField(parentStruct interface{}, fieldName string, fiel // given key. If the key already exists in the map, the corresponding value is // updated. func InsertIntoMapStructField(parentStruct interface{}, fieldName string, key, fieldValue interface{}) error { - DbgPrint("InsertIntoMapStructField field %s of parent type %T with key %v, value %v", fieldName, parentStruct, key, ValueStrDebug(fieldValue)) + if DebugLibraryEnabled() { + DbgPrint("InsertIntoMapStructField field %s of parent type %T with key %v, value %v", fieldName, parentStruct, key, ValueStrDebug(fieldValue)) + } v := reflect.ValueOf(parentStruct) t := reflect.TypeOf(parentStruct) @@ -482,7 +496,11 @@ func ChildSchemaPreferShadow(schema *yang.Entry, f reflect.StructField) (*yang.E func childSchema(schema *yang.Entry, f reflect.StructField, preferShadowPath bool) (*yang.Entry, error) { pathTag, _ := f.Tag.Lookup("path") shadowPathTag, _ := f.Tag.Lookup("shadow-path") - DbgSchema("childSchema for schema %s, field %s, path tag %s, shadow-path tag\n", schema.Name, f.Name, pathTag, shadowPathTag) + + if DebugSchemaEnabled() { + DbgSchema("childSchema for schema %s, field %s, path tag %s, shadow-path tag\n", schema.Name, f.Name, pathTag, shadowPathTag) + } + p, err := relativeSchemaPath(f, preferShadowPath) if err != nil { return nil, err @@ -495,14 +513,24 @@ func childSchema(schema *yang.Entry, f reflect.StructField, preferShadowPath boo if schema.IsContainer() && len(p) > 1 && p[0] == schema.Name { p = p[1:] } - DbgSchema("RelativeSchemaPath yields %v\n", p) + + if DebugSchemaEnabled() { + DbgSchema("RelativeSchemaPath yields %v\n", p) + } + // For empty path, return the parent schema. childSchema := schema foundSchema := true // Traverse the returned schema path to get the child schema. - DbgSchema("traversing schema Dirs...") + if DebugSchemaEnabled() { + DbgSchema("traversing schema Dirs...") + } + for ; len(p) > 0; p = p[1:] { - DbgSchema("/%s", p[0]) + if DebugSchemaEnabled() { + DbgSchema("/%s", p[0]) + } + p := StripModulePrefix(p[0]) ns, ok := childSchema.Dir[p] if !ok { @@ -512,10 +540,15 @@ func childSchema(schema *yang.Entry, f reflect.StructField, preferShadowPath boo childSchema = ns } if foundSchema { - DbgSchema(" - found\n") + if DebugSchemaEnabled() { + DbgSchema(" - found\n") + } return childSchema, nil } - DbgSchema(" - not found\n") + + if DebugSchemaEnabled() { + DbgSchema(" - not found\n") + } // Path is not null and was not found in the schema. It could be inside a // choice/case schema element which is not represented in the path tags. @@ -532,27 +565,37 @@ func childSchema(schema *yang.Entry, f reflect.StructField, preferShadowPath boo } entries := FindFirstNonChoiceOrCase(schema) - DbgSchema("checking for %s against non choice/case entries: %v\n", p[0], stringMapKeys(entries)) + if DebugSchemaEnabled() { + DbgSchema("checking for %s against non choice/case entries: %v\n", p[0], stringMapKeys(entries)) + } for path, entry := range entries { splitPath := SplitPath(path) name := splitPath[len(splitPath)-1] - DbgSchema("%s ? ", name) + if DebugSchemaEnabled() { + DbgSchema("%s ? ", name) + } if StripModulePrefix(name) == p[0] { - DbgSchema(" - match\n") + if DebugSchemaEnabled() { + DbgSchema(" - match\n") + } return entry, nil } } - DbgSchema(" - no matches\n") + if DebugSchemaEnabled() { + DbgSchema(" - no matches\n") + } return nil, nil } // stringMapKeys returns the keys for map m. func stringMapKeys(m map[string]*yang.Entry) []string { - var out []string + out := make([]string, len(m)) + i := 0 for k := range m { - out = append(out, k) + out[i] = k + i++ } return out } @@ -702,7 +745,10 @@ func forEachFieldInternal(ni *NodeInfo, in, out interface{}, iterFunction FieldI nn.Schema = FirstChild(ni.Schema, p) if nn.Schema == nil { e := fmt.Errorf("forEachFieldInternal could not find child schema with path %v from schema name %s", p, ni.Schema.Name) - DbgPrint(e.Error()) + if DebugLibraryEnabled() { + DbgPrint(e.Error()) + } + log.Errorln(e) continue } @@ -938,8 +984,10 @@ func getNodesInternal(schema *yang.Entry, root interface{}, path *gpb.Path) ([]i path.Elem = path.GetElem()[1:] } - Indent() - DbgPrint("GetNode next path %v, value %v", path.GetElem()[0], ValueStrDebug(root)) + if DebugLibraryEnabled() { + Indent() + DbgPrint("GetNode next path %v, value %v", path.GetElem()[0], ValueStrDebug(root)) + } switch { case schema.IsContainer() || (schema.IsList() && IsTypeStructPtr(reflect.TypeOf(root))): @@ -958,7 +1006,9 @@ func getNodesInternal(schema *yang.Entry, root interface{}, path *gpb.Path) ([]i // type and matches each field against the first path element in path. If a // field matches, it recurses into that field with the remaining path. func getNodesContainer(schema *yang.Entry, root interface{}, path *gpb.Path) ([]interface{}, []*yang.Entry, error) { - DbgPrint("getNodesContainer: schema %s, next path %v, value %v", schema.Name, path.GetElem()[0], ValueStrDebug(root)) + if DebugLibraryEnabled() { + DbgPrint("getNodesContainer: schema %s, next path %v, value %v", schema.Name, path.GetElem()[0], ValueStrDebug(root)) + } rv := reflect.ValueOf(root) if !IsValueStructPtr(rv) { @@ -985,7 +1035,10 @@ func getNodesContainer(schema *yang.Entry, root interface{}, path *gpb.Path) ([] } ps, err := SchemaPaths(ft) - DbgPrint("check field name %s, paths %v", cschema.Name, ps) + if DebugLibraryEnabled() { + DbgPrint("check field name %s, paths %v", cschema.Name, ps) + } + if err != nil { return nil, nil, err } @@ -1010,7 +1063,9 @@ func getNodesContainer(schema *yang.Entry, root interface{}, path *gpb.Path) ([] // PathElem of the Path. If the key matches, it recurses into that field with // the remaining path. If empty key is specified, all list elements match. func getNodesList(schema *yang.Entry, root interface{}, path *gpb.Path) ([]interface{}, []*yang.Entry, error) { - DbgPrint("getNodesList: schema %s, next path %v, value %v", schema.Name, path.GetElem()[0], ValueStrDebug(root)) + if DebugLibraryEnabled() { + DbgPrint("getNodesList: schema %s, next path %v, value %v", schema.Name, path.GetElem()[0], ValueStrDebug(root)) + } rv := reflect.ValueOf(root) if schema.Key == "" { @@ -1022,7 +1077,9 @@ func getNodesList(schema *yang.Entry, root interface{}, path *gpb.Path) ([]inter } emptyKey := false if len(path.GetElem()[0].GetKey()) == 0 { - DbgPrint("path %v at %T points to list with empty wildcard key", path, root) + if DebugLibraryEnabled() { + DbgPrint("path %v at %T points to list with empty wildcard key", path, root) + } emptyKey = true } @@ -1035,7 +1092,10 @@ func getNodesList(schema *yang.Entry, root interface{}, path *gpb.Path) ([]inter // Iterate through all the map keys to see if any match the path. for _, k := range rv.MapKeys() { ev := rv.MapIndex(k) - DbgPrint("checking key %v, value %v", k.Interface(), ValueStrDebug(ev.Interface())) + if DebugLibraryEnabled() { + DbgPrint("checking key %v, value %v", k.Interface(), ValueStrDebug(ev.Interface())) + } + match := true if !emptyKey { // empty key matches everything. if !IsValueStruct(k) { @@ -1055,7 +1115,9 @@ func getNodesList(schema *yang.Entry, root interface{}, path *gpb.Path) ([]inter // off from the specification -- only that it works to uniquely identify // the key value. match = (fmt.Sprint(kv) == pathKey) - DbgPrint("check simple key value %s==%s ? %t", kv, pathKey, match) + if DebugLibraryEnabled() { + DbgPrint("check simple key value %s==%s ? %t", kv, pathKey, match) + } } else { // Must compare all the key fields. for i := 0; i < k.NumField(); i++ { @@ -1085,7 +1147,10 @@ func getNodesList(schema *yang.Entry, root interface{}, path *gpb.Path) ([]inter match = false break } - DbgPrint("key field value %s matches", pathKey) + + if DebugLibraryEnabled() { + DbgPrint("key field value %s matches", pathKey) + } } } } @@ -1093,7 +1158,10 @@ func getNodesList(schema *yang.Entry, root interface{}, path *gpb.Path) ([]inter if match { // Pass in the list schema, but the actual selected element // rather than the whole list. - DbgPrint("key matches") + if DebugLibraryEnabled() { + DbgPrint("key matches") + } + n, s, err := getNodesInternal(schema, ev.Interface(), PopGNMIPath(path)) if err != nil { return nil, nil, err diff --git a/util/yang.go b/util/yang.go index 5d026821c..844cceab3 100644 --- a/util/yang.go +++ b/util/yang.go @@ -576,7 +576,7 @@ func ResolveIfLeafRef(schema *yang.Entry) (*yang.Entry, error) { ykind = s.Type.Kind } - if s != orig { + if s != orig && DebugLibraryEnabled() { DbgPrint("follow schema leaf-ref from %s to %s, type %v", orig.Name, s.Name, s.Type.Kind) } return s, nil diff --git a/ygot/render.go b/ygot/render.go index 7f7a38385..2142bfd29 100644 --- a/ygot/render.go +++ b/ygot/render.go @@ -20,6 +20,7 @@ import ( "fmt" "reflect" "sort" + "strconv" "strings" "github.com/openconfig/gnmi/errlist" @@ -599,12 +600,28 @@ func KeyValueAsString(v interface{}) (string, error) { } switch kv.Kind() { - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - return fmt.Sprintf("%d", v), nil + case reflect.Int: + return strconv.Itoa(v.(int)), nil + case reflect.Int8: + return strconv.Itoa(int(v.(int8))), nil + case reflect.Int16: + return strconv.Itoa(int(v.(int16))), nil + case reflect.Int32: + return strconv.Itoa(int(v.(int32))), nil + case reflect.Uint: + return strconv.FormatUint(uint64(v.(uint)), 10), nil + case reflect.Uint8: + return strconv.FormatUint(uint64(v.(uint8)), 10), nil + case reflect.Uint16: + return strconv.FormatUint(uint64(v.(uint16)), 10), nil + case reflect.Uint32: + return strconv.FormatUint(uint64(v.(uint32)), 10), nil + case reflect.Uint64: + return strconv.FormatUint(v.(uint64), 10), nil case reflect.Float64: return fmt.Sprintf("%g", v), nil case reflect.String: - return fmt.Sprintf("%s", v), nil + return v.(string), nil case reflect.Bool: return fmt.Sprintf("%t", v), nil case reflect.Ptr: diff --git a/ytypes/choice.go b/ytypes/choice.go index 8a34bff30..8b1c91834 100644 --- a/ytypes/choice.go +++ b/ytypes/choice.go @@ -32,7 +32,10 @@ import ( // element in such cases. It returns all the field names that were selected in // the data tree from the Choice schema. func validateChoice(schema *yang.Entry, structValue ygot.GoStruct) (selected []string, errors []error) { - util.DbgPrint("validateChoice with value %s, schema name %s\n", util.ValueStrDebug(structValue), schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("validateChoice with value %s, schema name %s\n", util.ValueStrDebug(structValue), schema.Name) + } + // Validate that multiple cases are not selected. Since choice is always // inside a container, there's no need to validate each individual field // since that is part of container validation. diff --git a/ytypes/common.go b/ytypes/common.go index ee98b320c..d3446ae14 100644 --- a/ytypes/common.go +++ b/ytypes/common.go @@ -22,9 +22,11 @@ import ( // stringMapSetToSlice converts a string set expressed as a map m, into a slice // of strings. func stringMapSetToSlice(m map[string]interface{}) []string { - var out []string + out := make([]string, len(m)) + i := 0 for k := range m { - out = append(out, k) + out[i] = k + i++ } return out } diff --git a/ytypes/container.go b/ytypes/container.go index 12f1a173e..b95d364f4 100644 --- a/ytypes/container.go +++ b/ytypes/container.go @@ -39,7 +39,9 @@ func validateContainer(schema *yang.Entry, value ygot.GoStruct) util.Errors { return util.NewErrs(err) } - util.DbgPrint("validateContainer with value %v, type %T, schema name %s", util.ValueStrDebug(value), value, schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("validateContainer with value %v, type %T, schema name %s", util.ValueStrDebug(value), value, schema.Name) + } extraFields := make(map[string]interface{}) @@ -123,7 +125,9 @@ func unmarshalContainer(schema *yang.Entry, parent interface{}, jsonTree interfa return err } - util.DbgPrint("unmarshalContainer jsonTree %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(jsonTree), jsonTree, parent, schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("unmarshalContainer jsonTree %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(jsonTree), jsonTree, parent, schema.Name) + } // Since this is a container, the JSON data tree is a map. jt, ok := jsonTree.(map[string]interface{}) @@ -214,11 +218,16 @@ func unmarshalStruct(schema *yang.Entry, parent interface{}, jsonTree map[string } if jsonValue == nil { - util.DbgPrint("field %s paths %v not present in tree", ft.Name, sp) + if util.DebugLibraryEnabled() { + util.DbgPrint("field %s paths %v not present in tree", ft.Name, sp) + } continue } - util.DbgPrint("populating field %s type %s with paths %v.", ft.Name, ft.Type, sp) + if util.DebugLibraryEnabled() { + util.DbgPrint("populating field %s type %s with paths %v.", ft.Name, ft.Type, sp) + } + // Only create a new field if it is nil, otherwise update just the // fields that are in the data tree being passed to unmarshal, and // preserve all other existing values. @@ -252,7 +261,9 @@ func unmarshalStruct(schema *yang.Entry, parent interface{}, jsonTree map[string } } - util.DbgPrint("container after unmarshal:\n%s\n", pretty.Sprint(destv.Interface())) + if util.DebugLibraryEnabled() { + util.DbgPrint("container after unmarshal:\n%s\n", pretty.Sprint(destv.Interface())) + } return nil } diff --git a/ytypes/int_type.go b/ytypes/int_type.go index 17bc14d3b..a18739d0d 100644 --- a/ytypes/int_type.go +++ b/ytypes/int_type.go @@ -84,7 +84,9 @@ func validateInt(schema *yang.Entry, value interface{}) error { return err } - util.DbgPrint("validateInt type %s with value %v", util.YangTypeToDebugString(schema.Type), value) + if util.DebugLibraryEnabled() { + util.DbgPrint("validateInt type %s with value %v", util.YangTypeToDebugString(schema.Type), value) + } kind := schema.Type.Kind @@ -115,7 +117,9 @@ func validateIntSlice(schema *yang.Entry, value interface{}) error { return err } - util.DbgPrint("validateIntSlice type %s with value %v", util.YangTypeToDebugString(schema.Type), value) + if util.DebugLibraryEnabled() { + util.DbgPrint("validateIntSlice type %s with value %v", util.YangTypeToDebugString(schema.Type), value) + } kind := schema.Type.Kind val := reflect.ValueOf(value) diff --git a/ytypes/int_type_test.go b/ytypes/int_type_test.go index e44d0979f..03d4b97f1 100644 --- a/ytypes/int_type_test.go +++ b/ytypes/int_type_test.go @@ -509,51 +509,51 @@ func toGoType(kind yang.TypeKind, val int64) interface{} { func toGoSliceType(kind yang.TypeKind, in []int64) interface{} { switch kind { case yang.Yint8: - var out []int8 - for _, v := range in { - out = append(out, int8(v)) + out := make([]int8, len(in)) + for i, v := range in { + out[i] = int8(v) } return out case yang.Yint16: - var out []int16 - for _, v := range in { - out = append(out, int16(v)) + out := make([]int16, len(in)) + for i, v := range in { + out[i] = int16(v) } return out case yang.Yint32: - var out []int32 - for _, v := range in { - out = append(out, int32(v)) + out := make([]int32, len(in)) + for i, v := range in { + out[i] = int32(v) } return out case yang.Yint64: - var out []int64 - for _, v := range in { - out = append(out, int64(v)) + out := make([]int64, len(in)) + for i, v := range in { + out[i] = int64(v) } return out case yang.Yuint8: - var out []uint8 - for _, v := range in { - out = append(out, uint8(v)) + out := make([]uint8, len(in)) + for i, v := range in { + out[i] = uint8(v) } return out case yang.Yuint16: - var out []uint16 - for _, v := range in { - out = append(out, uint16(v)) + out := make([]uint16, len(in)) + for i, v := range in { + out[i] = uint16(v) } return out case yang.Yuint32: - var out []uint32 - for _, v := range in { - out = append(out, uint32(v)) + out := make([]uint32, len(in)) + for i, v := range in { + out[i] = uint32(v) } return out case yang.Yuint64: - var out []uint64 - for _, v := range in { - out = append(out, uint64(v)) + out := make([]uint64, len(in)) + for i, v := range in { + out[i] = uint64(v) } return out default: diff --git a/ytypes/leaf.go b/ytypes/leaf.go index 740cf2846..157e59e2f 100644 --- a/ytypes/leaf.go +++ b/ytypes/leaf.go @@ -47,7 +47,9 @@ func validateLeaf(inSchema *yang.Entry, value interface{}) util.Errors { return util.NewErrs(err) } - util.DbgPrint("validateLeaf with value %s (%T), schema name %s (%s)", util.ValueStrDebug(value), value, inSchema.Name, inSchema.Type.Kind) + if util.DebugLibraryEnabled() { + util.DbgPrint("validateLeaf with value %s (%T), schema name %s (%s)", util.ValueStrDebug(value), value, inSchema.Name, inSchema.Type.Kind) + } schema, err := util.ResolveIfLeafRef(inSchema) if err != nil { @@ -222,7 +224,10 @@ func validateUnion(schema *yang.Entry, value interface{}) util.Errors { return nil } - util.DbgPrint("validateUnion %s", schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("validateUnion %s", schema.Name) + } + v := reflect.ValueOf(value) if v.Kind() == reflect.Ptr { // The union could be a ptr - either a struct ptr or Go value ptr like *string. @@ -257,11 +262,15 @@ func validateUnion(schema *yang.Entry, value interface{}) util.Errors { func validateMatchingSchemas(schema *yang.Entry, value interface{}) util.Errors { var errors []error ss := findMatchingSchemasInUnion(schema.Type, value) - var kk []yang.TypeKind - for _, s := range ss { - kk = append(kk, s.Type.Kind) + kk := make([]yang.TypeKind, len(ss)) + for i, s := range ss { + kk[i] = s.Type.Kind + } + + if util.DebugLibraryEnabled() { + util.DbgPrint("validateMatchingSchemas for value %v (%T) for schema %s with types %v", value, value, schema.Name, kk) } - util.DbgPrint("validateMatchingSchemas for value %v (%T) for schema %s with types %v", value, value, schema.Name, kk) + if len(ss) == 0 { return util.NewErrs(fmt.Errorf("no types in schema %s match the type of value %v, which is %T", schema.Name, util.ValueStr(value), value)) } @@ -290,7 +299,10 @@ func validateMatchingSchemas(schema *yang.Entry, value interface{}) util.Errors func findMatchingSchemasInUnion(ytype *yang.YangType, value interface{}) []*yang.Entry { var matches []*yang.Entry - util.DbgPrint("findMatchingSchemasInUnion for type %T, kind %s", value, reflect.TypeOf(value).Kind()) + if util.DebugLibraryEnabled() { + util.DbgPrint("findMatchingSchemasInUnion for type %T, kind %s", value, reflect.TypeOf(value).Kind()) + } + for _, t := range ytype.Type { if t.Kind == yang.Yunion { // Recursively check all union types within this union. @@ -354,7 +366,9 @@ func unmarshalLeaf(inSchema *yang.Entry, parent interface{}, value interface{}, return err } - util.DbgPrint("unmarshalLeaf value %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(value), value, parent, inSchema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("unmarshalLeaf value %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(value), value, parent, inSchema.Name) + } fieldName, _, err := schemaToStructFieldName(inSchema, parent, hasPreferShadowPath(opts)) if err != nil { @@ -423,7 +437,10 @@ with field String set to "forty-two". */ func unmarshalUnion(schema *yang.Entry, parent interface{}, fieldName string, value interface{}, enc Encoding) error { - util.DbgPrint("unmarshalUnion value %v, type %T, into parent type %T field name %s, schema name %s", util.ValueStrDebug(value), value, parent, fieldName, schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("unmarshalUnion value %v, type %T, into parent type %T field name %s, schema name %s", util.ValueStrDebug(value), value, parent, fieldName, schema.Name) + } + parentV, parentT := reflect.ValueOf(parent), reflect.TypeOf(parent) if !util.IsTypeStructPtr(parentT) { return fmt.Errorf("%T is not a struct ptr in unmarshalUnion", parent) @@ -509,13 +526,19 @@ func unmarshalUnion(schema *yang.Entry, parent interface{}, fieldName string, va } for _, sk := range sks { - util.DbgPrint("try to unmarshal into type %s", sk) + if util.DebugLibraryEnabled() { + util.DbgPrint("try to unmarshal into type %s", sk) + } + sch := yangKindToLeafEntry(sk) gv, err := unmarshalScalar(parent, sch, fieldName, value, enc) if err == nil { return setUnionFieldWithTypedValue(parentT, destUnionFieldV, destUnionFieldElemT, gv) } - util.DbgPrint("could not unmarshal %v into type %s: %s", value, sk, err) + + if util.DebugLibraryEnabled() { + util.DbgPrint("could not unmarshal %v into type %s: %s", value, sk, err) + } } return fmt.Errorf("could not find suitable union type to unmarshal value %v type %T into parent struct type %T field %s", value, value, parent, fieldName) @@ -524,7 +547,10 @@ func unmarshalUnion(schema *yang.Entry, parent interface{}, fieldName string, va // setUnionFieldWithTypedValue sets the field destV with value v after converting it // to destElemT using the union conversion function of the given parent type. func setUnionFieldWithTypedValue(parentT reflect.Type, destV reflect.Value, destElemT reflect.Type, v interface{}) error { - util.DbgPrint("setUnionFieldWithTypedValue value %v into type %s", util.ValueStrDebug(v), destElemT) + if util.DebugLibraryEnabled() { + util.DbgPrint("setUnionFieldWithTypedValue value %v into type %s", util.ValueStrDebug(v), destElemT) + } + eiv, err := getUnionVal(parentT, destElemT, v) if err != nil { return err @@ -541,7 +567,10 @@ func setUnionFieldWithTypedValue(parentT reflect.Type, destV reflect.Value, dest // getUnionVal converts the input value v to the target union type using the // union conversion function of the parent type. func getUnionVal(parentT reflect.Type, destElemT reflect.Type, v interface{}) (reflect.Value, error) { - util.DbgPrint("getUnionVal value %v into type %s", util.ValueStrDebug(v), destElemT) + if util.DebugLibraryEnabled() { + util.DbgPrint("getUnionVal value %v into type %s", util.ValueStrDebug(v), destElemT) + } + if destElemT.Kind() == reflect.Slice { // leaf-list case destElemT = destElemT.Elem() @@ -561,7 +590,9 @@ func getUnionVal(parentT reflect.Type, destElemT reflect.Type, v interface{}) (r return reflect.ValueOf(nil), fmt.Errorf("unmarshaled %v type %T does not have a union type: %v", v, v, ee) } - util.DbgPrint("unmarshaling %v into type %s", v, reflect.TypeOf(ei)) + if util.DebugLibraryEnabled() { + util.DbgPrint("unmarshaling %v into type %s", v, reflect.TypeOf(ei)) + } return reflect.ValueOf(ei), nil } @@ -635,7 +666,9 @@ func schemaToEnumTypes(schema *yang.Entry, t reflect.Type) ([]reflect.Type, erro return nil, fmt.Errorf("%s ΛEnumTypes function returned wrong type %T, want map[string][]reflect.Type", t, ei) } - util.DbgPrint("path is %s for schema %s", absoluteSchemaDataPath(schema), schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("path is %s for schema %s", absoluteSchemaDataPath(schema), schema.Name) + } return enumTypesMap[absoluteSchemaDataPath(schema)], nil } @@ -659,7 +692,9 @@ func unmarshalScalar(parent interface{}, schema *yang.Entry, fieldName string, v return nil, err } - util.DbgPrint("unmarshalScalar value %v, type %T, into parent type %T field %s", value, value, parent, fieldName) + if util.DebugLibraryEnabled() { + util.DbgPrint("unmarshalScalar value %v, type %T, into parent type %T field %s", value, value, parent, fieldName) + } switch enc { case JSONEncoding: diff --git a/ytypes/leaf_list.go b/ytypes/leaf_list.go index 053de5991..9390be2c4 100644 --- a/ytypes/leaf_list.go +++ b/ytypes/leaf_list.go @@ -39,7 +39,9 @@ func validateLeafList(schema *yang.Entry, value interface{}) util.Errors { return util.NewErrs(err) } - util.DbgPrint("validateLeafList with value %v, type %T, schema name %s", util.ValueStrDebug(value), value, schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("validateLeafList with value %v, type %T, schema name %s", util.ValueStrDebug(value), value, schema.Name) + } switch reflect.TypeOf(value).Kind() { case reflect.Slice: @@ -106,7 +108,9 @@ func unmarshalLeafList(schema *yang.Entry, parent interface{}, value interface{} return err } - util.DbgPrint("unmarshalLeafList value %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(value), value, parent, schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("unmarshalLeafList value %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(value), value, parent, schema.Name) + } // The leaf schema is just the leaf-list schema without the list attrs. leafSchema := *schema @@ -155,7 +159,9 @@ func unmarshalLeafList(schema *yang.Entry, parent interface{}, value interface{} // clearSliceField sets updates a field called fieldName (which must exist, but may be // nil) in parentStruct, with value nil. func clearSliceField(parentStruct interface{}, fieldName string) error { - util.DbgPrint("clearSliceField field %s of parent type %T with value %v", fieldName, parentStruct) + if util.DebugLibraryEnabled() { + util.DbgPrint("clearSliceField field %s of parent type %T with value %v", fieldName, parentStruct) + } if util.IsValueNil(parentStruct) { return fmt.Errorf("parent is nil in clearSliceField for field %s", fieldName) diff --git a/ytypes/leafref.go b/ytypes/leafref.go index 2d534c1e0..2aa2cca98 100644 --- a/ytypes/leafref.go +++ b/ytypes/leafref.go @@ -75,7 +75,9 @@ func ValidateLeafRefData(schema *yang.Entry, value interface{}, opt *LeafrefOpti } pathStr := util.StripModulePrefixesStr(schema.Type.Path) - util.DbgPrint("Verifying leafref at %s, matching nodes are: %v", pathStr, util.ValueStrDebug(matchNodes)) + if util.DebugLibraryEnabled() { + util.DbgPrint("Verifying leafref at %s, matching nodes are: %v", pathStr, util.ValueStrDebug(matchNodes)) + } match, err := matchesNodes(ni, matchNodes) if err != nil { @@ -84,7 +86,11 @@ func ValidateLeafRefData(schema *yang.Entry, value interface{}, opt *LeafrefOpti if !match { e := fmt.Errorf("field name %s value %s schema path %s has leafref path %s not equal to any target nodes", ni.StructField.Name, util.ValueStr(ni.FieldValue.Interface()), ni.Schema.Path(), pathStr) - util.DbgPrint("ERR: %s", e) + + if util.DebugLibraryEnabled() { + util.DbgPrint("ERR: %s", e) + } + return leafrefErrOrLog(util.NewErrs(e), opt) } @@ -173,7 +179,10 @@ func leafRefToGNMIPath(root *util.NodeInfo, path string, pathQueryNode *util.Pat // dataNodesAtPath returns all nodes that match the given path from the given // node. func dataNodesAtPath(ni *util.NodeInfo, path *gpb.Path, pathQueryNode *util.PathQueryNodeMemo) ([]interface{}, error) { - util.DbgPrint("DataNodeAtPath got leafref with path %s from node path %s, field name %s", path, ni.Schema.Path(), ni.StructField.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("DataNodeAtPath got leafref with path %s from node path %s, field name %s", path, ni.Schema.Path(), ni.StructField.Name) + } + if path == nil || len(path.GetElem()) == 0 { return []interface{}{ni}, nil } @@ -205,15 +214,21 @@ func dataNodesAtPath(ni *util.NodeInfo, path *gpb.Path, pathQueryNode *util.Path continue } else { path.Elem = removeParentDirPrefix(path.GetElem(), root.PathFromParent) - util.DbgPrint("going up data tree from type %s to %s, schema path from parent is %v, remaining path %v", - root.FieldValue.Type(), root.Parent.FieldValue.Type(), root.PathFromParent, path) + + if util.DebugLibraryEnabled() { + util.DbgPrint("going up data tree from type %s to %s, schema path from parent is %v, remaining path %v", + root.FieldValue.Type(), root.Parent.FieldValue.Type(), root.PathFromParent, path) + } + root = root.Parent pathQueryRoot = pathQueryRoot.Parent } } } - util.DbgPrint("root element type %s with remaining path %s", root.FieldValue.Type(), path) + if util.DebugLibraryEnabled() { + util.DbgPrint("root element type %s with remaining path %s", root.FieldValue.Type(), path) + } // Check whether we have already done a lookup for the path specified by 'path' from this node before // -- if so, return it from the cache rather than walking the tree again @@ -260,12 +275,16 @@ func matchesNodes(ni *util.NodeInfo, matchNodes []interface{}) (bool, error) { pathStr := util.StripModulePrefixesStr(ni.Schema.Type.Path) if util.IsNilOrInvalidValue(ni.FieldValue) || util.IsValueNilOrDefault(ni.FieldValue.Interface()) { if len(matchNodes) == 0 { - util.DbgPrint("OK: source value is nil, dest is empty or list") + if util.DebugLibraryEnabled() { + util.DbgPrint("OK: source value is nil, dest is empty or list") + } return true, nil } other := matchNodes[0] if util.IsValueNilOrDefault(other) { - util.DbgPrint("OK: both values are nil for leafref") + if util.DebugLibraryEnabled() { + util.DbgPrint("OK: both values are nil for leafref") + } return true, nil } return true, nil @@ -293,17 +312,27 @@ func matchesNodes(ni *util.NodeInfo, matchNodes []interface{}) (bool, error) { ov := reflect.ValueOf(other) switch { case util.IsValueScalar(ov): - util.DbgPrint("comparing leafref values %s vs %s", util.ValueStrDebug(sourceNode), util.ValueStrDebug(other)) + if util.DebugLibraryEnabled() { + util.DbgPrint("comparing leafref values %s vs %s", util.ValueStrDebug(sourceNode), util.ValueStrDebug(other)) + } if util.DeepEqualDerefPtrs(sourceNode, other) { - util.DbgPrint("values are equal") + if util.DebugLibraryEnabled() { + util.DbgPrint("values are equal") + } return true, nil } case util.IsValueSlice(ov): sourceNode := ni.FieldValue.Interface() - util.DbgPrint("checking whether value %s is leafref leaf-list %v", util.ValueStrDebug(sourceNode), util.ValueStrDebug(other)) + + if util.DebugLibraryEnabled() { + util.DbgPrint("checking whether value %s is leafref leaf-list %v", util.ValueStrDebug(sourceNode), util.ValueStrDebug(other)) + } + for i := 0; i < ov.Len(); i++ { if util.DeepEqualDerefPtrs(sourceNode, ov.Index(i).Interface()) { - util.DbgPrint("value exists in list") + if util.DebugLibraryEnabled() { + util.DbgPrint("value exists in list") + } return true, nil } } diff --git a/ytypes/list.go b/ytypes/list.go index a8fb934a2..ffb794aa3 100644 --- a/ytypes/list.go +++ b/ytypes/list.go @@ -39,7 +39,9 @@ func validateList(schema *yang.Entry, value interface{}) util.Errors { return util.NewErrs(err) } - util.DbgPrint("validateList with value %v, type %T, schema name %s", value, value, schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("validateList with value %v, type %T, schema name %s", value, value, schema.Name) + } kind := reflect.TypeOf(value).Kind() if kind == reflect.Slice || kind == reflect.Map { @@ -288,7 +290,9 @@ func unmarshalList(schema *yang.Entry, parent interface{}, jsonList interface{}, return err } - util.DbgPrint("unmarshalList jsonList %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(jsonList), jsonList, parent, schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("unmarshalList jsonList %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(jsonList), jsonList, parent, schema.Name) + } // Parent must be a map, slice ptr, or struct ptr. t := reflect.TypeOf(parent) @@ -330,7 +334,11 @@ func unmarshalList(schema *yang.Entry, parent interface{}, jsonList interface{}, var err error jt := le.(map[string]interface{}) newVal := reflect.New(listElementType.Elem()) - util.DbgPrint("creating a new list element val of type %v", newVal.Type()) + + if util.DebugLibraryEnabled() { + util.DbgPrint("creating a new list element val of type %v", newVal.Type()) + } + if err := unmarshalStruct(schema, newVal.Interface(), jt, enc, opts...); err != nil { return err } @@ -352,7 +360,10 @@ func unmarshalList(schema *yang.Entry, parent interface{}, jsonList interface{}, return err } } - util.DbgPrint("list after unmarshal:\n%s\n", pretty.Sprint(parent)) + + if util.DebugLibraryEnabled() { + util.DbgPrint("list after unmarshal:\n%s\n", pretty.Sprint(parent)) + } return nil } @@ -477,7 +488,11 @@ func makeKeyForInsert(schema *yang.Entry, parentMap interface{}, newVal reflect. if !nv.IsValid() { return reflect.ValueOf(nil), fmt.Errorf("%v field doesn't have a valid value", kfn) } - util.DbgPrint("Setting value of %v (%T) in key struct (%T)", nv.Interface(), nv.Interface(), newKey.Interface()) + + if util.DebugLibraryEnabled() { + util.DbgPrint("Setting value of %v (%T) in key struct (%T)", nv.Interface(), nv.Interface(), newKey.Interface()) + } + newKeyField := newKey.FieldByName(kfn) if !nv.Type().AssignableTo(newKeyField.Type()) { return reflect.ValueOf(nil), fmt.Errorf("multi-key %v is not assignable to %v", nv.Type(), newKeyField.Type()) @@ -494,7 +509,10 @@ func makeKeyForInsert(schema *yang.Entry, parentMap interface{}, newVal reflect. if err != nil { return reflect.ValueOf(nil), err } - util.DbgPrint("key value is %v.", kv) + + if util.DebugLibraryEnabled() { + util.DbgPrint("key value is %v.", kv) + } rvKey := reflect.ValueOf(kv) diff --git a/ytypes/node.go b/ytypes/node.go index fc174394d..c1f23672e 100644 --- a/ytypes/node.go +++ b/ytypes/node.go @@ -327,13 +327,15 @@ func retrieveNodeContainer(schema *yang.Entry, root interface{}, path *gpb.Path, // with keys corresponding to the key supplied in path. // Function returns list of nodes, list of schemas and error. func retrieveNodeList(schema *yang.Entry, root interface{}, path, traversedPath *gpb.Path, args retrieveNodeArgs) ([]*TreeNode, error) { - rv := reflect.ValueOf(root) switch { case schema.Key == "": return nil, status.Errorf(codes.InvalidArgument, "unkeyed list can't be traversed, type %T, path %v", root, path) case len(path.GetElem()) == 0: return nil, status.Errorf(codes.InvalidArgument, "path length is 0, schema %v, root %v", schema, root) - case !util.IsValueMap(rv): + } + + rv := reflect.ValueOf(root) + if !util.IsValueMap(rv) { return nil, status.Errorf(codes.InvalidArgument, "root has type %T, expect map", root) } diff --git a/ytypes/unmarshal.go b/ytypes/unmarshal.go index 306aeaa70..b1401d042 100644 --- a/ytypes/unmarshal.go +++ b/ytypes/unmarshal.go @@ -75,13 +75,18 @@ const ( // is GNMIEncoding, the schema needs to be pointing to a leaf or leaf list // schema. func unmarshalGeneric(schema *yang.Entry, parent interface{}, value interface{}, enc Encoding, opts ...UnmarshalOpt) error { - util.Indent() - defer util.Dedent() + if util.DebugLibraryEnabled() { + util.Indent() + defer util.Dedent() + } if schema == nil { return fmt.Errorf("nil schema for parent type %T, value %v (%T)", parent, value, value) } - util.DbgPrint("Unmarshal value %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(value), value, parent, schema.Name) + + if util.DebugLibraryEnabled() { + util.DbgPrint("Unmarshal value %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(value), value, parent, schema.Name) + } if enc == GNMIEncoding && !(schema.IsLeaf() || schema.IsLeafList()) { return errors.New("unmarshalling a non leaf node isn't supported in GNMIEncoding mode") diff --git a/ytypes/util_schema.go b/ytypes/util_schema.go index 2cb4c555f..514bc470b 100644 --- a/ytypes/util_schema.go +++ b/ytypes/util_schema.go @@ -155,10 +155,16 @@ func directDescendantSchema(f reflect.StructField) (string, error) { if err != nil { return "", err } + + // Early return for performance. + if strings.Index(pathAnnotation, "|") == -1 && strings.Index(pathAnnotation, "/") == -1 { + return pathAnnotation, nil + } + paths := strings.Split(pathAnnotation, "|") for _, pth := range paths { - if len(strings.Split(pth, "/")) == 1 { + if strings.Index(pth, "/") == -1 { return pth, nil } } @@ -174,7 +180,10 @@ func dataTreePaths(parentSchema, schema *yang.Entry, f reflect.StructField) ([][ return nil, err } n, err := removeNonDataPathElements(parentSchema, schema, out) - util.DbgPrint("have paths %v, removing non-data from %s -> %v", out, schema.Name, n) + + if util.DebugLibraryEnabled() { + util.DbgPrint("have paths %v, removing non-data from %s -> %v", out, schema.Name, n) + } return n, err } @@ -184,15 +193,18 @@ func dataTreePaths(parentSchema, schema *yang.Entry, f reflect.StructField) ([][ func shadowDataTreePaths(parentSchema, schema *yang.Entry, f reflect.StructField) ([][]string, error) { out := util.ShadowSchemaPaths(f) n, err := removeNonDataPathElements(parentSchema, schema, out) - util.DbgPrint("have shadow paths %v, removing non-data from %s -> %v", out, schema.Name, n) + + if util.DebugLibraryEnabled() { + util.DbgPrint("have shadow paths %v, removing non-data from %s -> %v", out, schema.Name, n) + } return n, err } // removeNonDataPathElements removes any path elements in paths not found in // the data tree given the terminal node schema and the schema of its parent. func removeNonDataPathElements(parentSchema, schema *yang.Entry, paths [][]string) ([][]string, error) { - var out [][]string - for _, path := range paths { + out := make([][]string, len(paths)) + for i, path := range paths { var po []string s := parentSchema if path[0] == s.Name { @@ -213,7 +225,7 @@ func removeNonDataPathElements(parentSchema, schema *yang.Entry, paths [][]strin po = append(po, pe) } } - out = append(out, po) + out[i] = po } return out, nil @@ -292,7 +304,6 @@ func checkDataTreeAgainstPaths(jsonTree map[string]interface{}, dataPaths [][]st // It returns empty string and nil error if the field does not exist in the // parent struct. func schemaToStructFieldName(schema *yang.Entry, parent interface{}, preferShadowPath bool) (string, *yang.Entry, error) { - v := reflect.ValueOf(parent) if util.IsNilOrInvalidValue(v) { return "", nil, fmt.Errorf("parent field is nil in schemaToStructFieldName for node %s", schema.Name) diff --git a/ytypes/util_types.go b/ytypes/util_types.go index 33c6edb6a..ae2d86d18 100644 --- a/ytypes/util_types.go +++ b/ytypes/util_types.go @@ -30,7 +30,10 @@ import ( // enumStringToValue returns the enum type value that enumerated string value // of type fieldName maps to in the parent, which must be a struct ptr. func enumStringToValue(parent interface{}, fieldName, value string) (interface{}, error) { - util.DbgPrint("enumStringToValue with parent type %T, fieldName %s, value %s", parent, fieldName, value) + if util.DebugLibraryEnabled() { + util.DbgPrint("enumStringToValue with parent type %T, fieldName %s, value %s", parent, fieldName, value) + } + v := reflect.ValueOf(parent) if !util.IsValueStructPtr(v) { return 0, fmt.Errorf("enumStringToIntValue: %T is not a struct ptr", parent) @@ -66,7 +69,9 @@ func enumAndNonEnumTypesForUnion(schema *yang.Entry, parentT reflect.Type) ([]re return nil, nil, err } - util.DbgPrint("enumAndNonEnumTypesForUnion: possible union types are enums %v or scalars %v", ets, sks) + if util.DebugLibraryEnabled() { + util.DbgPrint("enumAndNonEnumTypesForUnion: possible union types are enums %v or scalars %v", ets, sks) + } return ets, sks, nil } @@ -101,9 +106,15 @@ func getLoneUnionType(schema *yang.Entry, unionT reflect.Type, ets []reflect.Typ // the string value, and returns upon success. If the string value can't be // casted to any, nil is returned (without error). func castToOneEnumValue(ets []reflect.Type, value string) (interface{}, error) { - util.DbgPrint("castToOneEnumValue: %q", value) + if util.DebugLibraryEnabled() { + util.DbgPrint("castToOneEnumValue: %q", value) + } + for _, et := range ets { - util.DbgPrint("try to unmarshal into enum type %v", et) + if util.DebugLibraryEnabled() { + util.DbgPrint("try to unmarshal into enum type %v", et) + } + ev, err := castToEnumValue(et, value) if err != nil { return nil, err @@ -111,7 +122,10 @@ func castToOneEnumValue(ets []reflect.Type, value string) (interface{}, error) { if ev != nil { return ev, nil } - util.DbgPrint("could not unmarshal %q into enum type, err: %v", value, err) + + if util.DebugLibraryEnabled() { + util.DbgPrint("could not unmarshal %q into enum type, err: %v", value, err) + } } return nil, nil } @@ -124,7 +138,10 @@ func castToEnumValue(ft reflect.Type, value string) (interface{}, error) { ft = ft.Elem() } - util.DbgPrint("checking for matching enum value for type %s", ft) + if util.DebugLibraryEnabled() { + util.DbgPrint("checking for matching enum value for type %s", ft) + } + mapMethod := reflect.New(ft).MethodByName("ΛMap") if !mapMethod.IsValid() { return 0, fmt.Errorf("%s does not have a ΛMap function", ft) @@ -293,7 +310,9 @@ func stringToKeyType(schema *yang.Entry, parent interface{}, fieldName string, v // stringToUnionType converts a string value into a suitable union type // determined by where it is located in the YANG tree. func stringToUnionType(schema *yang.Entry, parent interface{}, fieldName string, value string) (reflect.Value, error) { - util.DbgPrint("stringToUnionType value %v, into parent type %T field name %s, schema name %s", util.ValueStrDebug(value), parent, fieldName, schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("stringToUnionType value %v, into parent type %T field name %s, schema name %s", util.ValueStrDebug(value), parent, fieldName, schema.Name) + } if !util.IsTypeStructPtr(reflect.TypeOf(parent)) { return reflect.ValueOf(nil), fmt.Errorf("stringToKeyType: %T is not a struct ptr", parent) } @@ -336,12 +355,18 @@ func stringToUnionType(schema *yang.Entry, parent interface{}, fieldName string, } for _, sk := range sks { - util.DbgPrint("try to convert string %q into type %s", value, sk) + if util.DebugLibraryEnabled() { + util.DbgPrint("try to convert string %q into type %s", value, sk) + } + gv, err := stringToKeyType(yangKindToLeafEntry(sk), parent, fieldName, value) if err == nil { return getUnionVal(reflect.TypeOf(parent), fieldType, gv.Interface()) } - util.DbgPrint("could not unmarshal %v into type %v: %v", value, sk, err) + + if util.DebugLibraryEnabled() { + util.DbgPrint("could not unmarshal %v into type %v: %v", value, sk, err) + } } return reflect.ValueOf(nil), fmt.Errorf("could not find suitable union type to unmarshal value %q into parent struct type %T field %s", value, parent, fieldName) diff --git a/ytypes/validate.go b/ytypes/validate.go index 4f563370d..40108e6ec 100644 --- a/ytypes/validate.go +++ b/ytypes/validate.go @@ -96,7 +96,9 @@ func Validate(schema *yang.Entry, value interface{}, opts ...ygot.ValidationOpti } } - util.DbgPrint("Validate with value %v, type %T, schema name %s", util.ValueStr(value), value, schema.Name) + if util.DebugLibraryEnabled() { + util.DbgPrint("Validate with value %v, type %T, schema name %s", util.ValueStr(value), value, schema.Name) + } switch { case schema.IsLeaf(): From 5fd566f9d457ab03525b43b65bde72be29b6f8e3 Mon Sep 17 00:00:00 2001 From: Jay Zhu Date: Mon, 1 May 2023 16:37:32 -0600 Subject: [PATCH 2/5] fix(render): try int/uint casts before itoa --- ygot/render.go | 60 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/ygot/render.go b/ygot/render.go index 2142bfd29..96c5a7b75 100644 --- a/ygot/render.go +++ b/ygot/render.go @@ -601,27 +601,67 @@ func KeyValueAsString(v interface{}) (string, error) { switch kv.Kind() { case reflect.Int: - return strconv.Itoa(v.(int)), nil + if val, ok := v.(int); ok { + return strconv.Itoa(int(val)), nil + } + + return fmt.Sprintf("%d", v), nil case reflect.Int8: - return strconv.Itoa(int(v.(int8))), nil + if val, ok := v.(int8); ok { + return strconv.Itoa(int(val)), nil + } + + return fmt.Sprintf("%d", v), nil case reflect.Int16: - return strconv.Itoa(int(v.(int16))), nil + if val, ok := v.(int16); ok { + return strconv.FormatInt(int64(val), 10), nil + } + + return fmt.Sprintf("%d", v), nil case reflect.Int32: - return strconv.Itoa(int(v.(int32))), nil + if val, ok := v.(int32); ok { + return strconv.FormatInt(int64(val), 10), nil + } + + return fmt.Sprintf("%d", v), nil case reflect.Uint: - return strconv.FormatUint(uint64(v.(uint)), 10), nil + if val, ok := v.(uint); ok { + return strconv.FormatUint(uint64(val), 10), nil + } + + return fmt.Sprintf("%d", v), nil case reflect.Uint8: - return strconv.FormatUint(uint64(v.(uint8)), 10), nil + if val, ok := v.(uint8); ok { + return strconv.FormatUint(uint64(val), 10), nil + } + + return fmt.Sprintf("%d", v), nil case reflect.Uint16: - return strconv.FormatUint(uint64(v.(uint16)), 10), nil + if val, ok := v.(uint16); ok { + return strconv.FormatUint(uint64(val), 10), nil + } + + return fmt.Sprintf("%d", v), nil case reflect.Uint32: - return strconv.FormatUint(uint64(v.(uint32)), 10), nil + if val, ok := v.(uint32); ok { + return strconv.FormatUint(uint64(val), 10), nil + } + + return fmt.Sprintf("%d", v), nil case reflect.Uint64: - return strconv.FormatUint(v.(uint64), 10), nil + if val, ok := v.(uint64); ok { + return strconv.FormatUint(uint64(val), 10), nil + } + + return fmt.Sprintf("%d", v), nil case reflect.Float64: return fmt.Sprintf("%g", v), nil case reflect.String: - return v.(string), nil + if val, ok := v.(string); ok { + return val, nil + } + + return fmt.Sprintf("%s", v), nil case reflect.Bool: return fmt.Sprintf("%t", v), nil case reflect.Ptr: From a347329040019443e06e63d6185b8f489e41f1cb Mon Sep 17 00:00:00 2001 From: Jay Zhu Date: Tue, 2 May 2023 15:46:56 -0600 Subject: [PATCH 3/5] fix(lint): resolve static check errors --- util/path.go | 4 ++-- ytypes/util_schema.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/util/path.go b/util/path.go index 1ddc5d362..61935df00 100644 --- a/util/path.go +++ b/util/path.go @@ -31,7 +31,7 @@ func SchemaPaths(f reflect.StructField) ([][]string, error) { } // Early return for performance. - if strings.Index(pathTag, "|") == -1 { + if !strings.Contains(pathTag, "|") { return [][]string{stripModulePrefixes(strings.Split(pathTag, "/"))}, nil } @@ -131,7 +131,7 @@ func relativeSchemaPath(f reflect.StructField, preferShadowPath bool) ([]string, return nil, fmt.Errorf("field %s did not specify a shadow-path", f.Name) } - if strings.Index(pathTag, "|") == -1 { + if !strings.Contains(pathTag, "|") { pathTag = strings.TrimPrefix(pathTag, "/") return strings.Split(pathTag, "/"), nil } diff --git a/ytypes/util_schema.go b/ytypes/util_schema.go index 514bc470b..496d2d3b6 100644 --- a/ytypes/util_schema.go +++ b/ytypes/util_schema.go @@ -157,14 +157,14 @@ func directDescendantSchema(f reflect.StructField) (string, error) { } // Early return for performance. - if strings.Index(pathAnnotation, "|") == -1 && strings.Index(pathAnnotation, "/") == -1 { + if !strings.Contains(pathAnnotation, "|") && !strings.Contains(pathAnnotation, "/") { return pathAnnotation, nil } paths := strings.Split(pathAnnotation, "|") for _, pth := range paths { - if strings.Index(pth, "/") == -1 { + if !strings.Contains(pth, "/") { return pth, nil } } From fc261b5d8740eef5470b98b0f87d30e509a9ec95 Mon Sep 17 00:00:00 2001 From: Jay Zhu Date: Tue, 2 May 2023 18:18:41 -0600 Subject: [PATCH 4/5] cleanup(util/path): remove unnecessary length check in `StripModulePrefix` --- util/path.go | 2 +- util/path_test.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/util/path.go b/util/path.go index 61935df00..0da4c4e2c 100644 --- a/util/path.go +++ b/util/path.go @@ -348,7 +348,7 @@ func stripModulePrefixWithCheck(name string) (string, error) { // where remote paths are referenced. func StripModulePrefix(name string) string { for i, r := range name { - if r == ':' && len(name) > i+1 { + if r == ':' { return name[i+1:] } } diff --git a/util/path_test.go b/util/path_test.go index 3576e47db..1ac8b2165 100644 --- a/util/path_test.go +++ b/util/path_test.go @@ -830,6 +830,10 @@ func TestStripModulePrefix(t *testing.T) { desc: "empty string", inName: "", wantName: "", + }, { + desc: "a single `:`", + inName: ":", + wantName: "", }} for _, tt := range tests { From 98111faaef89f462c98155a596a4ced8574b5ada Mon Sep 17 00:00:00 2001 From: Jay Zhu Date: Wed, 3 May 2023 20:56:02 -0600 Subject: [PATCH 5/5] refactor(render): use a type switch for performant int/uint to string calls --- ygot/render.go | 77 ++++++++++++++++---------------------------------- 1 file changed, 24 insertions(+), 53 deletions(-) diff --git a/ygot/render.go b/ygot/render.go index 96c5a7b75..6ac6e30ca 100644 --- a/ygot/render.go +++ b/ygot/render.go @@ -599,60 +599,31 @@ func KeyValueAsString(v interface{}) (string, error) { return name, nil } - switch kv.Kind() { - case reflect.Int: - if val, ok := v.(int); ok { - return strconv.Itoa(int(val)), nil - } - - return fmt.Sprintf("%d", v), nil - case reflect.Int8: - if val, ok := v.(int8); ok { - return strconv.Itoa(int(val)), nil - } - - return fmt.Sprintf("%d", v), nil - case reflect.Int16: - if val, ok := v.(int16); ok { - return strconv.FormatInt(int64(val), 10), nil - } - - return fmt.Sprintf("%d", v), nil - case reflect.Int32: - if val, ok := v.(int32); ok { - return strconv.FormatInt(int64(val), 10), nil - } - - return fmt.Sprintf("%d", v), nil - case reflect.Uint: - if val, ok := v.(uint); ok { - return strconv.FormatUint(uint64(val), 10), nil - } - - return fmt.Sprintf("%d", v), nil - case reflect.Uint8: - if val, ok := v.(uint8); ok { - return strconv.FormatUint(uint64(val), 10), nil - } - - return fmt.Sprintf("%d", v), nil - case reflect.Uint16: - if val, ok := v.(uint16); ok { - return strconv.FormatUint(uint64(val), 10), nil - } - - return fmt.Sprintf("%d", v), nil - case reflect.Uint32: - if val, ok := v.(uint32); ok { - return strconv.FormatUint(uint64(val), 10), nil - } - - return fmt.Sprintf("%d", v), nil - case reflect.Uint64: - if val, ok := v.(uint64); ok { - return strconv.FormatUint(uint64(val), 10), nil - } + // Use `strconv` to handle integer to string conversion whenever possible. + switch val := v.(type) { + case int: + return strconv.Itoa(val), nil + case int8: + return strconv.Itoa(int(val)), nil + case int16: + return strconv.FormatInt(int64(val), 10), nil + case int32: + return strconv.FormatInt(int64(val), 10), nil + case uint: + return strconv.FormatUint(uint64(val), 10), nil + case uint8: + return strconv.FormatUint(uint64(val), 10), nil + case uint16: + return strconv.FormatUint(uint64(val), 10), nil + case uint32: + return strconv.FormatUint(uint64(val), 10), nil + case uint64: + return strconv.FormatUint(uint64(val), 10), nil + } + switch kv.Kind() { + // Handle int/uint type aliases (e.g.: UnionInt8). + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: return fmt.Sprintf("%d", v), nil case reflect.Float64: return fmt.Sprintf("%g", v), nil