Looks good to me. I might be inclined to use camel case for the new assign_c_value function, since it's siblings are written that way. It's fine as is though.
Acked-by: Ethan Jackson <et...@nicira.com> On Tue, Mar 5, 2013 at 4:28 PM, Ben Pfaff <b...@nicira.com> wrote: > For 1000 tunnels with CFM enabled, this reduces CPU use from > about 36% to about 30%. > > Signed-off-by: Ben Pfaff <b...@nicira.com> > --- > lib/ovsdb-idl.c | 46 > +++++++++++++++++++++++++++++++++++----------- > lib/ovsdb-idl.h | 5 ++++- > ovsdb/ovsdb-idlc.in | 40 +++++++++++++++++++++++++++++++--------- > python/ovs/db/types.py | 11 ++++++++++- > 4 files changed, 80 insertions(+), 22 deletions(-) > > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index fbf4bba..0c644a5 100644 > --- 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 > --- 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 > --- 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 > --- 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 > -- > 1.7.2.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev