ovsdb-idlc: Make no-op writes to write-only columns cheaper.

For 1000 tunnels with CFM enabled, this reduces CPU use from
about 36% to about 30%.

Bug #15171.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index fbf4bba..0c644a5 100644 (file)
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1826,10 +1826,10 @@ ovsdb_idl_txn_complete(struct ovsdb_idl_txn *txn,
  * Takes ownership of what 'datum' points to (and in some cases destroys that
  * data before returning) but makes a copy of 'datum' itself.  (Commonly
  * 'datum' is on the caller's stack.) */
-void
-ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
-                    const struct ovsdb_idl_column *column,
-                    struct ovsdb_datum *datum)
+static void
+ovsdb_idl_txn_write__(const struct ovsdb_idl_row *row_,
+                      const struct ovsdb_idl_column *column,
+                      struct ovsdb_datum *datum, bool owns_datum)
 {
     struct ovsdb_idl_row *row = CONST_CAST(struct ovsdb_idl_row *, row_);
     const struct ovsdb_idl_table_class *class;
@@ -1837,8 +1837,7 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
     bool write_only;
 
     if (ovsdb_idl_row_is_synthetic(row)) {
-        ovsdb_datum_destroy(datum, &column->type);
-        return;
+        goto discard_datum;
     }
 
     class = row->table->class;
@@ -1853,8 +1852,7 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
     if (row->table->idl->verify_write_only && !write_only) {
         VLOG_ERR("Bug: Attempt to write to a read/write column (%s:%s) when"
                  " explicitly configured not to.", class->name, column->name);
-        ovsdb_datum_destroy(datum, &column->type);
-        return;
+        goto discard_datum;
     }
 
     /* If this is a write-only column and the datum being written is the same
@@ -1870,8 +1868,7 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
      * ovsdb_idl_txn_commit().) */
     if (write_only && ovsdb_datum_equals(ovsdb_idl_read(row, column),
                                          datum, &column->type)) {
-        ovsdb_datum_destroy(datum, &column->type);
-        return;
+        goto discard_datum;
     }
 
     if (hmap_node_is_null(&row->txn_node)) {
@@ -1889,9 +1886,36 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
     } else {
         bitmap_set1(row->written, column_idx);
     }
-    row->new[column_idx] = *datum;
+    if (owns_datum) {
+        row->new[column_idx] = *datum;
+    } else {
+        ovsdb_datum_clone(&row->new[column_idx], datum, &column->type);
+    }
     (column->unparse)(row);
     (column->parse)(row, &row->new[column_idx]);
+    return;
+
+discard_datum:
+    if (owns_datum) {
+        ovsdb_datum_destroy(datum, &column->type);
+    }
+}
+
+void
+ovsdb_idl_txn_write(const struct ovsdb_idl_row *row,
+                    const struct ovsdb_idl_column *column,
+                    struct ovsdb_datum *datum)
+{
+    ovsdb_idl_txn_write__(row, column, datum, true);
+}
+
+void
+ovsdb_idl_txn_write_clone(const struct ovsdb_idl_row *row,
+                          const struct ovsdb_idl_column *column,
+                          const struct ovsdb_datum *datum)
+{
+    ovsdb_idl_txn_write__(row, column,
+                          CONST_CAST(struct ovsdb_datum *, datum), false);
 }
 
 /* Causes the original contents of 'column' in 'row_' to be verified as a

diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index 27008b7..b428501 100644 (file)
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -202,6 +202,9 @@ const struct uuid *ovsdb_idl_txn_get_insert_uuid(const struct ovsdb_idl_txn *,
 void ovsdb_idl_txn_write(const struct ovsdb_idl_row *,
                          const struct ovsdb_idl_column *,
                          struct ovsdb_datum *);
+void ovsdb_idl_txn_write_clone(const struct ovsdb_idl_row *,
+                               const struct ovsdb_idl_column *,
+                               const struct ovsdb_datum *);
 void ovsdb_idl_txn_delete(const struct ovsdb_idl_row *);
 const struct ovsdb_idl_row *ovsdb_idl_txn_insert(
     struct ovsdb_idl_txn *, const struct ovsdb_idl_table_class *,

diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index dc0839e..1f21950 100755 (executable)
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -517,34 +517,54 @@ void
             print "{"
             print "    struct ovsdb_datum datum;"
             if type.n_min == 1 and type.n_max == 1:
+                print "    union ovsdb_atom key;"
+                if type.value:
+                    print "    union ovsdb_atom value;"
                 print
                 print "    ovs_assert(inited);"
                 print "    datum.n = 1;"
-                print "    datum.keys = xmalloc(sizeof *datum.keys);"
-                print "    " + type.key.copyCValue("datum.keys[0].%s" % type.key.type.to_string(), keyVar)
+                print "    datum.keys = &key;"
+                print "    " + type.key.assign_c_value_casting_away_const("key.%s" % type.key.type.to_string(), keyVar)
                 if type.value:
-                    print "    datum.values = xmalloc(sizeof *datum.values);"
-                    print "    "+ type.value.copyCValue("datum.values[0].%s" % type.value.type.to_string(), valueVar)
+                    print "    datum.values = &value;"
+                    print "    "+ type.value.assign_c_value_casting_away_const("value.%s" % type.value.type.to_string(), valueVar)
                 else:
                     print "    datum.values = NULL;"
+                txn_write_func = "ovsdb_idl_txn_write_clone"
             elif type.is_optional_pointer():
+                print "    union ovsdb_atom key;"
                 print
                 print "    ovs_assert(inited);"
                 print "    if (%s) {" % keyVar
                 print "        datum.n = 1;"
-                print "        datum.keys = xmalloc(sizeof *datum.keys);"
-                print "        " + type.key.copyCValue("datum.keys[0].%s" % type.key.type.to_string(), keyVar)
+                print "        datum.keys = &key;"
+                print "        " + type.key.assign_c_value_casting_away_const("key.%s" % type.key.type.to_string(), keyVar)
+                print "    } else {"
+                print "        datum.n = 0;"
+                print "        datum.keys = NULL;"
+                print "    }"
+                print "    datum.values = NULL;"
+                txn_write_func = "ovsdb_idl_txn_write_clone"
+            elif type.n_max == 1:
+                print "    union ovsdb_atom key;"
+                print
+                print "    ovs_assert(inited);"
+                print "    if (%s) {" % nVar
+                print "        datum.n = 1;"
+                print "        datum.keys = &key;"
+                print "        " + type.key.assign_c_value_casting_away_const("key.%s" % type.key.type.to_string(), "*" + keyVar)
                 print "    } else {"
                 print "        datum.n = 0;"
                 print "        datum.keys = NULL;"
                 print "    }"
                 print "    datum.values = NULL;"
+                txn_write_func = "ovsdb_idl_txn_write_clone"
             else:
                 print "    size_t i;"
                 print
                 print "    ovs_assert(inited);"
                 print "    datum.n = %s;" % nVar
-                print "    datum.keys = xmalloc(%s * sizeof *datum.keys);" % nVar
+                print "    datum.keys = %s ? xmalloc(%s * sizeof *datum.keys) : NULL;" % (nVar, nVar)
                 if type.value:
                     print "    datum.values = xmalloc(%s * sizeof *datum.values);" % nVar
                 else:
@@ -560,8 +580,10 @@ void
                     valueType = "OVSDB_TYPE_VOID"
                 print "    ovsdb_datum_sort_unique(&datum, %s, %s);" % (
                     type.key.toAtomicType(), valueType)
-            print "    ovsdb_idl_txn_write(&row->header_, &%(s)s_columns[%(S)s_COL_%(C)s], &datum);" \
-                % {'s': structName,
+                txn_write_func = "ovsdb_idl_txn_write"
+            print "    %(f)s(&row->header_, &%(s)s_columns[%(S)s_COL_%(C)s], &datum);" \
+                % {'f': txn_write_func,
+                   's': structName,
                    'S': structName.upper(),
                    'C': columnName.upper()}
             print "}"

diff --git a/python/ovs/db/types.py b/python/ovs/db/types.py
index bd1c259..c858471 100644 (file)
--- a/python/ovs/db/types.py
+++ b/python/ovs/db/types.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+# Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -353,6 +353,15 @@ class BaseType(object):
         else:
             return "%(dst)s = %(src)s;" % args
 
+    def assign_c_value_casting_away_const(self, dst, src):
+        args = {'dst': dst, 'src': src}
+        if self.ref_table_name:
+            return ("%(dst)s = %(src)s->header_.uuid;") % args
+        elif self.type == StringType:
+            return "%(dst)s = CONST_CAST(char *, %(src)s);" % args
+        else:
+            return "%(dst)s = %(src)s;" % args
+
     def initCDefault(self, var, is_optional):
         if self.ref_table_name:
             return "%s = NULL;" % var