From 29235d5547f96b89f5437fe2106c5c9c4adaa085 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Mon, 11 May 2020 15:29:58 +0200 Subject: [PATCH] Removed and enabled code for compile options: RESTART_PLUGIN_RPC, XML_NEW_DIFFERENTIATE, OPTIMIZE_45_BIND and OPTIMIZE_45_SORT --- CHANGELOG.md | 34 +++++++++++++++------------ apps/backend/backend_client.c | 4 ---- apps/backend/backend_commit.c | 3 +-- apps/backend/backend_commit.h | 2 -- include/clixon_custom.h | 20 ---------------- lib/src/clixon_xml.c | 43 ++++------------------------------- lib/src/clixon_xml_bind.c | 20 +--------------- lib/src/clixon_xml_nsctx.c | 4 +--- lib/src/clixon_xml_sort.c | 9 -------- 9 files changed, 27 insertions(+), 112 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f976799..8d85e4ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,13 +35,13 @@ Expected: May 2020 * ca_trans_commit_done: Called when all plugin commits have been done. * Note: If you have used "end" callback and usign transaction data, you should probably use this instead. -### API changes on existing protocol/config features (You may have have to change how you use Clixon) +### API changes on existing protocol/config features (For users, you may have have to change how you use Clixon) * Stricter validation detecting duplicate container or leaf in XML. * Eg `` is invalid if `a` is a leaf or container. * New clixon-lib@2020-04-23.yang revision - * Added: stats RPC for clixon XML and memory statistics. - * Added: restart-plugin RPC for restarting individual plugins without restarting backend. + * New RPC: `stats` for clixon XML and memory statistics. + * New RPC: `restart-plugin` for restarting individual plugins without restarting backend. * New clixon-config@2020-04-23.yang revision * Removed xml-stats non-config data (replaced by rpc `stats` in clixon-lib.yang) * Added option `CLICON_YANG_UNKNOWN_ANYDATA` to treat unknown XML (wrt YANG) as anydata. @@ -49,10 +49,13 @@ Expected: May 2020 * Stricter incoming RPC sanity checking, error messages may have changed. * Changed output of `clixon_cli -G` option to show generated CLI spec original text instead of resulting parse-tree, which gives better detail from a debugging perspective. -### C-API changes on existing features (you may need to change your plugin C-code) +### C-API changes on existing features (For developers: you may need to change your plugin C-code) -* Length of xml vector in many structs changed from `size_t` to `int`since it is a vector size, not byte size. This includes `transaction_data_t` +* Length of xml vector in many structs changed from `size_t` to `int`since it is a vector size, not byte size. + * Example: `transaction_data_t` * `xml_merge()` changed to use 3-value return: 1:OK, 0:Yang failed, -1: Error +* Some function changed to use 3-value return: 1:OK, 0:Yang failed, -1: Error. + * Example: `xml_merge()` * `clixon_netconf_error(category, xerr, msg, arg)` removed first argument -> `clixon_netconf_error(xerr, msg, arg)` * CLI * `clicon_parse()`: Changed signature due to new cligen error and result handling: @@ -62,20 +65,23 @@ Expected: May 2020 * Added decriptive error message when plugins produce invalid state XML. * Example: `operation-failedmystateNo such yang module. Internal error, state callback returned invalid XML: example_backend` + * Such a message means there is something wrong in the internal plugins or elsewehere, ie it is not a proper end-user error. * Compile-time option: `USE_CLIGEN44` for running clixon-45 with cligen-44. * Temporary fix since cligen-45 have some non-backward compatible behaviour. -* Optimizations - * Reduced memory for attribute and body objects, see `XML_NEW_DIFFERENTIATE` compile-time option. - * Optimized prefix checks at xml parse time: using many prefixes slowed down parsing considerably - * Optimized cbuf handling in parsing and xml2cbuf functions. +* Cycle optimizations + * Optimized namespace prefix checks at xml parse time: using many prefixes slowed down parsing considerably + * Optimized cbuf handling in parsing and xml2cbuf functions: use of new `cbuf_append()` function. * Optimized xml scanner to read strings rather than single chars - * Optimized xml_merge for the case of disjunct trees. - * Cleared startup-db cache after restart - * Experimental optimzations of yang-bind and sort for large lists - * Enabled by compile-time options: `OPTIMIZE_45_BIND` and `OPTIMIZE_45_SORT` + * Identify early that trees are disjunct instead of recursively merge in `xml_merge` + * Optimizations of `yang_bind` for large lists: use a "sibling/template" to use same binding as previous element. +* Memory optimizations + * Reduced memory for attribute and body objects, by allocating less memory in `xml_new()` than for elements, reducing XML storage with ca 25% + * Cleared startup-db cache after restart, slashing datastore cache with (best-case) a third. + * Removed nameserver binding cache for leaf/leaf-list objects. + * Remove xml value cache after sorting (just use cligen value cache at sorting, remove after use) * Adapted to CLIgen 4.5 API changes, eg: `cliread()` and `cliread_parse()` * Renamed utility function `clixon_util_insert()` to `clixon_util_xml_mod()` and added merge functionality. -* Sanity check of duplicates prefixes in Yang modules and submodules as defined in RFC 7950 Sec 7.1.4 +* Sanity check of duplicate prefixes in Yang modules and submodules as defined in RFC 7950 Sec 7.1.4 ### Corrected Bugs diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 5bcc0e14..98474914 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -1425,7 +1425,6 @@ from_client_stats(clicon_handle h, return retval; } -#ifdef RESTART_PLUGIN_RPC /*! Request restart of specific plugins * @param[in] h Clicon handle * @param[in] xe Request: @@ -1472,7 +1471,6 @@ from_client_restart_plugin(clicon_handle h, free(vec); return retval; } -#endif /* RESTART_PLUGIN_RPC */ /*! Verify nacm user with peer uid credentials * @param[in] mode Peer credential mode: none, exact or except @@ -1838,11 +1836,9 @@ backend_rpc_init(clicon_handle h) if (rpc_callback_register(h, from_client_stats, NULL, CLIXON_LIB_NS, "stats") < 0) goto done; -#ifdef RESTART_PLUGIN_RPC if (rpc_callback_register(h, from_client_restart_plugin, NULL, CLIXON_LIB_NS, "restart-plugin") < 0) goto done; -#endif retval =0; done: return retval; diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index 7f917fad..c020dc90 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -810,7 +810,6 @@ from_client_validate(clicon_handle h, return retval; } /* from_client_validate */ -#ifdef RESTART_PLUGIN_RPC /*! Restart specific backend plugins without full backend restart * Note, depending on plugin callbacks, there may be other dependencies which may make this * difficult in the general case. @@ -928,4 +927,4 @@ from_client_restart_one(clicon_handle h, retval = 0; goto done; } -#endif /* RESTART_PLUGIN_RPC */ + diff --git a/apps/backend/backend_commit.h b/apps/backend/backend_commit.h index 8ee85a7b..8601bfce 100644 --- a/apps/backend/backend_commit.h +++ b/apps/backend/backend_commit.h @@ -48,8 +48,6 @@ int from_client_commit(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void int from_client_discard_changes(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg); int from_client_cancel_commit(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg); int from_client_validate(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg); -#ifdef RESTART_PLUGIN_RPC int from_client_restart_one(clicon_handle h, clixon_plugin *cp, cbuf *cbret); -#endif #endif /* _BACKEND_COMMIT_H_ */ diff --git a/include/clixon_custom.h b/include/clixon_custom.h index e49f8850..57913560 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -101,26 +101,6 @@ */ #define STATE_ORDERED_BY_SYSTEM -/*! Restart specific backend plugins - * Note, depending on plugin callbacks, there may be other dependencies which may make this - * difficult in the general case. - */ -#define RESTART_PLUGIN_RPC - -/*! Differentiate creating XML object body/element vs elenmet to reduce space - */ -#define XML_NEW_DIFFERENTIATE - -/*! Clixon 4.5 optimizing experiments for yang bind - * Primarily for large lists - */ -#define OPTIMIZE_45_BIND - -/*! Clixon 4.5 optimizing experiments for sorting yang-bound XML trees - * Primarily for large lists - */ -#define OPTIMIZE_45_SORT - /*! Use cligen 4.4 instead of master / cligen 4.5.pre * Temporary fix */ diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index feed862d..02baef8c 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -158,6 +158,7 @@ struct search_index{ * - Local name: In either case the "local name" is N (also "prefix") * It is this combination of the universally managed URI namespace with the * vocabulary's local names that is effective in avoiding name clashes. + * @see struct xmlbody For XML body and attributes */ struct xml{ enum cxobj_type x_type; /* type of node: element, attribute, body */ @@ -186,9 +187,8 @@ struct xml{ #endif }; - -#ifdef XML_NEW_DIFFERENTIATE -/* This is experimental variant of struct xml for use by non-elements to save space +/* Variant of struct xml for use by non-elements to save space + * @see struct xml For XML elements */ struct xmlbody{ enum cxobj_type xb_type; /* type of node: element, attribute, body */ @@ -201,7 +201,6 @@ struct xmlbody{ see xml_enumerate and xml_cmp */ cbuf *xb_value_cb; /* attribute and body nodes have values */ }; -#endif /* XML_NEW_DIFFERENTIATE */ /* * Variables @@ -277,11 +276,7 @@ xml_stats_one(cxobj *x, break; case CX_BODY: case CX_ATTR: -#ifdef XML_NEW_DIFFERENTIATE sz += sizeof(struct xmlbody); -#else - sz += sizeof(struct xml); -#endif if (x->x_value_cb) sz += cbuf_buflen(x->x_value_cb); break; @@ -1042,10 +1037,9 @@ xml_childvec_get(cxobj *x) * ... * xml_free(x); * @endcode + * @note Differentiates between body/attribute vs element to reduce mem allocation * @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, @@ -1085,35 +1079,6 @@ xml_new(char *name, return x; } -#else /* XML_NEW_DIFFERENTIATE */ - -cxobj * -xml_new(char *name, - cxobj *xp, - enum cxobj_type type) -{ - cxobj *x; - - if ((x = malloc(sizeof(cxobj))) == NULL){ - clicon_err(OE_XML, errno, "malloc"); - return NULL; - } - memset(x, 0, sizeof(cxobj)); - xml_type_set(x, type); - if (name != NULL && - 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 /* XML_NEW_DIFFERENTIATE */ - /*! Create a new XML node and set it's body to a value * * @param[in] name The name of the new node diff --git a/lib/src/clixon_xml_bind.c b/lib/src/clixon_xml_bind.c index 3bb46ab3..be691797 100644 --- a/lib/src/clixon_xml_bind.c +++ b/lib/src/clixon_xml_bind.c @@ -128,9 +128,7 @@ strip_whitespace(cxobj *xt) */ static int populate_self_parent(cxobj *xt, -#ifdef OPTIMIZE_45_BIND cxobj *xsibling, -#endif cxobj **xerr) { int retval = -1; @@ -143,14 +141,12 @@ populate_self_parent(cxobj *xt, cbuf *cb = NULL; name = xml_name(xt); -#ifdef OPTIMIZE_45_BIND /* optimization for massive lists - use the first element as role model */ if (xsibling && xml_child_nr_type(xt, CX_ATTR) == 0){ y = xml_spec(xsibling); goto set; } -#endif xp = xml_parent(xt); if (xp == NULL){ if (xerr && @@ -202,9 +198,7 @@ populate_self_parent(cxobj *xt, goto done; goto fail; } -#ifdef OPTIMIZE_45_BIND set: -#endif xml_spec_set(xt, y); #ifdef XML_EXPLICIT_INDEX if (xml_search_index_p(xt)) @@ -354,7 +348,6 @@ xml_bind_yang(cxobj *xt, goto done; } -#ifdef OPTIMIZE_45_BIND int xml_bind_yang0_opt(cxobj *xt, yang_bind yb, @@ -424,8 +417,6 @@ xml_bind_yang0_opt(cxobj *xt, retval = 0; goto done; } -#endif /* OPTIMIZE_45_BIND */ - /*! Find yang spec association of tree of XML nodes * @@ -455,11 +446,7 @@ xml_bind_yang0(cxobj *xt, goto done; break; case YB_PARENT: - if ((ret = populate_self_parent(xt, -#ifdef OPTIMIZE_45_BIND - NULL, -#endif - xerr)) < 0) + if ((ret = populate_self_parent(xt, NULL, xerr)) < 0) goto done; break; case YB_NONE: @@ -477,13 +464,8 @@ xml_bind_yang0(cxobj *xt, strip_whitespace(xt); xc = NULL; /* Apply on children */ while ((xc = xml_child_each(xt, xc, CX_ELMNT)) != NULL) { -#ifdef OPTIMIZE_45_BIND if ((ret = xml_bind_yang0_opt(xc, YB_PARENT, NULL, xerr)) < 0) goto done; -#else - if ((ret = xml_bind_yang0(xc, YB_PARENT, NULL, xerr)) < 0) - goto done; -#endif if (ret == 0) failed++; } diff --git a/lib/src/clixon_xml_nsctx.c b/lib/src/clixon_xml_nsctx.c index 61fd863b..fadd1e58 100644 --- a/lib/src/clixon_xml_nsctx.c +++ b/lib/src/clixon_xml_nsctx.c @@ -464,9 +464,7 @@ xml2ns(cxobj *x, * If not, this is devastating when populating deep yang structures */ if (ns && -#ifdef OPTIMIZE_45_BIND /* Dont set cache if few children: if 1 child typically a body */ - xml_child_nr(x) > 1 && -#endif + xml_child_nr(x) > 1 && /* Dont set cache if few children: if 1 child typically a body */ nscache_set(x, prefix, ns) < 0) goto done; ok: diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index 6ded7f9a..ca91b503 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -143,7 +143,6 @@ xml_cv_cache(cxobj *x, return retval; } -#ifdef OPTIMIZE_45_SORT static int xml_cv_cache_clear(cxobj *xt) { @@ -157,7 +156,6 @@ xml_cv_cache_clear(cxobj *xt) done: return retval; } -#endif /* OPTIMIZE_45_SORT */ /*! Help function to qsort for sorting entries in xml child vector same parent * @param[in] x1 object 1 @@ -433,7 +431,6 @@ xml_sort_recurse(cxobj *xn) cxobj *x; int ret; -#ifdef OPTIMIZE_45_SORT ret = xml_sort_verify(xn, NULL); if (ret == 1) /* This node is not sortable */ goto ok; @@ -445,12 +442,6 @@ xml_sort_recurse(cxobj *xn) } if (xml_cv_cache_clear(xn) < 0) goto done; -#else - if ((ret = xml_sort(xn)) < 0) - goto done; - if (ret == 1) /* This node is not sortable */ - goto ok; -#endif x = NULL; while ((x = xml_child_each(xn, x, CX_ELMNT)) != NULL) { if (xml_sort_recurse(x) < 0)