* Experimental optimzations of yang-bind and sort for large lists

* Enabled by compile-time options: `OPTIMIZE_45_BIND` and `OPTIMIZE_45_SORT`

CHANGELOG.md

@@ -55,6 +55,8 @@ Expected: May 2020
   * 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`
 * Experimental: restart_plugin
 * Two new plugin callbacks added
   * ca_daemon: Called just after a server has "daemonized", ie put in background.

apps/backend/backend_client.c

@@ -645,7 +645,7 @@ from_client_edit_config(clicon_handle h,
 	/* Cant do this earlier since we dont have a yang spec to
 	 * the upper part of the tree, until we get the "config" tree.
 	 */
-	if (xml_apply0(xc, CX_ELMNT, xml_sort, h) < 0)
+	if (xml_sort_recurse(xc) < 0)
 	    goto done;
 	if ((ret = xmldb_put(h, target, operation, xc, username, cbret)) < 0){
 	    clicon_debug(1, "%s ERROR PUT", __FUNCTION__);	

apps/backend/backend_commit.c

@@ -216,7 +216,7 @@ startup_common(clicon_handle       h,
     /* After upgrading, XML tree needs to be sorted and yang spec populated */
     if (xml_bind_yang(xt, YB_MODULE, yspec, NULL) < 0)
 	goto done;
-    if (xml_apply0(xt, CX_ELMNT, xml_sort, h) < 0)
+    if (xml_sort_recurse(xt) < 0)
 	goto done;
     /* Handcraft transition with with only add tree */
     td->td_target = xt;

apps/backend/backend_plugin.c

@@ -251,9 +251,9 @@ clixon_plugin_statedata_all(clicon_handle    h,
 	/* XXX: ret == 0 invalid yang binding should be handled as internal error */
 	if (xml_bind_yang(x, YB_MODULE, yspec, NULL) < 0)
 	    goto done;
-	if (xml_apply(x, CX_ELMNT, xml_sort, h) < 0)
+	if (xml_sort_recurse(x) < 0)
 	    goto done;
-	if (xml_apply(x, CX_ELMNT, xml_default, h) < 0)
+	if (xml_default_recurse(x) < 0)
 	    goto done;
 	if ((ret = netconf_trymerge(x, yspec, xret)) < 0)
 	    goto done;

apps/restconf/restconf_lib.c

@@ -744,7 +744,7 @@ restconf_insert_attributes(cxobj *xdata,
 	    goto done;
     }
     if (nsc)
-	xml_sort(xdata, NULL); /* Ensure attr is first */
+	xml_sort(xdata); /* Ensure attr is first */
     cprintf(cb, "/>");
     retval = 0;
  done:

include/clixon_custom.h

@@ -109,3 +109,13 @@
 /*! 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
+ */
+#undef OPTIMIZE_45_SORT

lib/clixon/clixon_xml_map.h

@@ -59,7 +59,8 @@ int xml_diff(yang_stmt *yspec, cxobj *x0, cxobj *x1,
 int xml_tree_prune_flagged_sub(cxobj *xt, int flag, int test, int *upmark);
 int xml_tree_prune_flagged(cxobj *xt, int flag, int test);
 int xml_namespace_change(cxobj *x, char *namespace, char *prefix);
-int xml_default(cxobj *x, void  *arg);
+int xml_default(cxobj *x);
+int xml_default_recurse(cxobj *xn);
 int xml_sanity(cxobj *x, void  *arg);
 int xml_non_config_data(cxobj *xt, void *arg);
 

lib/clixon/clixon_xml_sort.h

@@ -42,7 +42,7 @@
  * Prototypes
  */
 int xml_cmp(cxobj *x1, cxobj *x2, int same, int skip1, char *explicit);
-int xml_sort(cxobj *x0, void *arg);
+int xml_sort(cxobj *x0);
 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);

lib/src/clixon_datastore_read.c

@@ -451,7 +451,7 @@ xmldb_get_nocache(clicon_handle       h,
 	goto done;
 
     /* Add default values (if not set) */
-    if (xml_apply(xt, CX_ELMNT, xml_default, h) < 0)
+    if (xml_default_recurse(xt) < 0)
     	goto done;
 #if 0 /* debug */
     if (xml_apply0(xt, -1, xml_sort_verify, NULL) < 0)
@@ -560,7 +560,7 @@ xmldb_get_cache(clicon_handle    h,
 	goto done;
     /* x1t is wrong here should be <config><system>.. but is <system>.. */
     /* XXX where should we apply default values once? */
-    if (xml_apply(x1t, CX_ELMNT, xml_default, h) < 0)
+    if (xml_default_recurse(x1t) < 0)
 	goto done;
     /* Copy the matching parts of the (relevant) XML tree.
      * If cache was empty, also update to datastore cache
@@ -637,7 +637,7 @@ xmldb_get_zerocopy(clicon_handle    h,
 	xml_apply_ancestor(x0, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE);
     }
     /* Apply default values (removed in clear function) */
-    if (xml_apply(x0t, CX_ELMNT, xml_default, h) < 0)
+    if (xml_default_recurse(x0t) < 0)
 	goto done;
     if (debug>1)
     	clicon_xml2file(stderr, x0t, 0, 1);

lib/src/clixon_datastore_write.c

@@ -191,7 +191,7 @@ check_body_namespace(cxobj     *x0,
 	    }
 	    if (xml_value_set(xa, ns0) < 0)
 		goto done;
-	    xml_sort(x, NULL); /* Ensure attr is first / XXX xml_insert? */
+	    xml_sort(x); /* Ensure attr is first / XXX xml_insert? */
 	}
     }
  ok:

lib/src/clixon_json.c

@@ -1271,9 +1271,6 @@ _json_parse(char      *str,
     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;
  done:
     clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);

lib/src/clixon_options.c

@@ -261,7 +261,7 @@ parse_configfile(clicon_handle  h,
 	    
 	goto done;
     }
-    if (xml_apply0(xc, CX_ELMNT, xml_default, h) < 0)
+    if (xml_default_recurse(xc) < 0)
 	goto done;	
     if ((ret = xml_yang_validate_add(h, xc, &xerr)) < 0)
 	goto done;

lib/src/clixon_validate.c

@@ -372,7 +372,7 @@ xml_yang_validate_rpc(clicon_handle h,
 	    goto done; /* error or validation fail */
 	if ((retval = xml_yang_validate_add(h, xn, xret)) < 1)
 	    goto done; /* error or validation fail */
-	if (xml_apply0(xn, CX_ELMNT, xml_default, h) < 0)
+	if (xml_default_recurse(xn) < 0)
 	    goto done;
     }
     // ok: /* pass validation */

lib/src/clixon_xml_bind.c

@@ -113,6 +113,9 @@ strip_whitespace(cxobj *xt)
  */
 static int
 populate_self_parent(cxobj  *xt,
+#ifdef OPTIMIZE_45_BIND
+		     cxobj  *xsibling,
+#endif
 		     cxobj **xerr)
 {
     int        retval = -1;
@@ -124,8 +127,16 @@ populate_self_parent(cxobj  *xt,
     char      *nsy = NULL;   /* Yang namespace of xt */
     cbuf      *cb = NULL;
 
-    xp = xml_parent(xt);
     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 &&
 	    netconf_bad_element_xml(xerr, "application", name, "Missing parent") < 0)
@@ -172,6 +183,9 @@ 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))
@@ -317,6 +331,79 @@ xml_bind_yang(cxobj     *xt,
     goto done;
 }
 
+#ifdef OPTIMIZE_45_BIND
+int
+xml_bind_yang0_opt(cxobj     *xt, 
+		   yang_bind  yb,
+		   cxobj     *xsibling,
+		   cxobj    **xerr)
+{
+    int        retval = -1;
+    cxobj     *xc;           /* xml child */
+    int        ret;
+    int        failed = 0; /* we continue loop after failure, should we stop at fail?`*/
+    yang_stmt *yc0 = NULL;
+    cxobj     *xc0 = NULL;
+    cxobj     *xs;
+    char      *name0 = NULL;
+    char      *prefix0 = NULL;
+    char      *name;
+    char      *prefix;
+
+    switch (yb){
+    case YB_PARENT:
+	if ((ret = populate_self_parent(xt, xsibling, xerr)) < 0)
+	    goto done;
+	break;
+    default:
+	clicon_err(OE_XML, EINVAL, "Invalid yang binding: %d", yb);
+	goto done;
+	break;
+    }
+    if (ret == 0)
+	goto fail;
+    else if (ret == 2)     /* ret=2 for anyxml from parent^ */
+    	goto ok;
+    strip_whitespace(xt);
+    xc = NULL;     /* Apply on children */
+    while ((xc = xml_child_each(xt, xc, CX_ELMNT)) != NULL) {
+	/* It is xml2ns in populate_self_parent that needs improvement */
+	/* cache previous + prefix */
+	name = xml_name(xc);
+	prefix = xml_prefix(xc);
+	if (yc0 != NULL &&
+	    clicon_strcmp(name0, name) == 0 &&
+	    clicon_strcmp(prefix0, prefix) == 0){
+	    if ((ret = xml_bind_yang0_opt(xc, YB_PARENT, xc0, xerr)) < 0)
+		goto done;
+	}
+	else if (xsibling &&
+		 (xs = xml_find_type(xsibling, prefix, name, CX_ELMNT)) != NULL){
+	    if ((ret = xml_bind_yang0_opt(xc, YB_PARENT, xs, xerr)) < 0)
+		goto done;
+	}
+	else if ((ret = xml_bind_yang0_opt(xc, YB_PARENT, NULL, xerr)) < 0)
+	    goto done;
+	if (ret == 0)
+	    failed++;
+	xc0 = xc;
+	yc0 = xml_spec(xc); /* cache */
+	name0 = xml_name(xc);
+	prefix0 = xml_prefix(xc);
+    }
+    if (failed)
+	goto fail;
+ ok:
+    retval = 1;
+ done:
+    return retval;
+ fail:
+    retval = 0;
+    goto done;
+}
+#endif /* OPTIMIZE_45_BIND */
+
+
 /*! Find yang spec association of tree of XML nodes
  *
  * @param[in]   xt     XML tree node
@@ -334,10 +421,10 @@ xml_bind_yang0(cxobj     *xt,
 	       yang_stmt *yspec,
 	       cxobj    **xerr)
 {
-    int    retval = -1;
+    int        retval = -1;
-    cxobj *xc;           /* xml child */
+    cxobj     *xc;           /* xml child */
-    int    ret;
+    int        ret;
-    int    failed = 0; /* we continue loop after failure, should we stop at fail?`*/
+    int        failed = 0; /* we continue loop after failure, should we stop at fail?`*/
 
     switch (yb){
     case YB_MODULE:
@@ -345,9 +432,16 @@ xml_bind_yang0(cxobj     *xt,
 	    goto done;
 	break;
     case YB_PARENT:
-	if ((ret = populate_self_parent(xt, xerr)) < 0)
+	if ((ret = populate_self_parent(xt,
+#ifdef OPTIMIZE_45_BIND
+					NULL,
+#endif
+					xerr)) < 0)
 	    goto done;
 	break;
+    case YB_NONE:
+	ret = 1;
+	break;
     default:
 	clicon_err(OE_XML, EINVAL, "Invalid yang binding: %d", yb);
 	goto done;
@@ -360,8 +454,13 @@ xml_bind_yang0(cxobj     *xt,
     strip_whitespace(xt);
     xc = NULL;     /* Apply on children */
     while ((xc = xml_child_each(xt, xc, CX_ELMNT)) != NULL) {
-	if ((ret = xml_bind_yang0(xc, YB_PARENT, yspec, xerr)) < 0)
+#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++;
     }

lib/src/clixon_xml_map.c

@@ -846,7 +846,7 @@ add_namespace(cxobj *x,
     }
     if (xml_value_set(xa, namespace) < 0)
 	goto done;
-    xml_sort(xp, NULL); /* Ensure attr is first / XXX xml_insert? */
+    xml_sort(xp); /* Ensure attr is first / XXX xml_insert? */
 	
     retval = 0;
  done:
@@ -905,15 +905,12 @@ xml_namespace_change(cxobj *x,
 
 /*! Add default values (if not set)
  * @param[in]   xt      XML tree with some node marked
- * @param[in]   arg     Ignored
  * Typically called in a recursive apply function:
- * @code
+ * @retval      0       OK
- *   xml_apply(xt, CX_ELMNT, xml_default, NULL);
+ * @retval      -1      Error
- * @endcode
  */
 int
-xml_default(cxobj *xt, 
+xml_default(cxobj *xt)
-	    void   *arg)
 {
     int        retval = -1;
     yang_stmt *ys;
@@ -988,6 +985,27 @@ xml_default(cxobj *xt,
     return retval;
 }
 
+/*! Recursively fill in default values in a tree
+ * Alt to use xml_apply
+ */
+int
+xml_default_recurse(cxobj *xn)
+{
+    int    retval = -1;
+    cxobj *x;
+    
+    if (xml_default(xn) < 0)
+	goto done;
+    x = NULL;
+    while ((x = xml_child_each(xn, x, CX_ELMNT)) != NULL) {
+	if (xml_default_recurse(x) < 0)
+	    goto done;
+    }
+    retval = 0;
+ done:
+    return retval;
+}
+
 /*! Sanitize an xml tree: xml node has matching yang_stmt pointer 
  * @param[in]   xt      XML top of tree
  */

lib/src/clixon_xml_nsctx.c

@@ -463,7 +463,11 @@ xml2ns(cxobj *x,
      * no cache was found 
      * If not, this is devastating when populating deep yang structures
      */
-    if (ns && nscache_set(x, prefix, ns) < 0)
+    if (ns &&
+#ifdef OPTIMIZE_45_BIND  /* Dont set cache if few children: if 1 child typically a body */
+	xml_child_nr(x) > 1 &&
+#endif
+	nscache_set(x, prefix, ns) < 0)
 	goto done;
  ok:
     if (namespace)

lib/src/clixon_xml_sort.c

@@ -143,6 +143,22 @@ xml_cv_cache(cxobj   *x,
     return retval;
 }
 
+#ifdef OPTIMIZE_45_SORT
+static int
+xml_cv_cache_clear(cxobj *xt)
+{
+    int    retval = -1;
+    cxobj *x = NULL;
+
+    while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL)
+	if (xml_cv_set(x, NULL) < 0)
+	    goto done;
+    retval = 0;
+ 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
  * @param[in]  x2    object 2
@@ -386,7 +402,6 @@ xml_cmp_qsort(const void* arg1,
 /*! Sort children of an XML node 
  * Assume populated by yang spec.
  * @param[in] x0   XML node
- * @param[in] arg  Dummy so it can be called by xml_apply()
  * @retval    -1    Error, aborted at first error encounter
  * @retval     0    OK, all nodes traversed (subparts may have been skipped)
  * @retval     1    OK, aborted on first fn returned 1
@@ -394,8 +409,7 @@ xml_cmp_qsort(const void* arg1,
  * @see xml_sort_verify
  */
 int
-xml_sort(cxobj *x,
+xml_sort(cxobj *x)
-	 void  *arg)
 {
 #ifndef STATE_ORDERED_BY_SYSTEM
     yang_stmt *ys;
@@ -419,15 +433,30 @@ xml_sort_recurse(cxobj *xn)
     cxobj *x;
     int    ret;
     
-    x = NULL;
+#ifdef OPTIMIZE_45_SORT
-    while ((x = xml_child_each(xn, x, CX_ELMNT)) != NULL) {
+    ret = xml_sort_verify(xn, NULL);
-	if ((ret = xml_sort(x, NULL)) < 0)
+    if (ret == 1) /* This node is not sortable */
+	goto ok;
+    if (ret == -1){ /* not sorted */
+	if ((ret = xml_sort(xn)) < 0)
 	    goto done;
 	if (ret == 1) /* This node is not sortable */
-	    break;
+	    goto ok;
+    }
+    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)
 	    goto done;
     }
+ ok:
     retval = 0;
  done:
     return retval;
@@ -1068,10 +1097,11 @@ xml_insert(cxobj           *xp,
 }
 
 /*! Verify all children of XML node are sorted according to xml_sort()
- * @param[in]   x       XML node. Check its children
+ * @param[in]   x    XML node. Check its children
- * @param[in]   arg     Dummy. Ensures xml_apply can be used with this fn
+ * @param[in]   arg  Dummy. Ensures xml_apply can be used with this fn
- * @retval      0       Sorted
+ * @retval      1    Not sortable
- * @retval     -1       Not sorted
+ * @retval      0    Sorted
+ * @retval     -1    Not sorted
  * @see xml_apply
  */
 int

test/test_perf_state_only.sh

@@ -3,6 +3,8 @@
 # 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)
+# Alternative, run as:
+# sudo clixon_backend -Fs init -f /var/tmp/./test_perf_state_only.sh/config.xml -- -siS /home/olof/tmp/state_100K.xml
 
 # Magic line must be first in script (see README.md)
 s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
@@ -69,7 +71,7 @@ module $APPNAME{
           }
           leaf enabled {
             type boolean;
-            default true;
+            default true; 
           }
           leaf status {
             type string;
@@ -114,6 +116,7 @@ wait_restconf
 
 new "cli get large config"
 echo "$clixon_cli -1f $cfg show state xml"
+# baseline on thinkpad i5-3320M CPU @ 2.60GHz and 500K entries: 39.71s
 $TIMEFN $clixon_cli -1f $cfg show state xml 2>&1 | awk '/real/ {print $2}'
 
 # START actual tests

test/test_xml_trees.sh

@@ -90,10 +90,10 @@ new "parent 1st element"
 testrun parent "$x0a$x0b" "<a><x>1</x></a>" $p 0 '<c xmlns="urn:example:example"><a><x>1</x></a></c>'
 
 new "parse parent element"
-testrun parent "$x0a<a><x>2</x></a>$x0b" '<a><x>1</x></a>' $p 0 '<c xmlns="urn:example:example"><a><x>2</x></a><a><x>1</x></a></c>'
+testrun parent "$x0a<a><x>2</x></a>$x0b" '<a><x>1</x></a>' $p 0 '<c xmlns="urn:example:example"><a><x>1</x></a><a><x>2</x></a></c>'
 
 new "parse parent container"
-testrun parent "$x0a<a><x>1</x></a>$x0b" '<d>42</d>' c 0 '<c xmlns="urn:example:example"><a><x>1</x></a><d>42</d></c>'
+testrun parent "$x0a<a><x>1</x></a>$x0b" '<d>42</d>' c 0 '<c xmlns="urn:example:example"><d>42</d><a><x>1</x></a></c>'
 
 # -------- merge
 

util/clixon_util_path.c

@@ -223,12 +223,12 @@ main(int    argc,
 	if (xml_bind_yang(x, YB_MODULE, yspec, NULL) < 0)
 	    goto done;
 	/* sort */
-	if (xml_apply0(x, CX_ELMNT, xml_sort, h) < 0)
+	if (xml_sort_recurse(x) < 0)
 	    goto done;
 	if (xml_apply0(x, -1, xml_sort_verify, h) < 0)
 	    clicon_log(LOG_NOTICE, "%s: sort verify failed", __FUNCTION__);
 	/* Add default values */
-	if (xml_apply(x, CX_ELMNT, xml_default, h) < 0)
+	if (xml_default_recurse(x) < 0)
 	    goto done;
 	if ((ret = xml_yang_validate_all_top(h, x, &xerr)) < 0) 
 	    goto done;

util/clixon_util_xml.c

@@ -80,7 +80,7 @@ validate_tree(clicon_handle h,
 
     /* should already be populated */
     /* Add default values */
-    if (xml_apply(xt, CX_ELMNT, xml_default, h) < 0)
+    if (xml_default_recurse(xt) < 0)
 	goto done;
     if (xml_apply(xt, -1, xml_sort_verify, h) < 0)
 	clicon_log(LOG_NOTICE, "%s: sort verify failed", __FUNCTION__);

util/clixon_util_xml_mod.c

@@ -261,7 +261,7 @@ main(int argc, char **argv)
 	xml_print(stderr, x0);
     }
     if (sort)
-	xml_apply(xb, CX_ELMNT, xml_sort, h);
+	xml_sort_recurse(xb);
     if (strcmp(xml_name(xb),"top")==0)
 	clicon_xml2file(stdout, xml_child_i_type(xb, 0, CX_ELMNT), 0, 0);
     else

util/clixon_util_xpath.c

@@ -291,11 +291,11 @@ main(int    argc,
 	if (xml_bind_yang(x0, YB_MODULE, yspec, NULL) < 0)
 	    goto done;
 	/* Sort */
-	if (xml_apply(x0, CX_ELMNT, xml_sort, h) < 0)
+	if (xml_sort_recurse(x0) < 0)
 	    goto done;
 
 	/* Add default values */
-	if (xml_apply(x0, CX_ELMNT, xml_default, h) < 0)
+	if (xml_default_recurse(x0) < 0)
 	    goto done;
 	if (xml_apply0(x0, -1, xml_sort_verify, h) < 0)
 	    clicon_log(LOG_NOTICE, "%s: sort verify failed", __FUNCTION__);