From 94cf4a88b3344cde2c9875a192143f28f74529d8 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Tue, 28 Apr 2020 22:31:58 +0200 Subject: [PATCH] * Optimizations * Reduced memory for attribute and body objects, see `XML_NEW_DIFFERENTIATE` compile-time option. * Optimized cbuf handling in parsing and xml2cbuf functions. * Optimized xml scanner to read strings rather than single chars * Optimized xml_merge for the case of disjunct trees. --- CHANGELOG.md | 5 ++ doc/DEVELOP.md | 9 ++- example/main/example_backend.c | 59 +++++++++++--- include/clixon_custom.h | 8 ++ lib/clixon/clixon_string.h | 1 + lib/clixon/clixon_xml_sort.h | 4 +- lib/src/Makefile.in | 3 +- lib/src/clixon_json.c | 7 +- lib/src/clixon_string.c | 53 +++++++++++++ lib/src/clixon_xml.c | 121 +++++++++++++++++------------ lib/src/clixon_xml_io.c | 55 +++++++------ lib/src/clixon_xml_map.c | 132 +++++++++++++++++++++----------- lib/src/clixon_xml_nsctx.c | 5 +- lib/src/clixon_xml_parse.l | 1 + lib/src/clixon_xml_parse.y | 69 +++++++++-------- lib/src/clixon_xml_sort.c | 24 ++++++ lib/src/clixon_xpath_optimize.c | 4 +- test/test_cli.sh | 4 +- test/test_perf_mem.sh | 18 +++-- test/test_perf_state.sh | 46 +++++------ test/test_perf_state_only.sh | 87 +++++++++------------ test/test_perf_xml.sh | 2 +- test/test_restconf.sh | 10 +-- test/test_xml_trees.sh | 7 +- 24 files changed, 477 insertions(+), 257 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e3ba649..2121c049 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,11 @@ Expected: May 2020 ### Minor changes +* Optimizations + * Reduced memory for attribute and body objects, see `XML_NEW_DIFFERENTIATE` compile-time option. + * Optimized cbuf handling in parsing and xml2cbuf functions. + * Optimized xml scanner to read strings rather than single chars + * Optimized xml_merge for the case of disjunct trees. * Experimental: restart_plugin * Two new plugin callbacks added * ca_daemon: Called just after a server has "daemonized", ie put in background. diff --git a/doc/DEVELOP.md b/doc/DEVELOP.md index a3aeadca..c466016b 100644 --- a/doc/DEVELOP.md +++ b/doc/DEVELOP.md @@ -98,12 +98,19 @@ EOF ### Run valgrind and callgrind ``` valgrind --leak-check=full --show-leak-kinds=all clixon_netconf -qf /tmp/myconf.xml -y /tmp/my.yang - valgrind --tool=callgrind clixon_netconf -qf /tmp/myconf.xml -y /tmp/my.yang + LD_BIND_NOW=y valgrind --tool=callgrind clixon_netconf -qf /tmp/myconf.xml -y /tmp/my.yang sudo kcachegrind valgrind --tool=massif clixon_netconf -qf /tmp/myconf.xml -y /tmp/my.yang massif-visualizer ``` +To turn callgrind off/on: + ``` + valgrind --tool=callgrind --instr-atstart=no clixon_netconf -qf /tmp/myconf.xml -y /tmp/my.yang + ... + callgrind_control -i on + ``` + ## New release What to think about when doing a new release. * Ensure all tests run OK diff --git a/example/main/example_backend.c b/example/main/example_backend.c index d95aba4d..ba2d2891 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -75,11 +75,17 @@ static int _reset = 0; */ static int _state = 0; -/*! File where state XML is read from, if _state is true +/*! File where state XML is read from, if _state is true -- -sS * Primarily for testing */ static char *_state_file = NULL; +/*! Read state file init on startup instead of on request + * Primarily for testing + */ +static int _state_file_init = 0; +static cxobj *_state_xstate = NULL; + /*! Variable to control module-specific upgrade callbacks. * If set, call test-case for upgrading ietf-interfaces, otherwise call * auto-upgrade @@ -337,12 +343,19 @@ example_statedata(clicon_handle h, /* If -S is set, then read state data from file, otherwise construct it programmatically */ if (_state_file){ - if ((fd = open(_state_file, O_RDONLY)) < 0){ - clicon_err(OE_UNIX, errno, "open(%s)", _state_file); - goto done; + if (_state_file_init){ + if (xml_copy(_state_xstate, xstate) < 0) + goto done; + } + else{ + if ((fd = open(_state_file, O_RDONLY)) < 0){ + clicon_err(OE_UNIX, errno, "open(%s)", _state_file); + goto done; + } + if (clixon_xml_parse_file(fd, YB_MODULE, yspec, NULL, &xstate, NULL) < 0) + goto done; + close(fd); } - if (clixon_xml_parse_file(fd, YB_MODULE, yspec, NULL, &xstate, NULL) < 0) - goto done; } else { /* Example of statedata, in this case merging state data with @@ -820,7 +833,30 @@ example_start(clicon_handle h) int example_daemon(clicon_handle h) { - return 0; + int retval = -1; + int ret; + + /* Read state file (or should this be in init/start?) */ + if (_state && _state_file && _state_file_init){ + int fd; + yang_stmt *yspec = clicon_dbspec_yang(h); + + if ((fd = open(_state_file, O_RDONLY)) < 0){ + clicon_err(OE_UNIX, errno, "open(%s)", _state_file); + goto done; + } + if ((ret = clixon_xml_parse_file(fd, YB_MODULE, yspec, NULL, &_state_xstate, NULL)) < 0) + goto done; + close(fd); + if (ret == 0){ + fprintf(stderr, "%s error\n", __FUNCTION__); + goto done; + } + fprintf(stderr, "%s done\n", __FUNCTION__); + } + retval = 0; + done: + return retval; } int @@ -873,17 +909,20 @@ clixon_plugin_init(clicon_handle h) goto done; opterr = 0; optind = 1; - while ((c = getopt(argc, argv, "rsS:uUt")) != -1) + while ((c = getopt(argc, argv, "rsS:iuUt")) != -1) switch (c) { case 'r': _reset = 1; break; - case 's': + case 's': /* state callback */ _state = 1; break; - case 'S': /* state file */ + case 'S': /* state file (requires -s) */ _state_file = optarg; break; + case 'i': /* read state file on init not by request (requires -sS */ + _state_file_init = 1; + break; case 'u': /* module-specific upgrade */ _module_upgrade = 1; break; diff --git a/include/clixon_custom.h b/include/clixon_custom.h index 354337c5..f9a7e65b 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -75,6 +75,8 @@ /*! Add explicit search indexes, so that binary search can be made for non-key list indexes * This also applies if there are multiple keys and you want to search on only the second for * example. + * There may be some cases where the index vector is not updated, need to verify before + * enabling this completely. */ #define XML_EXPLICIT_INDEX @@ -94,10 +96,16 @@ * This statement (red:The "ordered-by" Statement) is ignored if the list represents * state data,... * but it is not clear it is ignored because it should always be ordered-by system? + * Cant measure any diff on performance with this on using large state lists (500K) * clixon-4.4 */ #define STATE_ORDERED_BY_SYSTEM /*! Restart specific backend plugins + * Experimental code for now, needs some testing */ #undef RESTART_PLUGIN_RPC + +/*! Differentiate creating XML object body/element vs elenmet to reduce space + */ +#define XML_NEW_DIFFERENTIATE diff --git a/lib/clixon/clixon_string.h b/lib/clixon/clixon_string.h index b5ab2bd6..ba69d491 100644 --- a/lib/clixon/clixon_string.h +++ b/lib/clixon/clixon_string.h @@ -97,6 +97,7 @@ int xml_chardata_encode(char **escp, char *fmt, ... ) __attribute__ ((format (pr int uri_percent_encode(char **encp, char *str, ...); int xml_chardata_encode(char **escp, char *fmt, ...); #endif +int xml_chardata_cbuf_append(cbuf *cb, char *str); int uri_percent_decode(char *enc, char **str); const char *clicon_int2str(const map_str2int *mstab, int i); int clicon_str2int(const map_str2int *mstab, char *str); diff --git a/lib/clixon/clixon_xml_sort.h b/lib/clixon/clixon_xml_sort.h index 06e45d0d..7f0fdea6 100644 --- a/lib/clixon/clixon_xml_sort.h +++ b/lib/clixon/clixon_xml_sort.h @@ -3,7 +3,8 @@ ***** BEGIN LICENSE BLOCK ***** Copyright (C) 2009-2016 Olof Hagsand and Benny Holmgren - Copyright (C) 2017-2020 Olof Hagsand + Copyright (C) 2017-2019 Olof Hagsand + Copyright (C) 2020 Olof Hagsand and Rubicon Communications, LLC(Netgate) This file is part of CLIXON. @@ -42,6 +43,7 @@ */ int xml_cmp(cxobj *x1, cxobj *x2, int same, int skip1, char *explicit); int xml_sort(cxobj *x0, void *arg); +int xml_sort_recurse(cxobj *xn); int xml_insert(cxobj *xp, cxobj *xc, enum insert_type ins, char *key_val, cvec *nsckey); int xml_sort_verify(cxobj *x, void *arg); #ifdef XML_EXPLICIT_INDEX diff --git a/lib/src/Makefile.in b/lib/src/Makefile.in index 3e36981a..af9a8524 100644 --- a/lib/src/Makefile.in +++ b/lib/src/Makefile.in @@ -2,7 +2,8 @@ # ***** BEGIN LICENSE BLOCK ***** # # Copyright (C) 2009-2016 Olof Hagsand and Benny Holmgren -# Copyright (C) 2017-2020 Olof Hagsand +# Copyright (C) 2017-2019 Olof Hagsand +# Copyright (C) 2020 Olof Hagsand and Rubicon Communications, LLC(Netgate) # # This file is part of CLIXON # diff --git a/lib/src/clixon_json.c b/lib/src/clixon_json.c index 36d06a8d..b92854c3 100644 --- a/lib/src/clixon_json.c +++ b/lib/src/clixon_json.c @@ -1266,7 +1266,12 @@ _json_parse(char *str, } if (failed) goto fail; - /* This fails if xt is not bound to yang */ + /* Sort the complete tree after parsing. Sorting is not really meaningful if Yang + not bound */ + if (yb != YB_NONE) + if (xml_sort_recurse(xt) < 0) + goto done; + if (xml_apply0(xt, CX_ELMNT, xml_sort, NULL) < 0) goto done; retval = 1; diff --git a/lib/src/clixon_string.c b/lib/src/clixon_string.c index 90869120..15e11e58 100644 --- a/lib/src/clixon_string.c +++ b/lib/src/clixon_string.c @@ -314,6 +314,7 @@ uri_percent_decode(char *enc, * ' -> "' " may * ' -> "" " may * Optionally > + * @see xml_chardata_cbuf_append for a specialized version */ int xml_chardata_encode(char **escp, @@ -433,6 +434,58 @@ xml_chardata_encode(char **escp, return retval; } +/*! Escape characters according to XML definition and append to cbuf + * @param[in] cb CLIgen buf + * @param[in] fmt Not-encoded input string + * @see xml_chardata_encode for the generic function + */ +int +xml_chardata_cbuf_append(cbuf *cb, + char *str) +{ + int retval = -1; + int i; + int cdata; /* when set, skip encoding */ + + /* The orignal of this code is in xml_chardata_encode */ + /* Step: encode and expand str --> enc */ + /* Same code again, but now actually encode into output buffer */ + cdata = 0; + for (i=0; i", strlen("]]>")) == 0){ + cdata = 0; + cbuf_append(cb, str[i++]); + cbuf_append(cb, str[i++]); + } + cbuf_append(cb, str[i]); + } + else + switch (str[i]){ + case '&': + cbuf_append_str(cb, "&"); + break; + case '<': + if (strncmp(&str[i], "': + cbuf_append_str(cb, ">"); + break; + default: + cbuf_append(cb, str[i]); + } + } + retval = 0; + // done: + return retval; +} + + /*! Split a string into a cligen variable vector using 1st and 2nd delimiter * Split a string first into elements delimited by delim1, then into * pairs delimited by delim2. diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 33faedda..4d90f8cc 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -79,8 +79,12 @@ * Constants */ /* How many XML children to start with if any. Then add quadratic until threshold when - * add lineraly */ -#define XML_CHILDVEC_SIZE_START 1 + * add lineraly + * Heurestics: if child is body only single child is expected, but element children may + * have siblings + */ +#define XML_CHILDVEC_SIZE_START 1 +#define XML_CHILDVEC_SIZE_START_ELMNT 16 #define XML_CHILDVEC_SIZE_THRESHOLD 65536 /* Intention of these macros is to guard against access of type-specific fields @@ -182,6 +186,8 @@ struct xml{ #endif }; + +#ifdef XML_NEW_DIFFERENTIATE /* This is experimental variant of struct xml for use by non-elements to save space */ struct xmlbody{ @@ -196,6 +202,7 @@ struct xmlbody{ cbuf *xb_value_cb; /* attribute and body nodes have values (XXX: this consumes memory) cv? */ }; +#endif /* XML_NEW_DIFFERENTIATE */ /* * Variables @@ -245,13 +252,13 @@ xml_stats_one(cxobj *x, { size_t sz = 0; - sz += sizeof(struct xml); if (x->x_name) sz += strlen(x->x_name) + 1; if (x->x_prefix) sz += strlen(x->x_prefix) + 1; switch (xml_type(x)){ case CX_ELMNT: + sz += sizeof(struct xml); sz += x->x_childvec_max*sizeof(struct xml*); if (x->x_ns_cache) sz += cvec_size(x->x_ns_cache); @@ -270,9 +277,13 @@ xml_stats_one(cxobj *x, break; case CX_BODY: case CX_ATTR: +#ifdef XML_NEW_DIFFERENTIATE + sz += sizeof(struct xmlbody); +#else + sz += sizeof(struct xmlbody); +#endif if (x->x_value_cb) sz += cbuf_buflen(x->x_value_cb); - break; default: break; @@ -610,7 +621,7 @@ xml_value_set(cxobj *xn, } else cbuf_reset(xn->x_value_cb); - cprintf(xn->x_value_cb, "%s", val); + cbuf_append_str(xn->x_value_cb, val); retval = 0; done: return retval; @@ -642,7 +653,7 @@ xml_value_append(cxobj *xn, goto done; } } - if (cprintf(xn->x_value_cb, "%s", val) < 0){ + if (cbuf_append_str(xn->x_value_cb, val) < 0){ clicon_err(OE_XML, errno, "cprintf"); goto done; } @@ -878,17 +889,27 @@ xml_child_each(cxobj *xparent, /*! Extend child vector with one and insert xml node there * @note does not do anything with child, you may need to set its parent, etc + * @see xml_child_insert_pos + * XXX could insert hint if we know this is a yang list and not a leaf to increase start. */ static int xml_child_append(cxobj *xp, cxobj *xc) { + size_t start; + if (!is_element(xp)) return 0; + start = XML_CHILDVEC_SIZE_START; + /* Heurestics: if child is body only single child is expected, but element children may + * have siblings + */ + if (xml_type(xc) == CX_ELMNT) + start = XML_CHILDVEC_SIZE_START_ELMNT; xp->x_childvec_len++; if (xp->x_childvec_len > xp->x_childvec_max){ if (xp->x_childvec_len < XML_CHILDVEC_SIZE_THRESHOLD) - xp->x_childvec_max = xp->x_childvec_max?2*xp->x_childvec_max:XML_CHILDVEC_SIZE_START; + xp->x_childvec_max = xp->x_childvec_max?2*xp->x_childvec_max:start; else xp->x_childvec_max += XML_CHILDVEC_SIZE_THRESHOLD; xp->x_childvec = realloc(xp->x_childvec, xp->x_childvec_max*sizeof(cxobj*)); @@ -983,6 +1004,49 @@ xml_childvec_get(cxobj *x) * @endcode * @see xml_sort_insert */ +#ifdef XML_NEW_DIFFERENTIATE +/* Differentiate creating XML object body/element vs elenmet to reduce space */ +cxobj * +xml_new(char *name, + cxobj *xp, + enum cxobj_type type) +{ + struct xml *x = NULL; + size_t sz; + + switch (type){ + case CX_ELMNT: + sz = sizeof(struct xml); + break; + case CX_ATTR: + case CX_BODY: + sz = sizeof(struct xmlbody); + break; + default: + clicon_err(OE_XML, EINVAL, "Invalid type: %d", type); + return NULL; + break; + } + if ((x = malloc(sz)) == NULL){ + clicon_err(OE_XML, errno, "malloc"); + return NULL; + } + memset(x, 0, sz); + xml_type_set(x, type); + if (name && (xml_name_set(x, name)) < 0) + return NULL; + if (xp){ + xml_parent_set(x, xp); + if (xml_child_append(xp, x) < 0) + return NULL; + x->_x_i = xml_child_nr(xp)-1; + } + _stats_nr++; + return x; +} + +#else /* XML_NEW_DIFFERENTIATE */ + cxobj * xml_new(char *name, cxobj *xp, @@ -1008,6 +1072,7 @@ xml_new(char *name, _stats_nr++; return x; } +#endif /* XML_NEW_DIFFERENTIATE */ /*! Create a new XML node and set it's body to a value * @@ -1045,48 +1110,6 @@ xml_new_body(char *name, return new_node; } -#ifdef NOTYET -/*! Create new xml node given a name and parent. Free with xml_free(). - */ -cxobj * -xml_new2(char *name, - cxobj *xp, - enum cxobj_type type) -{ - struct xml *x = NULL; - size_t sz; - - switch (type){ - case CX_ELMNT: - sz = sizeof(struct xml); - break; - case CX_ATTR: - case CX_BODY: - sz = sizeof(struct xmlbody); - break; - default: - clicon_err(OE_XML, EINVAL, "Invalid type: %d", type); - return NULL; - break; - } - if ((x = malloc(sz)) == NULL){ - clicon_err(OE_XML, errno, "malloc"); - return NULL; - } - memset(x, 0, sz); - xml_type_set(x, type); - if (name && (xml_name_set(x, name)) < 0) - return NULL; - if (xp){ - xml_parent_set(x, xp); - if (xml_child_append(xp, x) < 0) - return NULL; - x->_x_i = xml_child_nr(xp)-1; - } - _stats_nr++; - return x; -} -#endif /* NOTYET */ /*! Return yang spec of node. * Not necessarily set. Either has not been set yet (by xml_spec_set( or anyxml. diff --git a/lib/src/clixon_xml_io.c b/lib/src/clixon_xml_io.c index 8fc40061..ad696fcf 100644 --- a/lib/src/clixon_xml_io.c +++ b/lib/src/clixon_xml_io.c @@ -241,7 +241,6 @@ clicon_xml2cbuf(cbuf *cb, int hasbody; int haselement; char *namespace; - char *encstr = NULL; /* xml encoded string */ char *val; if (depth == 0) @@ -252,21 +251,27 @@ clicon_xml2cbuf(cbuf *cb, case CX_BODY: if ((val = xml_value(x)) == NULL) /* incomplete tree */ break; - if (xml_chardata_encode(&encstr, "%s", val) < 0) + if (xml_chardata_cbuf_append(cb, val) < 0) goto done; - cprintf(cb, "%s", encstr); break; case CX_ATTR: - cprintf(cb, " "); - if (namespace) - cprintf(cb, "%s:", namespace); + cbuf_append_str(cb, " "); + if (namespace){ + cbuf_append_str(cb, namespace); + cbuf_append_str(cb, ":"); + } cprintf(cb, "%s=\"%s\"", name, xml_value(x)); break; case CX_ELMNT: - cprintf(cb, "%*s<", prettyprint?(level*XML_INDENT):0, ""); - if (namespace) - cprintf(cb, "%s:", namespace); - cprintf(cb, "%s", name); + if (prettyprint) + cprintf(cb, "%*s<", level*XML_INDENT, ""); + else + cbuf_append_str(cb, "<"); + if (namespace){ + cbuf_append_str(cb, namespace); + cbuf_append_str(cb, ":"); + } + cbuf_append_str(cb, name); hasbody = 0; haselement = 0; xc = NULL; @@ -288,11 +293,11 @@ clicon_xml2cbuf(cbuf *cb, } /* Check for special case instead of */ if (hasbody==0 && haselement==0) - cprintf(cb, "/>"); + cbuf_append_str(cb, "/>"); else{ - cprintf(cb, ">"); + cbuf_append_str(cb, ">"); if (prettyprint && hasbody == 0) - cprintf(cb, "\n"); + cbuf_append_str(cb, "\n"); xc = NULL; while ((xc = xml_child_each(x, xc, -1)) != NULL) if (xml_type(xc) != CX_ATTR) @@ -300,13 +305,16 @@ clicon_xml2cbuf(cbuf *cb, goto done; if (prettyprint && hasbody == 0) cprintf(cb, "%*s", level*XML_INDENT, ""); - cprintf(cb, "", name); + cbuf_append_str(cb, ""); } if (prettyprint) - cprintf(cb, "\n"); + cbuf_append_str(cb, "\n"); break; default: break; @@ -314,8 +322,6 @@ clicon_xml2cbuf(cbuf *cb, ok: retval = 0; done: - if (encstr) - free(encstr); return retval; } @@ -465,10 +471,11 @@ _xml_parse(const char *str, } if (failed) goto fail; - /* This fails if xt is not bound to yang */ - /* Sort the complete tree after parsing. Sorting is less meaningful if Yang not bound */ - if (xml_apply0(xt, CX_ELMNT, xml_sort, NULL) < 0) - goto done; + /* Sort the complete tree after parsing. Sorting is not really meaningful if Yang + not bound */ + if (yb != YB_NONE) + if (xml_sort_recurse(xt) < 0) + goto done; retval = 1; done: clixon_xml_parsel_exit(&xy); diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index cc70623d..e3fd10be 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -79,6 +79,18 @@ #include "clixon_yang_type.h" #include "clixon_xml_map.h" +/* Local types + */ +/* Merge code needs a two-phase pass where objects subject to merge are first checked for, + * the actually inserted. + * This is to mitigate a search problem where objects inserted are among the ones checked for + */ +typedef struct { + cxobj *mt_x0c; + cxobj *mt_x1c; + yang_stmt *mt_yc; +} merge_twophase; + /*! Is attribute and is either of form xmlns="", or xmlns:x="" */ int isxmlns(cxobj *x) @@ -1264,6 +1276,7 @@ assign_namespace(cxobj *x0, /* source */ /*! Given a src element node x0 and a target node x1, assign (optional) prefix and namespace * @param[in] x0 Source XML tree * @param[in] x1 Target XML tree + * @param[in] x1p Target XML tree parent * @retval 0 OK * @retval -1 OK * 1. Find N=namespace(x0) @@ -1380,33 +1393,42 @@ xml_merge1(cxobj *x0, /* the target */ cxobj *x0c; /* base child */ cxobj *x0b; /* base body */ cxobj *x1c; /* mod child */ - char *x1name; char *x1bstr; /* mod body string */ yang_stmt *yc; /* yang child */ cbuf *cbr = NULL; /* Reason buffer */ int ret; int i; - struct { - cxobj *w_x0c; - cxobj *w_x1c; - yang_stmt *w_yc; - } *second_wave = NULL; - + merge_twophase *twophase = NULL; + int twophase_len; + assert(x1 && xml_type(x1) == CX_ELMNT); assert(y0); - x1name = xml_name(x1); + if (x0 == NULL){ + cvec *nsc = NULL; + cg_var *cv; + char *ns; + char *px; + nsc = cvec_dup(nscache_get_all(x1)); + if (xml_rm(x1) < 0) + goto done; + if (xml_insert(x0p, x1, INS_LAST, NULL, NULL) < 0) + goto done; + cv = NULL; + while ((cv = cvec_each(nsc, cv)) != NULL){ + px = cv_name_get(cv); + ns = cv_string_get(cv); + /* Check if it exists */ + if (xml2prefix(x1, ns, NULL) == 0) + if (xmlns_set(x1, px, ns) < 0) + goto done; + } + if (nsc) + cvec_free(nsc); + goto ok; + } if (yang_keyword_get(y0) == Y_LEAF_LIST || yang_keyword_get(y0) == Y_LEAF){ x1bstr = xml_body(x1); - if (x0==NULL){ - if ((x0 = xml_new(x1name, NULL, CX_ELMNT)) == NULL) - goto done; - xml_spec_set(x0, y0); - if (x1bstr){ /* empty type does not have body */ - if ((x0b = xml_new("body", x0, CX_BODY)) == NULL) - goto done; - } - } if (x1bstr){ if ((x0b = xml_body_get(x0)) == NULL){ if ((x0b = xml_new("body", x0, CX_BODY)) == NULL) @@ -1422,14 +1444,10 @@ xml_merge1(cxobj *x0, /* the target */ goto done; } /* if LEAF|LEAF_LIST */ else { /* eg Y_CONTAINER, Y_LIST */ - if (x0==NULL){ - if ((x0 = xml_new(x1name, NULL, CX_ELMNT)) == NULL) - goto done; - xml_spec_set(x0, y0); - } if (assign_namespace_element(x1, x0, x0p) < 0) goto done; - if ((second_wave = calloc(xml_child_nr(x1), sizeof(*second_wave))) == NULL){ + twophase_len = xml_child_nr(x1); + if ((twophase = calloc(twophase_len, sizeof(*twophase))) == NULL){ clicon_err(OE_UNIX, errno, "calloc"); goto done; } @@ -1457,37 +1475,37 @@ xml_merge1(cxobj *x0, /* the target */ x0c = NULL; if (yc && match_base_child(x0, x1c, yc, &x0c) < 0) goto done; - /* Save x0c, x1c, yc and merge in second wave, so that x1c entries "interfer" + /* Save x0c, x1c, yc and merge in second wave, so that x1c entries dont "interfer" * with itself, ie that later searches are among earlier objects already added * to x0 */ - second_wave[i].w_x0c = x0c; - second_wave[i].w_x1c = x1c; - second_wave[i].w_yc = yc; + twophase[i].mt_x0c = x0c; + twophase[i].mt_x1c = x1c; + twophase[i].mt_yc = yc; i++; } /* while */ + twophase_len = i; /* Inital length included non-elements */ /* Second run where actual merging is done * Loop through children of the modification tree */ - x1c = NULL; - i = 0; - while ((x1c = xml_child_each(x1, x1c, CX_ELMNT)) != NULL) { - if ((ret = xml_merge1(second_wave[i].w_x0c, - second_wave[i].w_yc, + for (i=0; i\r\n { clixon_xml_parselval.string = "\n"; _XY->xy_linenum++; return WHITESPACE; } \r { clixon_xml_parselval.string = "\n";return WHITESPACE; } \n { clixon_xml_parselval.string = "\n"; _XY->xy_linenum++;return WHITESPACE; } +[^&\r\n \t\<]+ { clixon_xml_parselval.string = yytext; return CHARDATA; /* Optimized */} . { clixon_xml_parselval.string = yytext; return CHARDATA; } /* @see xml_chardata_encode */ diff --git a/lib/src/clixon_xml_parse.y b/lib/src/clixon_xml_parse.y index 6cba0024..57df0e4b 100644 --- a/lib/src/clixon_xml_parse.y +++ b/lib/src/clixon_xml_parse.y @@ -83,6 +83,13 @@ #include "clixon_xml_sort.h" #include "clixon_xml_parse.h" +/* Enable for debugging, steals some cycles otherwise */ +#if 0 +#define _PARSE_DEBUG(s) clicon_debug(2,(s)) +#else +#define _PARSE_DEBUG(s) +#endif + void clixon_xml_parseerror(void *_xy, char *s) @@ -94,7 +101,9 @@ clixon_xml_parseerror(void *_xy, return; } -/* +/*! Parse XML content, eg chars between >...< + * @param[in] xy + * @param[in] str Body string, direct pointer (copy before use, dont free) * Note that we dont handle escaped characters correctly * there may also be some leakage here on NULL return */ @@ -325,39 +334,39 @@ xml_parse_attr(clixon_xml_yacc *xy, %% /* [1] document ::= prolog element Misc* */ document : prolog element misclist MY_EOF - { clicon_debug(2, "document->prolog element misc* ACCEPT"); + { _PARSE_DEBUG("document->prolog element misc* ACCEPT"); YYACCEPT; } | elist MY_EOF - { clicon_debug(2, "document->elist ACCEPT"); /* internal exception*/ + { _PARSE_DEBUG("document->elist ACCEPT"); /* internal exception*/ YYACCEPT; } ; /* [22] prolog ::= XMLDecl? Misc* (doctypedecl Misc*)? */ prolog : xmldcl misclist - { clicon_debug(2, "prolog->xmldcl misc*"); } + { _PARSE_DEBUG("prolog->xmldcl misc*"); } | misclist - { clicon_debug(2, "prolog->misc*"); } + { _PARSE_DEBUG("prolog->misc*"); } ; -misclist : misclist misc { clicon_debug(2, "misclist->misclist misc"); } - | { clicon_debug(2, "misclist->"); } +misclist : misclist misc { _PARSE_DEBUG("misclist->misclist misc"); } + | { _PARSE_DEBUG("misclist->"); } ; /* [27] Misc ::= Comment | PI | S */ -misc : comment { clicon_debug(2, "misc->comment"); } - | pi { clicon_debug(2, "misc->pi"); } - | WHITESPACE { clicon_debug(2, "misc->white space"); } +misc : comment { _PARSE_DEBUG("misc->comment"); } + | pi { _PARSE_DEBUG("misc->pi"); } + | WHITESPACE { _PARSE_DEBUG("misc->white space"); } ; xmldcl : BXMLDCL verinfo encodingdecl sddecl EQMARK - { clicon_debug(2, "xmldcl->verinfo encodingdecl? sddecl?"); } + { _PARSE_DEBUG("xmldcl->verinfo encodingdecl? sddecl?"); } ; verinfo : VER '=' '\"' STRING '\"' { if (xml_parse_version(_XY, $4) <0) YYABORT; - clicon_debug(2, "verinfo->version=\"STRING\"");} + _PARSE_DEBUG("verinfo->version=\"STRING\"");} | VER '=' '\'' STRING '\'' { if (xml_parse_version(_XY, $4) <0) YYABORT; - clicon_debug(2, "verinfo->version='STRING'");} + _PARSE_DEBUG("verinfo->version='STRING'");} ; encodingdecl : ENC '=' '\"' STRING '\"' {if ($4)free($4);} @@ -371,53 +380,53 @@ sddecl : SD '=' '\"' STRING '\"' {if ($4)free($4);} ; /* [39] element ::= EmptyElemTag | STag content ETag */ element : '<' qname attrs element1 - { clicon_debug(2, "element -> < qname attrs element1"); } + { _PARSE_DEBUG("element -> < qname attrs element1"); } ; qname : NAME { if (xml_parse_prefixed_name(_XY, NULL, $1) < 0) YYABORT; - clicon_debug(2, "qname -> NAME %s", $1);} + _PARSE_DEBUG("qname -> NAME");} | NAME ':' NAME { if (xml_parse_prefixed_name(_XY, $1, $3) < 0) YYABORT; - clicon_debug(2, "qname -> NAME : NAME");} + _PARSE_DEBUG("qname -> NAME : NAME");} ; element1 : ESLASH {_XY->xy_xelement = NULL; - clicon_debug(2, "element1 -> />");} + _PARSE_DEBUG("element1 -> />");} | '>' { xml_parse_endslash_pre(_XY); } elist { xml_parse_endslash_mid(_XY); } endtag { xml_parse_endslash_post(_XY); - clicon_debug(2, "element1 -> > elist endtag");} + _PARSE_DEBUG("element1 -> > elist endtag");} ; endtag : BSLASH NAME '>' - { clicon_debug(2, "endtag -> < "); + { _PARSE_DEBUG("endtag -> < "); if (xml_parse_bslash(_XY, NULL, $2) < 0) YYABORT; } | BSLASH NAME ':' NAME '>' { if (xml_parse_bslash(_XY, $2, $4) < 0) YYABORT; - clicon_debug(2, "endtag -> < "); } + _PARSE_DEBUG("endtag -> < "); } ; -elist : elist content { clicon_debug(2, "elist -> elist content"); } - | content { clicon_debug(2, "elist -> content"); } +elist : elist content { _PARSE_DEBUG("elist -> elist content"); } + | content { _PARSE_DEBUG("elist -> content"); } ; /* Rule 43 */ -content : element { clicon_debug(2, "content -> element"); } - | comment { clicon_debug(2, "content -> comment"); } - | pi { clicon_debug(2, "content -> pi"); } +content : element { _PARSE_DEBUG("content -> element"); } + | comment { _PARSE_DEBUG("content -> comment"); } + | pi { _PARSE_DEBUG("content -> pi"); } | CHARDATA { if (xml_parse_content(_XY, $1) < 0) YYABORT; - clicon_debug(2, "content -> CHARDATA %s", $1); } + _PARSE_DEBUG("content -> CHARDATA"); } | WHITESPACE { if (xml_parse_whitespace(_XY, $1) < 0) YYABORT; - clicon_debug(2, "content -> WHITESPACE %s", $1); } - | { clicon_debug(2, "content -> "); } + _PARSE_DEBUG("content -> WHITESPACE"); } + | { _PARSE_DEBUG("content -> "); } ; comment : BCOMMENT ECOMMENT ; -pi : BQMARK NAME EQMARK {clicon_debug(2, "pi -> "); free($2); } +pi : BQMARK NAME EQMARK {_PARSE_DEBUG("pi -> "); free($2); } | BQMARK NAME STRING EQMARK - { clicon_debug(2, "pi -> "); free($2); free($3);} + { _PARSE_DEBUG("pi -> "); free($2); free($3);} ; diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index 16ad03ce..c01b2be0 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -407,6 +407,30 @@ xml_sort(cxobj *x, return 0; } +/*! Recursively sort a tree + * Alt to use xml_apply + */ +int +xml_sort_recurse(cxobj *xn) +{ + int retval = -1; + cxobj *x; + int ret; + + x = NULL; + while ((x = xml_child_each(xn, x, CX_ELMNT)) != NULL) { + if ((ret = xml_sort(x, NULL)) < 0) + goto done; + if (ret == 1) /* This node is not sortable */ + break; + if (xml_sort_recurse(x) < 0) + goto done; + } + retval = 0; + done: + return retval; +} + /*! Special case search for ordered-by user or state data where linear sort is used * * @param[in] xp Parent XML node (go through its childre) diff --git a/lib/src/clixon_xpath_optimize.c b/lib/src/clixon_xpath_optimize.c index 2b3bc286..96ec90df 100644 --- a/lib/src/clixon_xpath_optimize.c +++ b/lib/src/clixon_xpath_optimize.c @@ -341,8 +341,10 @@ xpath_optimize_check(xpath_tree *xs, _optimize_hits++; return 1; /* Optimized */ } - else + else{ + clixon_xvec_free(xvec); return 0; /* use regular code */ + } #else return 0; /* use regular code */ #endif diff --git a/test/test_cli.sh b/test/test_cli.sh index 4d38da35..eec5a924 100755 --- a/test/test_cli.sh +++ b/test/test_cli.sh @@ -25,9 +25,9 @@ cat < $cfg $IETFRFC clixon-example /usr/local/lib/$APPNAME/backend - /usr/local/lib/$APPNAME/clispec - /usr/local/lib/$APPNAME/cli $APPNAME + /usr/local/lib/$APPNAME/cli + /usr/local/lib/$APPNAME/clispec /usr/local/var/$APPNAME/$APPNAME.sock /usr/local/var/$APPNAME/$APPNAME.pidfile /usr/local/var/$APPNAME diff --git a/test/test_perf_mem.sh b/test/test_perf_mem.sh index 896aa229..91335eea 100755 --- a/test/test_perf_mem.sh +++ b/test/test_perf_mem.sh @@ -1,11 +1,14 @@ #!/usr/bin/env bash # Backend Memory tests, footprint using the clixon-conf state statistics # Create a large datastore, load it and measure +# Baseline: (thinkpad laptop) running db: +# 100K objects: 500K mem: 74M +# 1M objects: 5M mem: 747M # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi -# ENable this for massif memory profiling +# Enable this for massif memory profiling #clixon_backend="valgrind --tool=massif clixon_backend" clixon_util_xpath=clixon_util_xpath @@ -92,12 +95,14 @@ testrun(){ pid=$(cat $pidfile) - new "netconf get state" - res=$(echo "]]>]]>" | $clixon_netconf -qf $cfg) + new "netconf get stats" + res=$(echo ']]>]]>' | $clixon_netconf -qf $cfg) + objects=$(echo "$res" | $clixon_util_xpath -p "/rpc-reply/global/xmlnr" | awk -F ">" '{print $2}' | awk -F "<" '{print $1}') echo "Total" - echo -n " objects: " - echo $res | $clixon_util_xpath -p "/rpc-reply/data/clixon-stats/global/xmlnr" | awk -F ">" '{print $2}' | awk -F "<" '{print $1}' + echo " objects: $objects" + +# if [ -f /proc/$pid/statm ]; then # This ony works on Linux # cat /proc/$pid/statm echo -n " mem: " @@ -105,7 +110,7 @@ testrun(){ fi for db in running candidate startup; do echo "$db" - resdb=$(echo "$res" | $clixon_util_xpath -p "/rpc-reply/data/clixon-stats/datastore[name=\"$db\"]") + resdb=$(echo "$res" | $clixon_util_xpath -p "/rpc-reply/datastore[name=\"$db\"]") resdb=${resdb#"nodeset:0:"} echo -n " objects: " echo $resdb | $clixon_util_xpath -p "datastore/nr" | awk -F ">" '{print $2}' | awk -F "<" '{print $1}' @@ -134,4 +139,3 @@ rm -rf $dir # unset conditional parameters unset perfnr - diff --git a/test/test_perf_state.sh b/test/test_perf_state.sh index 011520ba..bb4f82c8 100755 --- a/test/test_perf_state.sh +++ b/test/test_perf_state.sh @@ -3,7 +3,7 @@ # Config + state data, only get # Restconf/Netconf/CLI # Use mixed interfaces config+state -# ALso added two layers a/b to get extra depth (som caching can break) +# Also added two layers a/b to get extra depth (some caching can break) # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -50,35 +50,35 @@ EOF cat < $fyang module $APPNAME{ - yang-version 1.1; - prefix ex; - namespace "urn:example:clixon"; - container interfaces { + yang-version 1.1; + prefix ex; + namespace "urn:example:clixon"; + container interfaces { list a{ key "name"; leaf name { type string; } - container b{ - list interface { - key "name"; - leaf name { - type string; - } - leaf type { - type string; - } - leaf enabled { - type boolean; - default true; - } - leaf status { - type string; - config false; + container b{ + list interface { + key "name"; + leaf name { + type string; + } + leaf type { + type string; + } + leaf enabled { + type boolean; + default true; + } + leaf status { + type string; + config false; + } + } } } -} -} } } EOF diff --git a/test/test_perf_state_only.sh b/test/test_perf_state_only.sh index 82193cc5..1ba92855 100755 --- a/test/test_perf_state_only.sh +++ b/test/test_perf_state_only.sh @@ -2,7 +2,7 @@ # Scaling/ performance tests # State data only, in particular non-config lists (ie not state leafs on a config list) # Restconf/Netconf/CLI -# ALso added two layers a/b to get extra depth (som caching can break) +# Also added two layers a/b to get extra depth (som caching can break) # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -42,42 +42,41 @@ cat < $cfg example /usr/local/lib/example/cli /usr/local/lib/example/clispec - 0 ietf-netconf:startup EOF cat < $fyang module $APPNAME{ - yang-version 1.1; - prefix ex; - namespace "urn:example:clixon"; - container interfaces { + yang-version 1.1; + prefix ex; + namespace "urn:example:clixon"; + container interfaces { config false; list a{ key "name"; leaf name { type string; } - container b{ - list interface { - key "name"; - leaf name { - type string; - } - leaf type { - type string; - } - leaf enabled { - type boolean; - default true; - } - leaf status { - type string; + container b{ + list interface { + key "name"; + leaf name { + type string; + } + leaf type { + type string; + } + leaf enabled { + type boolean; + default true; + } + leaf status { + type string; + } + } } } -} -} } } EOF @@ -113,22 +112,9 @@ start_restconf -f $cfg new "waiting" wait_restconf -if false; then -new "generate 'large' config with $perfnr list entries" -echo -n "foo" > $fconfig -for (( i=0; i<$perfnr; i++ )); do - echo -n "e$iex:eth" >> $fconfig -done -echo "]]>]]>" >> $fconfig - -# Now take large config file and write it via netconf to candidate -new "netconf write large config" -expecteof_file "time -p $clixon_netconf -qf $cfg" 0 "$fconfig" "^]]>]]>$" 2>&1 | awk '/real/ {print $2}' - -# Now commit it from candidate to running -new "netconf commit large config" -expecteof "time -p $clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" 2>&1 | awk '/real/ {print $2}' -fi +new "cli get large config" +echo "$clixon_cli -1f $cfg show state xml" +$TIMEFN $clixon_cli -1f $cfg show state xml 2>&1 | awk '/real/ {print $2}' # START actual tests # Having a large db, get single entries many times @@ -176,6 +162,7 @@ new "cli get $perfreq single reqs" $clixon_cli -1 -f $cfg show state xml interfaces a b interface e$rnd > /dev/null done } 2>&1 | awk '/real/ {print $2}' fi + # Get config in one large get new "netconf get large config" { time -p echo " ]]>]]>" | $clixon_netconf -qf $cfg > /tmp/netconf; } 2>&1 | awk '/real/ {print $2}' @@ -184,24 +171,22 @@ new "restconf get large config" $TIMEFN curl -sG http://localhost/restconf/data/example:interfaces/a=foo/b 2>&1 | awk '/real/ {print $2}' new "cli get large config" -$TIMEFN $clixon_cli -1f $cfg show state xml interfaces a foo b 2>&1 | awk '/real/ {print $2}' +$TIMEFN $clixon_cli -1f $cfg show state xml 2>&1 | awk '/real/ {print $2}' new "Kill restconf daemon" stop_restconf -if [ $BE -eq 0 ]; then - exit # BE +if [ $BE -ne 0 ]; then + new "Kill backend" + # Check if premature kill + pid=$(pgrep -u root -f clixon_backend) + if [ -z "$pid" ]; then + err "backend already dead" + fi + # kill backend + stop_backend -f $cfg fi -new "Kill backend" -# Check if premature kill -pid=$(pgrep -u root -f clixon_backend) -if [ -z "$pid" ]; then - err "backend already dead" -fi -# kill backend -stop_backend -f $cfg - rm -rf $dir # unset conditional parameters diff --git a/test/test_perf_xml.sh b/test/test_perf_xml.sh index 8843c0fc..c2f1ac3b 100755 --- a/test/test_perf_xml.sh +++ b/test/test_perf_xml.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Test: XML performance test +# Test: XML performance test (CDATA test only) # See https://github.com/clicon/clixon/issues/96 # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi diff --git a/test/test_restconf.sh b/test/test_restconf.sh index e046ceee..c6449270 100755 --- a/test/test_restconf.sh +++ b/test/test_restconf.sh @@ -193,16 +193,16 @@ new "restconf Add interfaces subtree eth/0/0 using POST" expectpart "$(curl -si -X POST http://localhost/restconf/data/ietf-interfaces:interfaces -H "Content-Type: application/yang-data+json" -d '{"ietf-interfaces:interface":{"name":"eth/0/0","type":"clixon-example:eth","enabled":true}}')" 0 "HTTP/1.1 201 Created" new "restconf Check eth/0/0 added config" -expectpart "$(curl -s -X GET -H 'Accept: application/yang-data+json' http://localhost/restconf/data/ietf-interfaces:interfaces)" 0 '{"ietf-interfaces:interfaces":{"interface":\[{"name":"eth/0/0","type":"clixon-example:eth","enabled":true,"oper-status":"up","clixon-example:my-status":{"int":42,"str":"foo"}}\]}}' +expectpart "$(curl -si -X GET -H 'Accept: application/yang-data+json' http://localhost/restconf/data/ietf-interfaces:interfaces)" 0 'HTTP/1.1 200 OK' '{"ietf-interfaces:interfaces":{"interface":\[{"name":"eth/0/0","type":"clixon-example:eth","enabled":true,"oper-status":"up","clixon-example:my-status":{"int":42,"str":"foo"}}\]}}' new "restconf Check eth/0/0 GET augmented state level 1" -expectpart "$(curl -s -X GET -H 'Accept: application/yang-data+json' http://localhost/restconf/data/ietf-interfaces:interfaces/interface=eth%2f0%2f0)" 0 '{"ietf-interfaces:interface":\[{"name":"eth/0/0","type":"clixon-example:eth","enabled":true,"oper-status":"up","clixon-example:my-status":{"int":42,"str":"foo"}}\]}' +expectpart "$(curl -si -X GET -H 'Accept: application/yang-data+json' http://localhost/restconf/data/ietf-interfaces:interfaces/interface=eth%2f0%2f0)" 0 'HTTP/1.1 200 OK' '{"ietf-interfaces:interface":\[{"name":"eth/0/0","type":"clixon-example:eth","enabled":true,"oper-status":"up","clixon-example:my-status":{"int":42,"str":"foo"}}\]}' new "restconf Check eth/0/0 GET augmented state level 2" -expectpart "$(curl -s -X GET -H 'Accept: application/yang-data+json' http://localhost/restconf/data/ietf-interfaces:interfaces/interface=eth%2f0%2f0/clixon-example:my-status)" 0 '{"clixon-example:my-status":{"int":42,"str":"foo"}}' +expectpart "$(curl -si -X GET -H 'Accept: application/yang-data+json' http://localhost/restconf/data/ietf-interfaces:interfaces/interface=eth%2f0%2f0/clixon-example:my-status)" 0 'HTTP/1.1 200 OK' '{"clixon-example:my-status":{"int":42,"str":"foo"}}' -new "restconf Check eth/0/0 added state" -expectpart "$(curl -s -X GET -H 'Accept: application/yang-data+json' http://localhost/restconf/data/clixon-example:state)" 0 '{"clixon-example:state":{"op":\["41","42","43"\]}}' +new "restconf Check eth/0/0 added state XXXXXXX" +expectpart "$(curl -si -X GET -H 'Accept: application/yang-data+json' http://localhost/restconf/data/clixon-example:state)" 0 'HTTP/1.1 200 OK' '{"clixon-example:state":{"op":\["41","42","43"\]}}' new "restconf Re-post eth/0/0 which should generate error" expectpart "$(curl -s -X POST -H "Content-Type: application/yang-data+json" -d '{"ietf-interfaces:interface":{"name":"eth/0/0","type":"clixon-example:eth","enabled":true}}' http://localhost/restconf/data/ietf-interfaces:interfaces)" 0 '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"data-exists","error-severity":"error","error-message":"Data already exists; cannot create new resource"}}} ' diff --git a/test/test_xml_trees.sh b/test/test_xml_trees.sh index 47122597..3947b5d1 100755 --- a/test/test_xml_trees.sh +++ b/test/test_xml_trees.sh @@ -89,12 +89,11 @@ testrun insert "$x0a1$x0b" "$x0a42$x0b" c 0 '1" $p 0 '1' -new "patrse parent element" -testrun parent "$x0a2$x0b" '1' $p 0 '12' - +new "parse parent element" +testrun parent "$x0a2$x0b" '1' $p 0 '21' new "parse parent container" -testrun parent "$x0a1$x0b" '42' c 0 '421' +testrun parent "$x0a1$x0b" '42' c 0 '142' # -------- merge