tree data OPTIMIZE diff and lyd_insert_node()

Function lyd_insert_node() has new options that ignore the ordering of data nodes. This is used in lyd_diff_add() because the diff as such does not need to be sorted.
CESNET · Feb 9, 2024 · 8026822 · 8026822
1 parent ae0e31a
commit 8026822
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 41 deletions.
diff --git a/src/diff.c b/src/diff.c
@@ -366,7 +366,7 @@ lyd_diff_add(const struct lyd_node *node, enum lyd_diff_op op, const char *orig_
 
         dup = diff_parent;
     } else {
-        diff_opts = LYD_DUP_NO_META | LYD_DUP_WITH_PARENTS | LYD_DUP_WITH_FLAGS;
+        diff_opts = LYD_DUP_NO_META | LYD_DUP_WITH_PARENTS | LYD_DUP_WITH_FLAGS | LYD_DUP_NO_LYDS;
         if ((op != LYD_DIFF_OP_REPLACE) || !lysc_is_userordered(node->schema) || (node->schema->flags & LYS_CONFIG_R)) {
             /* move applies only to the user-ordered list, no descendants */
             diff_opts |= LYD_DUP_RECURSIVE;

diff --git a/src/parser_json.c b/src/parser_json.c
@@ -1071,7 +1071,8 @@ lydjson_parse_opaq(struct lyd_json_ctx *lydctx, const char *name, size_t name_le
         /* continue with the next instance */
         LY_CHECK_GOTO(ret = lyjson_ctx_next(lydctx->jsonctx, status_inner_p), cleanup);
         assert(*node_p);
-        lydjson_maintain_children(parent, first_p, node_p, lydctx->parse_opts & LYD_PARSE_ORDERED ? 1 : 0, NULL);
+        lydjson_maintain_children(parent, first_p, node_p,
+                lydctx->parse_opts & LYD_PARSE_ORDERED ? LYD_INSERT_LAST : 0, NULL);
 
         LOG_LOCBACK(0, 1, 0, 0);
 

diff --git a/src/parser_lyb.c b/src/parser_lyb.c
@@ -931,7 +931,7 @@ lyb_insert_node(struct lyd_lyb_ctx *lybctx, struct lyd_node *parent, struct lyd_
     if (parent && (LYD_CTX(parent) != LYD_CTX(node))) {
         lyplg_ext_insert(parent, node);
     } else {
-        lyd_insert_node(parent, first_p, node, lybctx->parse_opts & LYD_PARSE_ORDERED ? 1 : 0);
+        lyd_insert_node(parent, first_p, node, lybctx->parse_opts & LYD_PARSE_ORDERED ? LYD_INSERT_LAST : 0);
     }
     while (!parent && (*first_p)->prev->next) {
         *first_p = (*first_p)->prev;

diff --git a/src/parser_xml.c b/src/parser_xml.c
@@ -1121,7 +1121,7 @@ lydxml_subtree_r(struct lyd_xml_ctx *lydctx, struct lyd_node *parent, struct lyd
         r = lyplg_ext_insert(parent, node);
         LY_CHECK_ERR_GOTO(r, rc = r; lyd_free_tree(node), cleanup);
     } else {
-        lyd_insert_node(parent, first_p, node, lydctx->parse_opts & LYD_PARSE_ORDERED ? 1 : 0);
+        lyd_insert_node(parent, first_p, node, lydctx->parse_opts & LYD_PARSE_ORDERED ? LYD_INSERT_LAST : 0);
     }
     while (!parent && (*first_p)->prev->next) {
         *first_p = (*first_p)->prev;
@@ -1438,7 +1438,7 @@ lydxml_opaq_r(struct lyxml_ctx *xmlctx, struct lyd_node *parent)
     }
 
     /* insert */
-    lyd_insert_node(parent, NULL, node, 1);
+    lyd_insert_node(parent, NULL, node, LYD_INSERT_LAST);
 
     /* update the value */
     opaq = (struct lyd_node_opaq *)node;
@@ -1572,7 +1572,7 @@ lydxml_env_netconf_rpc_reply_error_info(struct lyxml_ctx *xmlctx, struct lyd_nod
         LY_CHECK_GOTO(r = lyxml_ctx_next(xmlctx), error);
 
         /* insert */
-        lyd_insert_node(parent, NULL, child, 1);
+        lyd_insert_node(parent, NULL, child, LYD_INSERT_LAST);
     }
 
     return LY_SUCCESS;
@@ -1747,7 +1747,7 @@ lydxml_env_netconf_rpc_reply_error(struct lyxml_ctx *xmlctx, struct lyd_node *pa
         LY_CHECK_GOTO(r = lyxml_ctx_next(xmlctx), error);
 
         /* insert */
-        lyd_insert_node(parent, NULL, child, 1);
+        lyd_insert_node(parent, NULL, child, LYD_INSERT_LAST);
     }
 
     return LY_SUCCESS;
@@ -1791,7 +1791,7 @@ lydxml_env_netconf_reply(struct lyxml_ctx *xmlctx, struct lyd_node **envp, uint3
     r = lydxml_envelope(xmlctx, "ok", "urn:ietf:params:xml:ns:netconf:base:1.0", 0, &child);
     if (r == LY_SUCCESS) {
         /* insert */
-        lyd_insert_node(*envp, NULL, child, 1);
+        lyd_insert_node(*envp, NULL, child, LYD_INSERT_LAST);
 
         /* finish child parsing */
         if (xmlctx->status != LYXML_ELEM_CLOSE) {
@@ -1822,7 +1822,7 @@ lydxml_env_netconf_reply(struct lyxml_ctx *xmlctx, struct lyd_node **envp, uint3
         }
 
         /* insert */
-        lyd_insert_node(*envp, NULL, child, 1);
+        lyd_insert_node(*envp, NULL, child, LYD_INSERT_LAST);
 
         /* parse all children of "rpc-error" */
         LY_CHECK_GOTO(rc = lydxml_env_netconf_rpc_reply_error(xmlctx, child), cleanup);

diff --git a/src/tree_data.c b/src/tree_data.c
@@ -699,13 +699,6 @@ lyd_insert_node_last(struct lyd_node *parent, struct lyd_node **first_sibling, s
     assert(first_sibling && node);
 
     if (*first_sibling) {
-#ifndef NDEBUG
-        if (lyds_is_supported(node) && ((*first_sibling)->prev->schema == node->schema) &&
-                (lyds_compare_single((*first_sibling)->prev, node) > 0)) {
-            LOGWRN(LYD_CTX(node), "Data in \"%s\" are not sorted, inserted node should not be added to the end.",
-                    node->schema->name);
-        }
-#endif
         lyd_insert_after_node(first_sibling, (*first_sibling)->prev, node);
     } else if (parent) {
         lyd_insert_only_child(parent, node);
@@ -731,7 +724,7 @@ lyd_insert_node_ordby_schema(struct lyd_node *parent, struct lyd_node **first_si
 }
 
 void
-lyd_insert_node(struct lyd_node *parent, struct lyd_node **first_sibling_p, struct lyd_node *node, ly_bool last)
+lyd_insert_node(struct lyd_node *parent, struct lyd_node **first_sibling_p, struct lyd_node *node, uint32_t opts)
 {
     LY_ERR ret = LY_SUCCESS;
     struct lyd_node *first_sibling, *leader;
@@ -746,8 +739,10 @@ lyd_insert_node(struct lyd_node *parent, struct lyd_node **first_sibling_p, stru
     }
     first_sibling = parent ? lyd_child(parent) : *first_sibling_p;
 
-    if (last) {
+    if (opts & LYD_INSERT_LAST) {
         lyd_insert_node_last(parent, &first_sibling, node);
+    } else if (opts & LYD_INSERT_BY_SCHEMA) {
+        lyd_insert_node_ordby_schema(parent, &first_sibling, node);
     } else if (lyds_is_supported(node) &&
             (lyd_find_sibling_schema(first_sibling, node->schema, &leader) == LY_SUCCESS)) {
         ret = lyds_insert(&first_sibling, &leader, node);
@@ -776,6 +771,14 @@ lyd_insert_node(struct lyd_node *parent, struct lyd_node **first_sibling_p, stru
     if (first_sibling_p) {
         *first_sibling_p = first_sibling;
     }
+
+#ifndef NDEBUG
+    if ((opts & LYD_INSERT_LAST) && lyds_is_supported(node) &&
+            (node->prev->schema == node->schema) && (lyds_compare_single(node->prev, node) > 0)) {
+        LOGWRN(LYD_CTX(node), "Data in \"%s\" are not sorted, inserted node should not be added to the end.",
+                node->schema->name);
+    }
+#endif
 }
 
 /**
@@ -1048,7 +1051,7 @@ lyplg_ext_insert(struct lyd_node *parent, struct lyd_node *first)
     while (first) {
         iter = first->next;
         lyd_unlink(first);
-        lyd_insert_node(parent, NULL, first, 1);
+        lyd_insert_node(parent, NULL, first, LYD_INSERT_LAST);
         first = iter;
     }
     return LY_SUCCESS;
@@ -2104,7 +2107,7 @@ lyd_dup_r(const struct lyd_node *node, const struct ly_ctx *trg_ctx, struct lyd_
             /* duplicate all the children */
             LY_LIST_FOR(orig->child, child) {
                 LY_CHECK_GOTO(ret = lyd_dup_r(child, trg_ctx, dup, options, &dup_child), error);
-                lyd_insert_node(dup, NULL, dup_child, 1);
+                lyd_insert_node(dup, NULL, dup_child, LYD_INSERT_LAST);
             }
         }
         LY_CHECK_GOTO(ret = lydict_insert(trg_ctx, orig->name.name, 0, &opaq->name.name), error);
@@ -2141,13 +2144,13 @@ lyd_dup_r(const struct lyd_node *node, const struct ly_ctx *trg_ctx, struct lyd_
             /* duplicate all the children */
             LY_LIST_FOR(orig->child, child) {
                 LY_CHECK_GOTO(ret = lyd_dup_r(child, trg_ctx, dup, options, &dup_child), error);
-                lyd_insert_node(dup, NULL, dup_child, 1);
+                lyd_insert_node(dup, NULL, dup_child, LYD_INSERT_LAST);
             }
         } else if ((dup->schema->nodetype == LYS_LIST) && !(dup->schema->flags & LYS_KEYLESS)) {
             /* always duplicate keys of a list */
             for (child = orig->child; child && lysc_is_key(child->schema); child = child->next) {
                 LY_CHECK_GOTO(ret = lyd_dup_r(child, trg_ctx, dup, options, &dup_child), error);
-                lyd_insert_node(dup, NULL, dup_child, 1);
+                lyd_insert_node(dup, NULL, dup_child, LYD_INSERT_LAST);
             }
         }
         lyd_hash(dup);
@@ -2340,7 +2343,8 @@ lyd_dup(const struct lyd_node *node, const struct ly_ctx *trg_ctx, struct lyd_no
             }
         } else {
             rc = lyd_dup_r(orig, trg_ctx, local_parent, options, &dup);
-            lyd_insert_node(local_parent, &first_sibling, dup, 0);
+            lyd_insert_node(local_parent, &first_sibling, dup,
+                    options & LYD_DUP_NO_LYDS ? LYD_INSERT_BY_SCHEMA : 0);
             LY_CHECK_GOTO(rc, error);
         }
         first_dup = first_dup ? first_dup : dup;

diff --git a/src/tree_data.h b/src/tree_data.h
@@ -2002,6 +2002,8 @@ LIBYANG_API_DECL LY_ERR lyd_compare_meta(const struct lyd_meta *meta1, const str
 #define LYD_DUP_NO_EXT       0x10  /**< Do not duplicate nodes with the ::LYD_EXT flag (nested extension instance data). */
 #define LYD_DUP_WITH_PRIV    0x20  /**< Also copy data node private pointer. Only the pointer is copied, it still points
                                         to the same data. */
+#define LYD_DUP_NO_LYDS      0x40  /**< The node is inserted according to the schema, sorting using the
+                                        'lyds tree' does not happen. Useful for optimization. */
 
 /** @} dupoptions */
 

diff --git a/src/tree_data_internal.h b/src/tree_data_internal.h
@@ -388,16 +388,33 @@ void lyd_insert_after_node(struct lyd_node **first_sibling, struct lyd_node *sib
  */
 void lyd_insert_before_node(struct lyd_node *sibling, struct lyd_node *node);
 
+/**
+ * @defgroup insertoptions Data insert options.
+ *
+ * Various options for optimal node insertion.
+ * Flags that cause the order of nodes not to be checked are adapted to fast insertion but can cause
+ * nodes for (leaf-)lists with LYS_ORDBY_SYSTEM flag set to be out of order, which is an undesirable state,
+ * so these flags must be set carefully.
+ *
+ * Default behavior:
+ * - The node is inserted to preserve order.
+ * @{
+ */
+
+#define LYD_INSERT_LAST         0x01 /**< Node inserted as last sibling. Node order not checked. */
+#define LYD_INSERT_BY_SCHEMA    0x02 /**< The node is inserted according to the scheme. Node order not checked. */
+
+/** @} insertoptions */
+
 /**
  * @brief Insert a node into parent/siblings. Order and hashes are fully handled.
  *
  * @param[in] parent Parent to insert into, NULL for top-level sibling.
  * @param[in,out] first_sibling First sibling, NULL if no top-level sibling exist yet. Can be also NULL if @p parent is set.
  * @param[in] node Individual node (without siblings) to insert.
- * @param[in] last If set, do not search for the correct anchor but always insert at the end. Setting the flag can
- * break the ordering of nodes for (leaf-)lists that have the LYS_ORDBY_SYSTEM flag set.
+ * @param[in] opts Insert options (@ref insertoptions).
  */
-void lyd_insert_node(struct lyd_node *parent, struct lyd_node **first_sibling, struct lyd_node *node, ly_bool last);
+void lyd_insert_node(struct lyd_node *parent, struct lyd_node **first_sibling, struct lyd_node *node, uint32_t opts);
 
 /**
  * @brief Insert a node into parent/siblings.

diff --git a/src/tree_data_new.c b/src/tree_data_new.c
@@ -626,7 +626,7 @@ _lyd_new_list(struct lyd_node *parent, const struct lys_module *module, const ch
 
         rc = lyd_create_term(key_s, key_val, key_len, 0, NULL, format, NULL, LYD_HINT_DATA, NULL, &key);
         LY_CHECK_GOTO(rc, cleanup);
-        lyd_insert_node(ret, NULL, key, 1);
+        lyd_insert_node(ret, NULL, key, LYD_INSERT_LAST);
     }
 
     if (parent) {
@@ -722,7 +722,7 @@ lyd_new_ext_list(const struct lysc_ext_instance *ext, const char *name, struct l
         rc = lyd_create_term(key_s, key_val, key_val ? strlen(key_val) : 0, 0, NULL, LY_VALUE_JSON, NULL, LYD_HINT_DATA,
                 NULL, &key);
         LY_CHECK_GOTO(rc, cleanup);
-        lyd_insert_node(ret, NULL, key, 1);
+        lyd_insert_node(ret, NULL, key, LYD_INSERT_LAST);
     }
 
 cleanup:
@@ -829,7 +829,7 @@ _lyd_new_list3(struct lyd_node *parent, const struct lys_module *module, const c
 
         rc = lyd_create_term(key_s, key_val, key_len, 0, NULL, format, NULL, LYD_HINT_DATA, NULL, &key);
         LY_CHECK_GOTO(rc, cleanup);
-        lyd_insert_node(ret, NULL, key, 1);
+        lyd_insert_node(ret, NULL, key, LYD_INSERT_LAST);
     }
 
     if (parent) {
@@ -1140,7 +1140,7 @@ lyd_new_opaq(struct lyd_node *parent, const struct ly_ctx *ctx, const char *name
     LY_CHECK_RET(lyd_create_opaq(ctx, name, strlen(name), prefix, prefix ? strlen(prefix) : 0, module_name,
             strlen(module_name), value, strlen(value), NULL, LY_VALUE_JSON, NULL, 0, &ret));
     if (parent) {
-        lyd_insert_node(parent, NULL, ret, 1);
+        lyd_insert_node(parent, NULL, ret, LYD_INSERT_LAST);
     }
 
     if (node) {
@@ -1168,7 +1168,7 @@ lyd_new_opaq2(struct lyd_node *parent, const struct ly_ctx *ctx, const char *nam
     LY_CHECK_RET(lyd_create_opaq(ctx, name, strlen(name), prefix, prefix ? strlen(prefix) : 0, module_ns,
             strlen(module_ns), value, strlen(value), NULL, LY_VALUE_XML, NULL, 0, &ret));
     if (parent) {
-        lyd_insert_node(parent, NULL, ret, 1);
+        lyd_insert_node(parent, NULL, ret, LYD_INSERT_LAST);
     }
 
     if (node) {

diff --git a/src/tree_data_sorted.c b/src/tree_data_sorted.c
@@ -968,7 +968,11 @@ rb_remove_node(struct lyd_meta *root_meta, struct rb_node **rbt, struct lyd_node
 
     /* find @p node in the Red-black tree. */
     rbn = rb_find(*rbt, node);
-    assert(rbn && (RBN_DNODE(rbn) == node));
+    if (!rbn) {
+        /* node was not inserted to the lyds tree due to optimization */
+        return;
+    }
+    assert(RBN_DNODE(rbn) == node);
 
     /* remove node */
     rbn = rb_remove(rbt, rbn);
@@ -1725,7 +1729,9 @@ lyds_compare_single(struct lyd_node *node1, struct lyd_node *node2)
 {
     assert(node1 && node2 && (node1->schema == node2->schema) && lyds_is_supported(node1));
 
-    if (node1->schema->nodetype == LYS_LEAFLIST) {
+    if (node1 == node2) {
+        return 0;
+    } else if (node1->schema->nodetype == LYS_LEAFLIST) {
         return rb_compare_leaflists(node1, node2);
     } else {
         return rb_compare_lists(node1, node2);

diff --git a/tests/utests/data/test_diff.c b/tests/utests/data/test_diff.c
@@ -593,14 +593,14 @@ test_list(void **state)
             "  </list>\n"
             "</df>\n";
     const char *xml3 = "<df xmlns=\"urn:libyang:tests:defaults\">\n"
-            "  <list>\n"
-            "    <name>a</name>\n"
-            "    <value>2</value>\n"
-            "  </list>\n"
             "  <list>\n"
             "    <name>b</name>\n"
             "    <value>-2</value>\n"
             "  </list>\n"
+            "  <list>\n"
+            "    <name>a</name>\n"
+            "    <value>2</value>\n"
+            "  </list>\n"
             "</df>\n";
 
     const char *out_diff_1 =
@@ -620,14 +620,14 @@ test_list(void **state)
             "</df>\n";
     const char *out_diff_2 =
             "<df xmlns=\"urn:libyang:tests:defaults\" xmlns:yang=\"urn:ietf:params:xml:ns:yang:1\" yang:operation=\"none\">\n"
-            "  <list yang:operation=\"create\">\n"
-            "    <name>a</name>\n"
-            "    <value>2</value>\n"
-            "  </list>\n"
             "  <list yang:operation=\"delete\">\n"
             "    <name>c</name>\n"
             "    <value>3</value>\n"
             "  </list>\n"
+            "  <list yang:operation=\"create\">\n"
+            "    <name>a</name>\n"
+            "    <value>2</value>\n"
+            "  </list>\n"
             "</df>\n";
     const char *out_merge =
             "<df xmlns=\"urn:libyang:tests:defaults\" xmlns:yang=\"urn:ietf:params:xml:ns:yang:1\" yang:operation=\"none\">\n"