[ovs-dev] [PATCH v3] ovsdb: Weak references performance fix

Ben Pfaff blp at ovn.org
Tue Jul 26 17:44:22 UTC 2016


On Wed, Jul 13, 2016 at 05:28:51PM +0000, Rodriguez Betancourt, Esteban wrote:
> Prevents the cloning of rows with outgoing or
> incoming weak references when those rows aren't
> being modified.
> 
> It improves the OVSDB Server performance when
> many rows with weak references are involved
> in a transaction.
> 
> Signed-off-by: Esteban Rodriguez Betancourt <estebarb at hpe.com>
> ---
> 
> In the previous patch I forgot to update the test to use the new function name,
> otherwise this patch is identical to the previous one.

Applied, thanks.  I added a lot of the commentary from your explanation
to the commit message and I folded in the following minor comment style
fixes.

diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index 4fdaded..2e31580 100644
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -442,7 +442,7 @@ ovsdb_txn_update_weak_refs(struct ovsdb_txn *txn OVS_UNUSED,
 {
     struct ovsdb_weak_ref *weak, *next;
 
-    /* Remove the weak references originating in the old version of the row */
+    /* Remove the weak references originating in the old version of the row. */
     if (txn_row->old) {
         LIST_FOR_EACH_SAFE (weak, next, src_node, &txn_row->old->src_refs) {
             ovs_list_remove(&weak->src_node);
@@ -451,22 +451,22 @@ ovsdb_txn_update_weak_refs(struct ovsdb_txn *txn OVS_UNUSED,
         }
     }
 
-    /* Although the originating rows have the responsability of updating the
-     * weak references in the dst, is possible that some source rows aren't
-     * part of the transaction.
-     * In that situation this row needs to move the list of incoming weak
-     * references from the old row into the new one.
+    /* Although the originating rows have the responsibility of updating the
+     * weak references in the dst, it is possible that some source rows aren't
+     * part of the transaction.  In that situation this row needs to move the
+     * list of incoming weak references from the old row into the new one.
      */
     if (txn_row->old && txn_row->new) {
-        /* Move the incoming weak references from old to new */
-        ovs_list_push_back_all(&txn_row->new->dst_refs, &txn_row->old->dst_refs);
+        /* Move the incoming weak references from old to new. */
+        ovs_list_push_back_all(&txn_row->new->dst_refs,
+                               &txn_row->old->dst_refs);
     }
 
-    /* Insert the weak references originating in the new version of the row */
+    /* Insert the weak references originating in the new version of the row. */
     struct ovsdb_row *dst_row;
     if (txn_row->new) {
         LIST_FOR_EACH (weak, src_node, &txn_row->new->src_refs) {
-            /* dst_row MUST exist */
+            /* dst_row MUST exist. */
             dst_row = CONST_CAST(struct ovsdb_row *,
                     ovsdb_table_get_row(weak->dst_table, &weak->dst));
             ovs_list_insert(&dst_row->dst_refs, &weak->dst_node);
@@ -500,7 +500,7 @@ add_weak_ref(const struct ovsdb_row *src_, const struct ovsdb_row *dst_)
     weak->src = src;
     weak->dst_table = dst->table;
     weak->dst = *ovsdb_row_get_uuid(dst);
-    /* The dst_refs list is updated at commit time */
+    /* The dst_refs list is updated at commit time. */
     ovs_list_init(&weak->dst_node);
     ovs_list_push_back(&src->src_refs, &weak->src_node);
 }



More information about the dev mailing list