Refactoring for better performance of large auto-cli specs

* Fixed: [very slow execution of load_set_file #288](https://github.com/clicon/clixon/issues/288)
* New `clixon-lib@2021-11-11.yang` revision
  * Modified option: RPC stats extended with YANG stats
* Modified `clixon-config@2021-11-11.yang` revision
  * Added option:
    * CLICON_PLUGIN_CALLBACK_CHECK
    * Enable to make plugin context check before and after all callbacks.
* Added statistics for YANG: number of objects and memory used
* Use the treeref no-copy option of CLIgen to reduce memory
* Refactored cli-generation/autocli-start code
* Refactored cligen glue functions to use cligen_eval directly (remove clicon_eval,clixon_cligen_eval)
This commit is contained in:
Olof hagsand 2021-11-25 12:04:05 +01:00
parent b91ce762d5
commit 5388aace12
29 changed files with 760 additions and 451 deletions

View file

@ -63,6 +63,7 @@
#include "clixon_file.h"
#include "clixon_handle.h"
#include "clixon_yang.h"
#include "clixon_options.h"
#include "clixon_xml.h"
#include "clixon_xml_nsctx.h"
#include "clixon_yang_module.h"
@ -342,7 +343,7 @@ plugin_load_one(clicon_handle h,
clixon_plugin_t *cp = NULL;
char *name;
char *p;
plugin_context_t *pc = NULL;
void *wh = NULL;
clicon_debug(1, "%s file:%s function:%s", __FUNCTION__, file, function);
dlerror(); /* Clear any existing error */
@ -361,9 +362,9 @@ plugin_load_one(clicon_handle h,
goto done;
}
clicon_err_reset();
if ((pc = plugin_context_get()) < 0)
wh = NULL;
if (plugin_context_check(h, &wh, file, __FUNCTION__) < 0)
goto done;
if ((api = initfn(h)) == NULL) {
if (!clicon_errno){ /* if clicon_err() is not called then log and continue */
clicon_log(LOG_DEBUG, "Warning: failed to initiate %s", strrchr(file,'/')?strchr(file, '/'):file);
@ -375,7 +376,7 @@ plugin_load_one(clicon_handle h,
goto done;
}
}
if (plugin_context_check(pc, file, __FUNCTION__) < 0)
if (plugin_context_check(h, &wh, file, __FUNCTION__) < 0)
goto done;
/* Note: sizeof clixon_plugin_api which is largest of clixon_plugin_api:s */
@ -401,8 +402,8 @@ plugin_load_one(clicon_handle h,
retval = 1;
done:
clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);
if (pc)
free(pc);
if (wh != NULL)
free(wh);
if (retval != 1 && handle)
dlclose(handle);
if (cp)
@ -510,7 +511,7 @@ done:
* @retval NULL Error
* @see plugin_context_check
* */
plugin_context_t *
static void *
plugin_context_get(void)
{
int i;
@ -521,7 +522,6 @@ plugin_context_get(void)
goto done;
}
memset(pc, 0, sizeof(*pc));
if (sigprocmask(0, NULL, &pc->pc_sigset) < 0){
clicon_err(OE_UNIX, errno, "sigprocmask");
goto done;
@ -553,30 +553,53 @@ plugin_context_get(void)
/*! Given an existing, old plugin context, check if anything has changed
*
* Make a new check and compare with the old (procided as in-parameter).
* Called twice:
* 1) Make a check of resources
* 2) Make a new check and compare with the old check, return 1 on success, 0 on fail
* Log if there is a difference at loglevel WARNING.
* You can modify the code to also fail with assert if you want early fail.
* Controlled by option
*
* @param[in,out] oldpc Old plugin context
* @param[in] h Clixon handle
* @param[in,out] wh Either: NULL for init, will be assigned, OR previous handle (will be freed)
* @param[in] name Name of plugin for logging. Can be other name, context dependent
* @param[in] fn Typically name of callback, or caller function
* @retval -1 Error
* @retval 0 Fail, log on syslog using LOG_WARNING
* @retval 1 OK
* @note Only logs error, does not generate error
* @note name and fn are context dependent, since the env of callback calls are very different
* @see plugin_context_get
* @see CLICON_PLUGIN_CALLBACK_CHECK Enable to activate these checks
*/
int
plugin_context_check(plugin_context_t *oldpc0,
const char *name,
const char *fn)
plugin_context_check(clicon_handle h,
void **wh,
const char *name,
const char *fn)
{
int retval = -1;
int failed = 0;
int i;
struct plugin_context *oldpc = oldpc0;
struct plugin_context *oldpc;
struct plugin_context *newpc = NULL;
if (h == NULL){
errno = EINVAL;
return -1;
}
/* Check if plugion checks are enabled */
if (!clicon_option_bool(h, "CLICON_PLUGIN_CALLBACK_CHECK"))
return 1;
if (wh == NULL){
errno = EINVAL;
return -1;
}
if (*wh == NULL){
*wh = plugin_context_get();
return 1;
}
oldpc = (struct plugin_context *)*wh;
if ((newpc = plugin_context_get()) == NULL)
goto done;
if (oldpc->pc_termios.c_iflag != newpc->pc_termios.c_iflag){
@ -644,11 +667,14 @@ plugin_context_check(plugin_context_t *oldpc0,
}
if (failed)
goto fail;
retval = 1; /* OK */
done:
if (newpc)
free(newpc);
if (oldpc)
free(oldpc);
if (wh && *wh)
*wh = NULL;
return retval;
fail:
retval = 0;
@ -665,12 +691,13 @@ int
clixon_plugin_start_one(clixon_plugin_t *cp,
clicon_handle h)
{
int retval = -1;
plgstart_t *fn; /* Plugin start */
plugin_context_t *pc = NULL;
int retval = -1;
plgstart_t *fn; /* Plugin start */
void *wh = NULL;
if ((fn = cp->cp_api.ca_start) != NULL){
if ((pc = plugin_context_get()) == NULL)
wh = NULL;
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
if (fn(h) < 0) {
if (clicon_errno < 0)
@ -678,13 +705,11 @@ clixon_plugin_start_one(clixon_plugin_t *cp,
__FUNCTION__, cp->cp_name);
goto done;
}
if (plugin_context_check(pc, cp->cp_name, __FUNCTION__) < 0)
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
}
retval = 0;
done:
if (pc)
free(pc);
return retval;
}
@ -719,13 +744,14 @@ static int
clixon_plugin_exit_one(clixon_plugin_t *cp,
clicon_handle h)
{
int retval = -1;
char *error;
plgexit_t *fn;
plugin_context_t *pc = NULL;
int retval = -1;
char *error;
plgexit_t *fn;
void *wh = NULL;
if ((fn = cp->cp_api.ca_exit) != NULL){
if ((pc = plugin_context_get()) == NULL)
wh = NULL;
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
if (fn(h) < 0) {
if (clicon_errno < 0)
@ -733,7 +759,7 @@ clixon_plugin_exit_one(clixon_plugin_t *cp,
__FUNCTION__, cp->cp_name);
goto done;
}
if (plugin_context_check(pc, cp->cp_name, __FUNCTION__) < 0)
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
if (dlclose(cp->cp_handle) != 0) {
error = (char*)dlerror();
@ -742,8 +768,6 @@ clixon_plugin_exit_one(clixon_plugin_t *cp,
}
retval = 0;
done:
if (pc)
free(pc);
return retval;
}
@ -792,13 +816,14 @@ clixon_plugin_auth_one(clixon_plugin_t *cp,
clixon_auth_type_t auth_type,
char **authp)
{
int retval = -1;
plgauth_t *fn; /* Plugin auth */
plugin_context_t *pc = NULL;
int retval = -1;
plgauth_t *fn; /* Plugin auth */
void *wh = NULL;
clicon_debug(1, "%s", __FUNCTION__);
if ((fn = cp->cp_api.ca_auth) != NULL){
if ((pc = plugin_context_get()) == NULL)
wh = NULL;
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
if ((retval = fn(h, req, auth_type, authp)) < 0) {
if (clicon_errno < 0)
@ -806,14 +831,12 @@ clixon_plugin_auth_one(clixon_plugin_t *cp,
__FUNCTION__, cp->cp_name);
goto done;
}
if (plugin_context_check(pc, cp->cp_name, __FUNCTION__) < 0)
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
}
else
retval = 0; /* Ignored / no callback */
done:
if (pc)
free(pc);
clicon_debug(1, "%s retval:%d auth:%s", __FUNCTION__, retval, *authp);
return retval;
}
@ -877,12 +900,13 @@ clixon_plugin_extension_one(clixon_plugin_t *cp,
yang_stmt *yext,
yang_stmt *ys)
{
int retval = 1;
plgextension_t *fn; /* Plugin extension fn */
plugin_context_t *pc = NULL;
int retval = 1;
plgextension_t *fn; /* Plugin extension fn */
void *wh = NULL;
if ((fn = cp->cp_api.ca_extension) != NULL){
if ((pc = plugin_context_get()) == NULL)
wh = NULL;
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
if (fn(h, yext, ys) < 0) {
if (clicon_errno < 0)
@ -890,13 +914,11 @@ clixon_plugin_extension_one(clixon_plugin_t *cp,
__FUNCTION__, cp->cp_name);
goto done;
}
if (plugin_context_check(pc, cp->cp_name, __FUNCTION__) < 0)
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
}
retval = 0;
done:
if (pc)
free(pc);
return retval;
}
@ -950,10 +972,11 @@ clixon_plugin_datastore_upgrade_one(clixon_plugin_t *cp,
{
int retval = -1;
datastore_upgrade_t *fn;
plugin_context_t *pc = NULL;
void *wh = NULL;
if ((fn = cp->cp_api.ca_datastore_upgrade) != NULL){
if ((pc = plugin_context_get()) == NULL)
wh = NULL;
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
if (fn(h, db, xt, msd) < 0) {
if (clicon_errno < 0)
@ -961,13 +984,11 @@ clixon_plugin_datastore_upgrade_one(clixon_plugin_t *cp,
__FUNCTION__, cp->cp_name);
goto done;
}
if (plugin_context_check(pc, cp->cp_name, __FUNCTION__) < 0)
if (plugin_context_check(h, &wh, cp->cp_name, __FUNCTION__) < 0)
goto done;
}
retval = 0;
done:
if (pc)
free(pc);
return retval;
}
@ -1121,7 +1142,7 @@ rpc_callback_call(clicon_handle h,
char *ns;
int nr = 0; /* How many callbacks */
plugin_module_struct *ms = plugin_module_struct_get(h);
plugin_context_t *pc = NULL;
void *wh = NULL;
int ret;
if (ms == NULL){
@ -1136,19 +1157,18 @@ rpc_callback_call(clicon_handle h,
if (strcmp(rc->rc_name, name) == 0 &&
ns && rc->rc_namespace &&
strcmp(rc->rc_namespace, ns) == 0){
if ((pc = plugin_context_get()) == NULL)
wh = NULL;
if (plugin_context_check(h, &wh, rc->rc_name, __FUNCTION__) < 0)
goto done;
if (rc->rc_callback(h, xe, cbret, arg, rc->rc_arg) < 0){
clicon_debug(1, "%s Error in: %s", __FUNCTION__, rc->rc_name);
if (plugin_context_check(h, &wh, rc->rc_name, __FUNCTION__) < 0)
goto done;
goto done;
}
nr++;
if (plugin_context_check(pc, rc->rc_name, __FUNCTION__) < 0)
if (plugin_context_check(h, &wh, rc->rc_name, __FUNCTION__) < 0)
goto done;
if (pc){
free(pc);
pc = NULL;
}
}
rc = NEXTQ(rpc_callback_t *, rc);
} while (rc != ms->ms_rpc_callbacks);
@ -1162,8 +1182,6 @@ rpc_callback_call(clicon_handle h,
retval = 1; /* 0: none found, >0 nr of handlers called */
done:
clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);
if (pc)
free(pc);
return retval;
fail:
retval = 0;

View file

@ -1480,10 +1480,10 @@ rpc_reply_check(clicon_handle h,
char *rpcname,
cbuf *cbret)
{
int retval = -1;
cxobj *x = NULL;
cxobj *xret = NULL;
int ret;
int retval = -1;
cxobj *x = NULL;
cxobj *xret = NULL;
int ret;
yang_stmt *yspec;
if ((yspec = clicon_dbspec_yang(h)) == NULL){
@ -1505,6 +1505,7 @@ rpc_reply_check(clicon_handle h,
if ((ret = xml_bind_yang_rpc_reply(x, rpcname, yspec, &xret)) < 0)
goto done;
if (ret == 0){
clicon_debug(1, "%s failure when validating:%s", __FUNCTION__, cbuf_get(cbret));
cbuf_reset(cbret);
if (clicon_xml2cbuf(cbret, xret, 0, 0, -1) < 0)
goto done;
@ -1513,6 +1514,7 @@ rpc_reply_check(clicon_handle h,
if ((ret = xml_yang_validate_rpc_reply(h, x, &xret)) < 0)
goto done;
if (ret == 0){
clicon_debug(1, "%s failure when validating:%s", __FUNCTION__, cbuf_get(cbret));
cbuf_reset(cbret);
if (clicon_xml2cbuf(cbret, xret, 0, 0, -1) < 0)
goto done;

View file

@ -232,19 +232,20 @@ xml_type2str(enum cxobj_type type)
}
/* Stats */
uint64_t _stats_nr = 0;
static uint64_t _stats_xml_nr = 0;
/*! Get global statistics about XML objects
*
* @param[out] nr Number of existing XML objects (created - freed)
*/
int
xml_stats_global(uint64_t *nr)
{
if (nr)
*nr = _stats_nr;
*nr = _stats_xml_nr;
return 0;
}
/*! Return the alloced memory of a single XML obj
* @param[in] x XML object
* @param[out] szp Size of this XML obj
@ -257,7 +258,6 @@ xml_stats_one(cxobj *x,
{
size_t sz = 0;
if (x->x_name)
sz += strlen(x->x_name) + 1;
if (x->x_prefix)
@ -292,51 +292,12 @@ xml_stats_one(cxobj *x,
}
if (szp)
*szp = sz;
clicon_debug(1, "%s %zu", __FUNCTION__, sz);
return 0;
}
#if 0
/*! Print memory stats of a single object
*/
static int
xml_print_stats_one(FILE *f,
cxobj *x)
{
size_t sz = 0;
xml_stats_one(x, &sz);
fprintf(f, "%s:\n", xml_name(x));
fprintf(f, " sum: \t\t%u\n", (unsigned int)sz);
if (xml_type(x) == CX_ELMNT)
fprintf(f, " base struct: \t%u\n", (unsigned int)sizeof(struct xml));
else
fprintf(f, " base struct: \t%u\n", (unsigned int)sizeof(struct xmlbody));
if (x->x_name)
fprintf(f, " name: \t%u\n", (unsigned int)strlen(x->x_name) + 1);
if (x->x_prefix)
fprintf(f, " prefix: \t%u\n", (unsigned int)strlen(x->x_prefix) + 1);
if (xml_type(x) == CX_ELMNT){
if (x->x_childvec_max)
fprintf(f, " childvec: \t%u\n", (unsigned int)(x->x_childvec_max*sizeof(struct xml*)));
if (x->x_ns_cache)
fprintf(f, " ns-cache: \t%u\n", (unsigned int)cvec_size(x->x_ns_cache));
if (x->x_cv)
fprintf(f, " value-cv: \t%u\n", (unsigned int)cv_size(x->x_cv));
if (x->x_search_index)
fprintf(f, " search-index: \t%u\n",
(unsigned int)(strlen(x->x_search_index->si_name) + 1 + clixon_xvec_len(x->x_search_index->si_xvec)*sizeof(struct cxobj*)));
}
else{
if (x->x_value_cb)
fprintf(f, " value-cb: \t%u\n", cbuf_buflen(x->x_value_cb));
}
return 0;
}
#endif
/*! Return statistics of an XML tree recursively
* @param[in] xt XML object
* @param[out] nrp Number of XML obj recursively
* @param[out] szp Size of this XML obj recursively
* @retval 0 OK
* @retval -1 Error
@ -354,7 +315,6 @@ xml_stats(cxobj *xt,
clicon_err(OE_XML, EINVAL, "xml node is NULL");
goto done;
}
// xml_print_stats_one(stderr, xt);
*nrp += 1;
xml_stats_one(xt, &sz);
if (szp)
@ -366,7 +326,6 @@ xml_stats(cxobj *xt,
if (szp)
*szp += sz;
}
clicon_debug(1, "%s %zu", __FUNCTION__, *szp);
retval = 0;
done:
return retval;
@ -1131,7 +1090,7 @@ xml_new(char *name,
return NULL;
x->_x_i = xml_child_nr(xp)-1;
}
_stats_nr++;
_stats_xml_nr++;
return x;
}
@ -1901,7 +1860,7 @@ xml_free(cxobj *x)
break;
}
free(x);
_stats_nr--;
_stats_xml_nr--;
return 0;
}

View file

@ -473,6 +473,100 @@ yang_linenum_set(yang_stmt *ys,
/* End access functions */
/* Stats */
static uint64_t _stats_yang_nr = 0;
/*! Get global statistics about YANG statements: created - freed
*
* @param[out] nr Number of existing YANG objects (created - freed)
*/
int
yang_stats_global(uint64_t *nr)
{
if (nr)
*nr = _stats_yang_nr;
return 0;
}
/*! Return the alloced memory of a single YANG obj
* @param[in] y YANG object
* @param[out] szp Size of this YANG obj
* @retval 0 OK
* (baseline: )
*/
static int
yang_stats_one(yang_stmt *y,
size_t *szp)
{
size_t sz = 0;
yang_type_cache *yc;
sz += sizeof(struct yang_stmt);
sz += y->ys_len*sizeof(struct yang_stmt*);
if (y->ys_argument)
sz += strlen(y->ys_argument) + 1;
if (y->ys_cv)
sz += cv_size(y->ys_cv);
if (y->ys_cvec)
sz += cvec_size(y->ys_cvec);
if ((yc = y->ys_typecache) != NULL){
sz += sizeof(struct yang_type_cache);
if (yc->yc_cvv)
sz += cvec_size(yc->yc_cvv);
if (yc->yc_patterns)
sz += cvec_size(yc->yc_patterns);
if (yc->yc_regexps)
sz += cvec_size(yc->yc_regexps);
}
if (y->ys_when_xpath)
sz += strlen(y->ys_when_xpath) + 1;
if (y->ys_when_nsc)
sz += cvec_size(y->ys_when_nsc);
if (y->ys_filename)
sz += strlen(y->ys_filename) + 1;
if (szp)
*szp = sz;
return 0;
}
/*! Return statistics of an YANG-stmt tree recursively
* @param[in] yt YANG object
* @param[out] nrp Number of YANG objects recursively
* @param[out] szp Size of this YANG stmt recursively
* @retval 0 OK
* @retval -1 Error
*/
int
yang_stats(yang_stmt *yt,
uint64_t *nrp,
size_t *szp)
{
int retval = -1;
size_t sz = 0;
yang_stmt *ys;
if (yt == NULL){
clicon_err(OE_XML, EINVAL, "yang spec is NULL");
goto done;
}
*nrp += 1;
yang_stats_one(yt, &sz);
if (szp)
*szp += sz;
ys = NULL;
while ((ys = yn_each(yt, ys)) != NULL) {
sz = 0;
yang_stats(ys, nrp, &sz);
if (szp)
*szp += sz;
}
retval = 0;
done:
return retval;
}
/* stats end */
/*! Create new yang specification
* @retval yspec Free with ys_free()
* @retval NULL Error
@ -488,6 +582,7 @@ yspec_new(void)
}
memset(yspec, 0, sizeof(*yspec));
yspec->ys_keyword = Y_SPEC;
_stats_yang_nr++;
return yspec;
}
@ -514,6 +609,7 @@ ys_new(enum rfc_6020 keyw)
return NULL;
}
yang_cvec_set(ys, cvv);
_stats_yang_nr++;
return ys;
}
@ -555,8 +651,10 @@ ys_free1(yang_stmt *ys,
free(ys->ys_stmt);
if (ys->ys_filename)
free(ys->ys_filename);
if (self)
if (self){
free(ys);
_stats_yang_nr++;
}
return 0;
}