From b3a7e39d261c502a902423e4a64fd62b36037450 Mon Sep 17 00:00:00 2001
From: Olof hagsand <olof@hagsand.se>
Date: Mon, 29 May 2023 12:25:36 +0200
Subject: [PATCH] C-API: Added specialized xml_child_each_attr function to
 counter perf issue

---
 CHANGELOG.md               |  2 +-
 lib/clixon/clixon_xml.h    |  1 +
 lib/src/clixon_xml.c       | 43 ++++++++++++++++++++++++++++++++++----
 lib/src/clixon_xml_map.c   |  4 ++--
 lib/src/clixon_xml_nsctx.c |  4 ++--
 5 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 512fd027..3386258e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -59,7 +59,7 @@ Developers may need to change their code
 
 ### Minor features
 
-* Performance: A change in the `merge` code made "co-located" config and non-config get retrieval go considerable faster.
+* Performance: A change in the `merge` code made "co-located" config and non-config get retrieval go considerable faster. This is done by a specialized `xml_child_each_attr()` function.
 * CLI: Added `show statistics` example code for backend and CLI memory stats
 * [Support yang type union with are same subtypes with SNMP](https://github.com/clicon/clixon/pull/427)
 * Removed obsolete compile options introduced in 6.1:
diff --git a/lib/clixon/clixon_xml.h b/lib/clixon/clixon_xml.h
index 311757dc..c69ad249 100644
--- a/lib/clixon/clixon_xml.h
+++ b/lib/clixon/clixon_xml.h
@@ -246,6 +246,7 @@ cxobj    *xml_child_i_type(cxobj *xn, int i, enum cxobj_type type);
 cxobj    *xml_child_i_set(cxobj *xt, int i, cxobj *xc);
 int       xml_child_order(cxobj *xn, cxobj *xc);
 cxobj    *xml_child_each(cxobj *xparent, cxobj *xprev,  enum cxobj_type type);
+cxobj    *xml_child_each_attr(cxobj *xparent, cxobj *xprev);
 int       xml_child_insert_pos(cxobj *x, cxobj *xc, int i);
 int       xml_childvec_set(cxobj *x, int len);
 cxobj   **xml_childvec_get(cxobj *x);
diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c
index d69ebfad..b442f1b8 100644
--- a/lib/src/clixon_xml.c
+++ b/lib/src/clixon_xml.c
@@ -972,8 +972,8 @@ xml_child_order(cxobj *xp,
  *      xprev = x;
  *   }
  * @endcode
- * @note Assumes attributes are first in list
  * @see xml_child_index_each
+ * @see xml_child_each_attr  hardcoded for sorted list and attributes
  */
 cxobj *
 xml_child_each(cxobj           *xparent, 
@@ -991,10 +991,45 @@ xml_child_each(cxobj           *xparent,
         xn = xparent->x_childvec[i];
         if (xn == NULL)
             continue;
-        if (type != CX_ERROR && xml_type(xn) != type){
-            if (type == CX_ATTR) /* Assume sorted attributes are first */
-                return NULL;
+        if (type != CX_ERROR && xml_type(xn) != type)
             continue;
+        break; /* this is next object after previous */
+    }
+    if (i < xparent->x_childvec_len) /* found */
+        xn->_x_vector_i = i;
+    else
+        xn = NULL;
+    return xn;
+}
+
+/*! Same as xml_child_each but hard-coded for attributes
+ *
+ * Assumes attributes are first in list, which they are if they are sorted, but there are
+ * situations where the children have not (yet) been sorted, in which case you need to use the 
+ * original function.
+ * @param[in] xparent xml tree node whose children should be iterated
+ * @param[in] xprev   previous child, or NULL on init
+ * @retval    xn      Next XML node
+ * @retval    NULL    End of list
+ * @see xml_child_each
+ */
+cxobj *
+xml_child_each_attr(cxobj           *xparent, 
+                    cxobj           *xprev)
+{
+    int             i;
+    cxobj          *xn = NULL; 
+
+    if (xparent == NULL)
+        return NULL;
+    if (!is_element(xparent))
+        return NULL;
+    for (i=xprev?xprev->_x_vector_i+1:0; i<xparent->x_childvec_len; i++){
+        xn = xparent->x_childvec[i];
+        if (xn == NULL)
+            continue;
+        if (xml_type(xn) != CX_ATTR){
+            return NULL;
         }
         break; /* this is next object after previous */
     }
diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c
index a412a7f8..deddecd5 100644
--- a/lib/src/clixon_xml_map.c
+++ b/lib/src/clixon_xml_map.c
@@ -1021,7 +1021,7 @@ assign_namespace_body(cxobj *x0, /* source */
     cxobj *xa;
     
     xa = NULL;
-    while ((xa = xml_child_each(x0, xa, CX_ATTR)) != NULL) {
+    while ((xa = xml_child_each_attr(x0, xa)) != NULL) {
         prefix0 = xml_prefix(xa);
         name = xml_name(xa);
         namespace = xml_value(xa);
@@ -1478,7 +1478,7 @@ xml_copy_marked(cxobj *x0,
     
     /* Copy all attributes */
     x = NULL;
-    while ((x = xml_child_each(x0, x, CX_ATTR)) != NULL) {
+    while ((x = xml_child_each_attr(x0, x)) != NULL) {
         name = xml_name(x);
         if ((xcopy = xml_new(name, x1, CX_ATTR)) == NULL)
             goto done;
diff --git a/lib/src/clixon_xml_nsctx.c b/lib/src/clixon_xml_nsctx.c
index 08b9a90a..c359b7ed 100644
--- a/lib/src/clixon_xml_nsctx.c
+++ b/lib/src/clixon_xml_nsctx.c
@@ -230,7 +230,7 @@ xml_nsctx_node1(cxobj *xn,
     /* xmlns:t="<ns1>" prefix:xmlns, name:t
      * xmlns="<ns2>"   prefix:NULL   name:xmlns
      */
-    while ((xa = xml_child_each(xn, xa, CX_ATTR)) != NULL){
+    while ((xa = xml_child_each_attr(xn, xa)) != NULL){
         pf = xml_prefix(xa);
         nm = xml_name(xa);
         if (pf == NULL){
@@ -674,7 +674,7 @@ xml2prefix(cxobj *xn,
     if (nscache_get_prefix(xn, namespace, &prefix) == 1) /* found */
         goto found;
     xa = NULL;
-    while ((xa = xml_child_each(xn, xa, CX_ATTR)) != NULL) {
+    while ((xa = xml_child_each_attr(xn, xa)) != NULL) {
         /* xmlns=namespace */
         if (strcmp("xmlns", xml_name(xa)) == 0){ 
             if (strcmp(xml_value(xa), namespace) == 0){