On 7/1/22 01:34, Ilya Maximets wrote: > Currently ovsdb-server is using shallow copies of some JSON objects > by keeping a reference counter. JSON string objects are also used > directly as ovsdb atoms in database rows to avoid extra copies. > > Taking this approach one step further ovsdb_datum objects can also > be mostly deduplicated by postponing the copy until it actually > needed. datum object itself contains a type and 2 pointers to > data arrays. Adding a one more pointer to a reference counter > we may create a shallow copy of the datum by simply copying type > and pointers and increasing the reference counter.
Hi Ilya, The change makes sense to me! This is just an initial review though, I still want to take some time to try it (and patch 2/2) out. Some initial comments below. > > Before modifying the datum, special function needs to be called > to perform an actual copy of the object, a.k.a. unshare it. > Most of the datum modifications are performed inside the special > functions in ovsdb-data.c, so that is not very hard to track. > A few places like idl and column mutations in ovsdb are accessing I might have missed it but I don't see an unshare() call in the IDL. Is there one needed in the end? > and changing the data directly, so a few extra unshare() calls > has to be added there. > > This change doesn't affect the maximum memory consumption too much, > because most of the copies are short-living. However, not actually > performing these copies saves up to 40% of CPU time on operations > with large sets. > > Reported-at: https://bugzilla.redhat.com/2069089 > Signed-off-by: Ilya Maximets <[email protected]> > --- > lib/db-ctl-base.c | 4 +- > lib/ovsdb-data.c | 115 ++++++++++++++++++++++++++++++++------------ > lib/ovsdb-data.h | 13 ++--- > lib/ovsdb-idl.c | 11 +++-- > lib/ovsdb-types.c | 3 +- > ovsdb/condition.c | 4 +- > ovsdb/monitor.c | 5 +- > ovsdb/mutation.c | 4 +- > ovsdb/ovsdb-idlc.in | 35 +++++++------- > ovsdb/ovsdb-util.c | 6 ++- > ovsdb/row.c | 6 +-- > ovsdb/transaction.c | 8 ++- > tests/test-ovsdb.c | 1 + > 13 files changed, 135 insertions(+), 80 deletions(-) > > diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c > index 707456158..bc85e9921 100644 > --- a/lib/db-ctl-base.c > +++ b/lib/db-ctl-base.c > @@ -1503,7 +1503,7 @@ cmd_add(struct ctl_context *ctx) > } > > type = &column->type; > - ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type); > + ovsdb_datum_clone(&old, ovsdb_idl_read(row, column)); > for (i = 4; i < ctx->argc; i++) { > struct ovsdb_type add_type; > struct ovsdb_datum add; > @@ -1588,7 +1588,7 @@ cmd_remove(struct ctl_context *ctx) > } > > type = &column->type; > - ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type); > + ovsdb_datum_clone(&old, ovsdb_idl_read(row, column)); > for (i = 4; i < ctx->argc; i++) { > struct ovsdb_type rm_type; > struct ovsdb_datum rm; > diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c > index 61ad7679a..d4209e1d4 100644 > --- a/lib/ovsdb-data.c > +++ b/lib/ovsdb-data.c > @@ -896,6 +896,7 @@ ovsdb_datum_init_empty(struct ovsdb_datum *datum) > datum->n = 0; > datum->keys = NULL; > datum->values = NULL; > + datum->refcnt = NULL; > } > > /* Initializes 'datum' as a datum that has the default value for 'type'. > @@ -917,6 +918,7 @@ ovsdb_datum_init_default(struct ovsdb_datum *datum, > datum->n = type->n_min; > datum->keys = alloc_default_atoms(type->key.type, datum->n); > datum->values = alloc_default_atoms(type->value.type, datum->n); > + datum->refcnt = NULL; > } > > /* Returns a read-only datum of the given 'type' that has the default value > for > @@ -928,10 +930,12 @@ const struct ovsdb_datum * > ovsdb_datum_default(const struct ovsdb_type *type) > { > if (type->n_min == 0) { > - static const struct ovsdb_datum empty; > + static unsigned int refcnt = 1; > + static const struct ovsdb_datum empty = { .refcnt = &refcnt }; > return ∅ > } else if (type->n_min == 1) { > static struct ovsdb_datum default_data[OVSDB_N_TYPES][OVSDB_N_TYPES]; > + static unsigned int refcnt[OVSDB_N_TYPES][OVSDB_N_TYPES]; > struct ovsdb_datum *d; > int kt = type->key.type; > int vt = type->value.type; > @@ -946,6 +950,8 @@ ovsdb_datum_default(const struct ovsdb_type *type) > d->values = CONST_CAST(union ovsdb_atom *, > ovsdb_atom_default(vt)); > } > + d->refcnt = &refcnt[kt][vt]; > + *d->refcnt = 1; This doesn't look right, we can have: datum = ovsdb_datum_default(TYPE); /* refcnt = 1 */ ovsdb_datum_clone(datum, clone); /* refcnt = 2 */ datum2 = ovsdb_datum_default(TYPE); /* refcnt = 1 */ ovsdb_datum_unshare(clone, TYPE); /* refcnt = 0 */ > } > return d; > } else { > @@ -999,18 +1005,23 @@ clone_atoms(const union ovsdb_atom *old, enum > ovsdb_atomic_type type, size_t n) > } > } > > -/* Initializes 'new' as a copy of 'old', with the given 'type'. > +/* Initializes 'new' as a shallow copy of 'old_'. > * > * The caller must eventually arrange for 'new' to be destroyed (with > - * ovsdb_datum_destroy()). */ > + * ovsdb_datum_destroy()). The caller must call ovsdb_datum_unshare() > + * before attempting direct modifications of the 'new' or 'old_', i.e. > + * modifications outside of the ovsdb_datum_* API. */ > void > -ovsdb_datum_clone(struct ovsdb_datum *new, const struct ovsdb_datum *old, > - const struct ovsdb_type *type) > +ovsdb_datum_clone(struct ovsdb_datum *new, const struct ovsdb_datum *old_) > { > - unsigned int n = old->n; > - new->n = n; > - new->keys = clone_atoms(old->keys, type->key.type, n); > - new->values = clone_atoms(old->values, type->value.type, n); > + struct ovsdb_datum *old = CONST_CAST(struct ovsdb_datum *, old_); > + > + if (!old->refcnt) { > + old->refcnt = xmalloc(sizeof *old->refcnt); > + *old->refcnt = 1; > + } > + memcpy(new, old, sizeof *new); > + (*new->refcnt)++; > } > > static void > @@ -1037,8 +1048,23 @@ free_data(enum ovsdb_atomic_type type, > void > ovsdb_datum_destroy(struct ovsdb_datum *datum, const struct ovsdb_type *type) > { > - free_data(type->key.type, datum->keys, datum->n); > - free_data(type->value.type, datum->values, datum->n); > + if (!datum->refcnt || !--(*datum->refcnt)) { Nit: it took me a while to decipher but it's better than "!--*datum->refcnt" :) > + free_data(type->key.type, datum->keys, datum->n); > + free_data(type->value.type, datum->values, datum->n); > + free(datum->refcnt); > + } > +} > + > +void > +ovsdb_datum_unshare(struct ovsdb_datum *datum, const struct ovsdb_type *type) > +{ We're expecting other components to call this if needed, would it help to add a comment here too explaining how to use unshare()? > + if (!datum->refcnt || *datum->refcnt == 1) { > + return; > + } > + datum->keys = clone_atoms(datum->keys, type->key.type, datum->n); > + datum->values = clone_atoms(datum->values, type->value.type, datum->n); > + (*datum->refcnt)--; > + datum->refcnt = NULL; > } > > /* Swaps the contents of 'a' and 'b', which need not have the same type. */ > @@ -1107,7 +1133,7 @@ ovsdb_datum_sort__(struct ovsdb_datum *datum, enum > ovsdb_atomic_type key_type, > * caller must free the returned error when it is no longer needed. On > error, > * 'datum' is sorted but not unique. */ > struct ovsdb_error * > -ovsdb_datum_sort(struct ovsdb_datum *datum, enum ovsdb_atomic_type key_type) > +ovsdb_datum_sort(struct ovsdb_datum *datum, const struct ovsdb_type *type) > { > size_t i; > > @@ -1115,11 +1141,13 @@ ovsdb_datum_sort(struct ovsdb_datum *datum, enum > ovsdb_atomic_type key_type) > return NULL; > } > > - ovsdb_datum_sort__(datum, key_type, OVSDB_TYPE_VOID); > + ovsdb_datum_unshare(datum, type); > + > + ovsdb_datum_sort__(datum, type->key.type, OVSDB_TYPE_VOID); > > for (i = 0; i < datum->n - 1; i++) { > if (ovsdb_atom_equals(&datum->keys[i], &datum->keys[i + 1], > - key_type)) { > + type->key.type)) { > if (datum->values) { > return ovsdb_error(NULL, "map contains duplicate key"); > } else { > @@ -1135,9 +1163,9 @@ ovsdb_datum_sort(struct ovsdb_datum *datum, enum > ovsdb_atomic_type key_type) > * this function assert-fails if it actually does. */ > void > ovsdb_datum_sort_assert(struct ovsdb_datum *datum, > - enum ovsdb_atomic_type key_type) > + const struct ovsdb_type *type) > { > - struct ovsdb_error *error = ovsdb_datum_sort(datum, key_type); > + struct ovsdb_error *error = ovsdb_datum_sort(datum, type); > if (error) { > OVS_NOT_REACHED(); > } > @@ -1150,8 +1178,7 @@ ovsdb_datum_sort_assert(struct ovsdb_datum *datum, > * Returns the number of keys (or pairs) that were dropped. */ > size_t > ovsdb_datum_sort_unique(struct ovsdb_datum *datum, > - enum ovsdb_atomic_type key_type, > - enum ovsdb_atomic_type value_type) > + const struct ovsdb_type *type) > { > size_t src, dst; > > @@ -1159,20 +1186,21 @@ ovsdb_datum_sort_unique(struct ovsdb_datum *datum, > return 0; > } > > - ovsdb_datum_sort__(datum, key_type, value_type); > + ovsdb_datum_unshare(datum, type); > + ovsdb_datum_sort__(datum, type->key.type, type->value.type); > > dst = 1; > for (src = 1; src < datum->n; src++) { > if (ovsdb_atom_equals(&datum->keys[src], &datum->keys[dst - 1], > - key_type)) { > - ovsdb_atom_destroy(&datum->keys[src], key_type); > - if (value_type != OVSDB_TYPE_VOID) { > - ovsdb_atom_destroy(&datum->values[src], value_type); > + type->key.type)) { > + ovsdb_atom_destroy(&datum->keys[src], type->key.type); > + if (type->value.type != OVSDB_TYPE_VOID) { > + ovsdb_atom_destroy(&datum->values[src], type->value.type); > } > } else { > if (src != dst) { > datum->keys[dst] = datum->keys[src]; > - if (value_type != OVSDB_TYPE_VOID) { > + if (type->value.type != OVSDB_TYPE_VOID) { > datum->values[dst] = datum->values[src]; > } > } > @@ -1258,6 +1286,7 @@ ovsdb_datum_from_json__(struct ovsdb_datum *datum, > datum->n = 0; > datum->keys = xmalloc(n * sizeof *datum->keys); > datum->values = is_map ? xmalloc(n * sizeof *datum->values) : NULL; > + datum->refcnt = NULL; > for (i = 0; i < n; i++) { > const struct json *element = inner->array.elems[i]; > const struct json *key = NULL; > @@ -1298,6 +1327,7 @@ ovsdb_datum_from_json__(struct ovsdb_datum *datum, > datum->n = 1; > datum->keys = xmalloc(sizeof *datum->keys); > datum->values = NULL; > + datum->refcnt = NULL; > > error = ovsdb_atom_from_json(&datum->keys[0], &type->key, > json, symtab); > @@ -1331,7 +1361,7 @@ ovsdb_datum_from_json(struct ovsdb_datum *datum, > return error; > } > > - error = ovsdb_datum_sort(datum, type->key.type); > + error = ovsdb_datum_sort(datum, type); > if (error) { > ovsdb_datum_destroy(datum, type); > } > @@ -1609,7 +1639,7 @@ ovsdb_datum_from_string(struct ovsdb_datum *datum, > goto error; > } > > - dberror = ovsdb_datum_sort(datum, type->key.type); > + dberror = ovsdb_datum_sort(datum, type); > if (dberror) { > ovsdb_error_destroy(dberror); > if (ovsdb_type_is_map(type)) { > @@ -1687,6 +1717,7 @@ ovsdb_datum_from_smap(struct ovsdb_datum *datum, const > struct smap *smap) > datum->n = smap_count(smap); > datum->keys = xmalloc(datum->n * sizeof *datum->keys); > datum->values = xmalloc(datum->n * sizeof *datum->values); > + datum->refcnt = NULL; > > struct smap_node *node; > size_t i = 0; > @@ -1697,7 +1728,11 @@ ovsdb_datum_from_smap(struct ovsdb_datum *datum, const > struct smap *smap) > } > ovs_assert(i == datum->n); > > - ovsdb_datum_sort_unique(datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING); > + struct ovsdb_type type = { > + OVSDB_BASE_STRING_INIT, OVSDB_BASE_STRING_INIT, > + 0, UINT_MAX > + }; > + ovsdb_datum_sort_unique(datum, &type); > } > > struct ovsdb_error * OVS_WARN_UNUSED_RESULT > @@ -1774,6 +1809,10 @@ ovsdb_datum_compare_3way(const struct ovsdb_datum *a, > return a->n < b->n ? -1 : 1; > } > > + if (a->refcnt && a->refcnt == b->refcnt) { > + return 0; > + } > + Neat! Thanks, Dumitru > cmp = atom_arrays_compare_3way(a->keys, b->keys, type->key.type, a->n); > if (cmp) { > return cmp; > @@ -1893,6 +1932,8 @@ static void > ovsdb_datum_reallocate(struct ovsdb_datum *a, const struct ovsdb_type *type, > unsigned int capacity) > { > + ovsdb_datum_unshare(a, type); > + > a->keys = xrealloc(a->keys, capacity * sizeof *a->keys); > if (type->value.type != OVSDB_TYPE_VOID) { > a->values = xrealloc(a->values, capacity * sizeof *a->values); > @@ -1909,6 +1950,8 @@ void > ovsdb_datum_remove_unsafe(struct ovsdb_datum *datum, size_t idx, > const struct ovsdb_type *type) > { > + ovsdb_datum_unshare(datum, type); > + > ovsdb_atom_destroy(&datum->keys[idx], type->key.type); > datum->keys[idx] = datum->keys[datum->n - 1]; > if (type->value.type != OVSDB_TYPE_VOID) { > @@ -1940,6 +1983,9 @@ ovsdb_datum_add_unsafe(struct ovsdb_datum *datum, > const union ovsdb_atom *range_end_atom) > { > size_t idx = datum->n; > + > + ovsdb_datum_unshare(datum, type); > + > datum->n += range_end_atom ? > (range_end_atom->integer - key->integer + 1) : 1; > datum->keys = xrealloc(datum->keys, datum->n * sizeof *datum->keys); > @@ -1984,6 +2030,8 @@ ovsdb_datum_push_unsafe(struct ovsdb_datum *dst, > return; > } > > + ovsdb_datum_unshare(dst, type); > + > memcpy(&dst->keys[dst->n], &src->keys[start_idx], n * sizeof > src->keys[0]); > if (type->value.type != OVSDB_TYPE_VOID) { > memcpy(&dst->values[dst->n], &src->values[start_idx], > @@ -1999,6 +2047,7 @@ ovsdb_datum_union(struct ovsdb_datum *a, const struct > ovsdb_datum *b, > struct ovsdb_datum result; > unsigned int copied, pos; > > + ovsdb_datum_unshare(a, type); > ovsdb_datum_init_empty(&result); > > copied = 0; > @@ -2050,6 +2099,8 @@ ovsdb_datum_subtract(struct ovsdb_datum *a, const > struct ovsdb_type *a_type, > ovs_assert(a_type->value.type == b_type->value.type > || b_type->value.type == OVSDB_TYPE_VOID); > > + ovsdb_datum_unshare(a, a_type); > + > idx = xmalloc(b->n * sizeof *idx); > n_idx = 0; > for (size_t bi = 0; bi < b->n; bi++) { > @@ -2168,8 +2219,8 @@ ovsdb_datum_added_removed(struct ovsdb_datum *added, > ovsdb_datum_init_empty(added); > ovsdb_datum_init_empty(removed); > if (!ovsdb_type_is_composite(type)) { > - ovsdb_datum_clone(removed, old, type); > - ovsdb_datum_clone(added, new, type); > + ovsdb_datum_clone(removed, old); > + ovsdb_datum_clone(added, new); > return; > } > > @@ -2228,7 +2279,7 @@ ovsdb_datum_diff(struct ovsdb_datum *diff, > > ovsdb_datum_init_empty(diff); > if (!ovsdb_type_is_composite(type)) { > - ovsdb_datum_clone(diff, new, type); > + ovsdb_datum_clone(diff, new); > return; > } > > @@ -2283,10 +2334,12 @@ ovsdb_datum_apply_diff_in_place(struct ovsdb_datum *a, > > if (!ovsdb_type_is_composite(type)) { > ovsdb_datum_destroy(a, type); > - ovsdb_datum_clone(a, diff, type); > + ovsdb_datum_clone(a, diff); > return NULL; > } > > + ovsdb_datum_unshare(a, type); > + > operation = xmalloc(diff->n * sizeof *operation); > idx = xmalloc(diff->n * sizeof *idx); > new_size = a->n; > diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h > index ba5d179a6..dcb620513 100644 > --- a/lib/ovsdb-data.h > +++ b/lib/ovsdb-data.h > @@ -146,6 +146,8 @@ struct ovsdb_datum { > unsigned int n; /* Number of 'keys' and 'values'. */ > union ovsdb_atom *keys; /* Each of the ovsdb_type's 'key_type'. */ > union ovsdb_atom *values; /* Each of the ovsdb_type's 'value_type'. */ > + unsigned int *refcnt; /* Number of copies with the same > + * 'keys' and 'values'. */ > }; > #define OVSDB_DATUM_INITIALIZER { 0, NULL, NULL } > > @@ -155,22 +157,21 @@ void ovsdb_datum_init_default(struct ovsdb_datum *, > const struct ovsdb_type *); > bool ovsdb_datum_is_default(const struct ovsdb_datum *, > const struct ovsdb_type *); > const struct ovsdb_datum *ovsdb_datum_default(const struct ovsdb_type *); > -void ovsdb_datum_clone(struct ovsdb_datum *, const struct ovsdb_datum *, > - const struct ovsdb_type *); > +void ovsdb_datum_clone(struct ovsdb_datum *, const struct ovsdb_datum *); > void ovsdb_datum_destroy(struct ovsdb_datum *, const struct ovsdb_type *); > +void ovsdb_datum_unshare(struct ovsdb_datum *, const struct ovsdb_type *); > void ovsdb_datum_swap(struct ovsdb_datum *, struct ovsdb_datum *); > > /* Checking and maintaining invariants. */ > struct ovsdb_error *ovsdb_datum_sort(struct ovsdb_datum *, > - enum ovsdb_atomic_type key_type) > + const struct ovsdb_type *type) > OVS_WARN_UNUSED_RESULT; > > void ovsdb_datum_sort_assert(struct ovsdb_datum *, > - enum ovsdb_atomic_type key_type); > + const struct ovsdb_type *type); > > size_t ovsdb_datum_sort_unique(struct ovsdb_datum *, > - enum ovsdb_atomic_type key_type, > - enum ovsdb_atomic_type value_type); > + const struct ovsdb_type *type); > > struct ovsdb_error *ovsdb_datum_check_constraints( > const struct ovsdb_datum *, const struct ovsdb_type *) > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index 3b8741408..51dae1d02 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -1088,7 +1088,7 @@ ovsdb_idl_condition_add_clause__(struct > ovsdb_idl_condition *condition, > struct ovsdb_idl_clause *clause = xmalloc(sizeof *clause); > clause->function = src->function; > clause->column = src->column; > - ovsdb_datum_clone(&clause->arg, &src->arg, &src->column->type); > + ovsdb_datum_clone(&clause->arg, &src->arg); > hmap_insert(&condition->clauses, &clause->hmap_node, hash); > } > > @@ -1128,12 +1128,14 @@ ovsdb_idl_condition_add_clause(struct > ovsdb_idl_condition *condition, > struct ovsdb_idl_clause clause = { > .function = function, > .column = column, > - .arg = *arg, > }; > + ovsdb_datum_clone(&clause.arg, arg); > + > uint32_t hash = ovsdb_idl_clause_hash(&clause); > if (!ovsdb_idl_condition_find_clause(condition, &clause, hash)) { > ovsdb_idl_condition_add_clause__(condition, &clause, hash); > } > + ovsdb_datum_destroy(&clause.arg, &column->type); > } > } > > @@ -3611,7 +3613,7 @@ ovsdb_idl_txn_write__(const struct ovsdb_idl_row *row_, > if (owns_datum) { > row->new_datum[column_idx] = *datum; > } else { > - ovsdb_datum_clone(&row->new_datum[column_idx], datum, &column->type); > + ovsdb_datum_clone(&row->new_datum[column_idx], datum); > } > (column->unparse)(row); > (column->parse)(row, &row->new_datum[column_idx]); > @@ -3650,8 +3652,7 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row, > const struct ovsdb_idl_column *column, > struct ovsdb_datum *datum) > { > - ovsdb_datum_sort_unique(datum, > - column->type.key.type, column->type.value.type); > + ovsdb_datum_sort_unique(datum, &column->type); > ovsdb_idl_txn_write__(row, column, datum, true); > } > > diff --git a/lib/ovsdb-types.c b/lib/ovsdb-types.c > index 24ccdd1cc..61efe59cf 100644 > --- a/lib/ovsdb-types.c > +++ b/lib/ovsdb-types.c > @@ -189,8 +189,7 @@ ovsdb_base_type_clone(struct ovsdb_base_type *dst, > > if (src->enum_) { > dst->enum_ = xmalloc(sizeof *dst->enum_); > - ovsdb_datum_clone(dst->enum_, src->enum_, > - ovsdb_base_type_get_enum_type(dst->type)); > + ovsdb_datum_clone(dst->enum_, src->enum_); > } > > switch (dst->type) { > diff --git a/ovsdb/condition.c b/ovsdb/condition.c > index 9aa3788db..d0016fa7f 100644 > --- a/ovsdb/condition.c > +++ b/ovsdb/condition.c > @@ -376,9 +376,7 @@ ovsdb_clause_clone(struct ovsdb_clause *new, struct > ovsdb_clause *old) > { > new->function = old->function; > new->column = old->column; > - ovsdb_datum_clone(&new->arg, > - &old->arg, > - &old->column->type); > + ovsdb_datum_clone(&new->arg, &old->arg); > } > > bool > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c > index 952fa902e..191befcae 100644 > --- a/ovsdb/monitor.c > +++ b/ovsdb/monitor.c > @@ -334,9 +334,8 @@ clone_monitor_row_data(const struct ovsdb_monitor_table > *mt, > const struct ovsdb_column *c = mt->columns[i].column; > const struct ovsdb_datum *src = &row->fields[c->index]; > struct ovsdb_datum *dst = &data[i]; > - const struct ovsdb_type *type = &c->type; > > - ovsdb_datum_clone(dst, src, type); > + ovsdb_datum_clone(dst, src); > } > return data; > } > @@ -359,7 +358,7 @@ update_monitor_row_data(const struct ovsdb_monitor_table > *mt, > > if (!ovsdb_datum_equals(src, dst, type)) { > ovsdb_datum_destroy(dst, type); > - ovsdb_datum_clone(dst, src, type); > + ovsdb_datum_clone(dst, src); > } > } > } > diff --git a/ovsdb/mutation.c b/ovsdb/mutation.c > index 03d1c3499..cbc71bc49 100644 > --- a/ovsdb/mutation.c > +++ b/ovsdb/mutation.c > @@ -287,6 +287,8 @@ mutate_scalar(const struct ovsdb_type *dst_type, struct > ovsdb_datum *dst, > struct ovsdb_error *error; > unsigned int i; > > + ovsdb_datum_unshare(dst, dst_type); > + > if (base->type == OVSDB_TYPE_INTEGER) { > int64_t y = arg->integer; > for (i = 0; i < dst->n; i++) { > @@ -322,7 +324,7 @@ mutate_scalar(const struct ovsdb_type *dst_type, struct > ovsdb_datum *dst, > } > } > > - error = ovsdb_datum_sort(dst, dst_type->key.type); > + error = ovsdb_datum_sort(dst, dst_type); > if (error) { > ovsdb_error_destroy(error); > return ovsdb_error("constraint violation", > diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in > index 13c535939..5a97a8ea3 100755 > --- a/ovsdb/ovsdb-idlc.in > +++ b/ovsdb/ovsdb-idlc.in > @@ -951,7 +951,10 @@ void > 'c': columnName, > 'args': ', '.join(['%(type)s%(name)s' > % m for m in members])}) > + print() > + print(" datum.refcnt = NULL;") > if type.n_min == 1 and type.n_max == 1: > + print() > print(" union ovsdb_atom *key = xmalloc(sizeof *key);") > if type.value: > print(" union ovsdb_atom *value = xmalloc(sizeof > *value);") > @@ -1004,10 +1007,6 @@ void > if type.value: > print(" " + > type.value.copyCValue("datum.values[i].%s" % > type.value.type.to_lvalue_string(), "%s[i]" % valueVar)) > print(" }") > - if type.value: > - valueType = type.value.toAtomicType() > - else: > - valueType = "OVSDB_TYPE_VOID" > txn_write_func = "ovsdb_idl_txn_write" > print(" %(f)s(&row->header_, &%(s)s_col_%(c)s, &datum);" \ > % {'f': txn_write_func, > @@ -1035,6 +1034,7 @@ void > datum->n = 1; > datum->keys = xmalloc(datum->n * sizeof *datum->keys); > datum->values = xmalloc(datum->n * sizeof *datum->values); > + datum->refcnt = NULL; > ''' % {'s': structName, 'c': > columnName,'coltype':column.type.key.to_const_c_type(prefix), > 'valtype':column.type.value.to_const_c_type(prefix), 'S': > structName.upper(), > 'C': columnName.upper(), 't': tableName}) > @@ -1060,6 +1060,7 @@ void > datum->n = 1; > datum->keys = xmalloc(datum->n * sizeof *datum->keys); > datum->values = NULL; > + datum->refcnt = NULL; > ''' % {'s': structName, 'c': > columnName,'coltype':column.type.key.to_const_c_type(prefix), > 'valtype':column.type.value.to_const_c_type(prefix), 'S': > structName.upper(), > 'C': columnName.upper(), 't': tableName}) > @@ -1087,6 +1088,7 @@ void > datum->n = 1; > datum->keys = xmalloc(datum->n * sizeof *datum->values); > datum->values = NULL; > + datum->refcnt = NULL; > ''' % {'s': structName, 'c': columnName, > 'valtype':column.type.key.to_const_c_type(prefix), 't': tableName}) > print(" " + type.key.copyCValue("datum->keys[0].%s" % > type.key.type.to_lvalue_string(), "new_value")) > @@ -1110,6 +1112,7 @@ void > datum->n = 1; > datum->keys = xmalloc(datum->n * sizeof *datum->values); > datum->values = NULL; > + datum->refcnt = NULL; > ''' % {'s': structName, 'c': > columnName,'coltype':column.type.key.to_const_c_type(prefix), > 'valtype':column.type.key.to_const_c_type(prefix), 'S': > structName.upper(), > 'C': columnName.upper(), 't': tableName}) > @@ -1179,8 +1182,11 @@ void > 'args': ', '.join(['%(type)s%(name)s' % m for m in > members])}) > print("{") > print(" struct ovsdb_datum datum;") > + print("") > + print(" datum.refcnt = NULL;") > free = [] > if type.n_min == 1 and type.n_max == 1: > + print() > print(" union ovsdb_atom *key = xmalloc(sizeof *key);") > if type.value: > print(" union ovsdb_atom *value = xmalloc(sizeof > *value);") > @@ -1228,12 +1234,8 @@ void > if type.value: > print(" " + > type.value.copyCValue("datum.values[i].%s" % > type.value.type.to_lvalue_string(), "%s[i]" % valueVar, refTable=False)) > print(" }") > - if type.value: > - valueType = type.value.toAtomicType() > - else: > - valueType = "OVSDB_TYPE_VOID" > - print(" ovsdb_datum_sort_unique(&datum, %s, %s);" % ( > - type.key.toAtomicType(), valueType)) > + print(" ovsdb_datum_sort_unique(&datum, > &%(s)s_col_%(c)s.type);" \ > + % {'s': structName, 'c': columnName}) > > print(""" ovsdb_idl_condition_add_clause(cond, > function, > @@ -1341,6 +1343,7 @@ struct %(s)s * > datum->n = smap_count(%(c)s); > datum->keys = xmalloc(datum->n * sizeof *datum->keys); > datum->values = xmalloc(datum->n * sizeof *datum->values); > + datum->refcnt = NULL; > > i = 0; > SMAP_FOR_EACH (node, %(c)s) { > @@ -1348,7 +1351,7 @@ struct %(s)s * > datum->values[i].s = ovsdb_atom_string_create(node->value); > i++; > } > - ovsdb_datum_sort_unique(datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING); > + ovsdb_datum_sort_unique(datum, &%(s)s_col_%(c)s.type); > } else { > ovsdb_datum_init_empty(datum); > } > @@ -1387,6 +1390,8 @@ struct %(s)s * > 'args': ', '.join(['%(type)s%(name)s' % m for m in > members])}) > print("{") > print(" struct ovsdb_datum datum;") > + print() > + print(" datum.refcnt = NULL;") > if type.n_min == 1 and type.n_max == 1: > print(" union ovsdb_atom *key = xmalloc(sizeof(union > ovsdb_atom));") > if type.value: > @@ -1443,12 +1448,8 @@ struct %(s)s * > if type.value: > print(" " + > type.value.copyCValue("datum.values[i].%s" % > type.value.type.to_lvalue_string(), "%s[i]" % valueVar)) > print(" }") > - if type.value: > - valueType = type.value.toAtomicType() > - else: > - valueType = "OVSDB_TYPE_VOID" > - print(" ovsdb_datum_sort_unique(&datum, %s, %s);" % ( > - type.key.toAtomicType(), valueType)) > + print(" ovsdb_datum_sort_unique(&datum, > &%(s)s_col_%(c)s.type);" > + % {'s': structName, 'c': columnName}) > txn_write_func = "ovsdb_idl_index_write" > print(" %(f)s(CONST_CAST(struct ovsdb_idl_row *, > &row->header_), &%(s)s_columns[ %(S)s_COL_%(C)s ], &datum, > &%(p)stable_classes[%(P)sTABLE_%(T)s]);" \ > % {'f': txn_write_func, > diff --git a/ovsdb/ovsdb-util.c b/ovsdb/ovsdb-util.c > index 6d7be066b..303191dc8 100644 > --- a/ovsdb/ovsdb-util.c > +++ b/ovsdb/ovsdb-util.c > @@ -221,6 +221,8 @@ ovsdb_util_write_singleton(struct ovsdb_row *row, const > char *column_name, > return; > } > > + ovsdb_datum_unshare(datum, &column->type); > + > if (datum->n == 1) { > if (ovsdb_atom_equals(&datum->keys[0], atom, type)) { > return; > @@ -231,6 +233,7 @@ ovsdb_util_write_singleton(struct ovsdb_row *row, const > char *column_name, > datum->n = 1; > datum->keys = xmalloc(sizeof *datum->keys); > datum->values = NULL; > + datum->refcnt = NULL; > } > ovsdb_atom_clone(&datum->keys[0], atom, type); > } > @@ -305,6 +308,7 @@ ovsdb_util_write_string_string_column(struct ovsdb_row > *row, > datum->n = n; > datum->keys = xmalloc(n * sizeof *datum->keys); > datum->values = xmalloc(n * sizeof *datum->values); > + datum->refcnt = NULL; > > for (i = 0; i < n; ++i) { > datum->keys[i].s = ovsdb_atom_string_create_nocopy(keys[i]); > @@ -312,5 +316,5 @@ ovsdb_util_write_string_string_column(struct ovsdb_row > *row, > } > > /* Sort and check constraints. */ > - ovsdb_datum_sort_assert(datum, column->type.key.type); > + ovsdb_datum_sort_assert(datum, &column->type); > } > diff --git a/ovsdb/row.c b/ovsdb/row.c > index f93fe2306..fd50c7e7b 100644 > --- a/ovsdb/row.c > +++ b/ovsdb/row.c > @@ -143,8 +143,7 @@ ovsdb_row_clone(const struct ovsdb_row *old) > SHASH_FOR_EACH (node, &table->schema->columns) { > const struct ovsdb_column *column = node->data; > ovsdb_datum_clone(&new->fields[column->index], > - &old->fields[column->index], > - &column->type); > + &old->fields[column->index]); > } > > struct ovsdb_weak_ref *weak, *clone; > @@ -257,8 +256,7 @@ ovsdb_row_update_columns(struct ovsdb_row *dst, > } else { > ovsdb_datum_destroy(&dst->fields[column->index], &column->type); > ovsdb_datum_clone(&dst->fields[column->index], > - &src->fields[column->index], > - &column->type); > + &src->fields[column->index]); > } > } > return NULL; > diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c > index ab7bb0eb1..6864fe099 100644 > --- a/ovsdb/transaction.c > +++ b/ovsdb/transaction.c > @@ -693,8 +693,7 @@ assess_weak_refs(struct ovsdb_txn *txn, struct > ovsdb_txn_row *txn_row) > ovs_list_init(&weak->src_node); > } > } > - ovsdb_datum_sort_unique(&deleted_refs, column->type.key.type, > - column->type.value.type); > + ovsdb_datum_sort_unique(&deleted_refs, &column->type); > > /* Removing elements that references deleted rows. */ > ovsdb_datum_subtract(datum, &column->type, > @@ -708,7 +707,7 @@ assess_weak_refs(struct ovsdb_txn *txn, struct > ovsdb_txn_row *txn_row) > datum, &column->type); > } else { > ovsdb_datum_init_empty(&removed); > - ovsdb_datum_clone(&added, datum, &column->type); > + ovsdb_datum_clone(&added, datum); > } > > /* Checking added data and creating new references. */ > @@ -732,8 +731,7 @@ assess_weak_refs(struct ovsdb_txn *txn, struct > ovsdb_txn_row *txn_row) > } > if (deleted_refs.n) { > /* Removing all the references that doesn't point to valid rows. > */ > - ovsdb_datum_sort_unique(&deleted_refs, column->type.key.type, > - column->type.value.type); > + ovsdb_datum_sort_unique(&deleted_refs, &column->type); > ovsdb_datum_subtract(datum, &column->type, > &deleted_refs, &column->type); > ovsdb_datum_destroy(&deleted_refs, &column->type); > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c > index 0fef1f978..343833151 100644 > --- a/tests/test-ovsdb.c > +++ b/tests/test-ovsdb.c > @@ -1679,6 +1679,7 @@ do_transact_set_integer(struct ovsdb_row *row, const > char *column_name, > > column = ovsdb_table_schema_get_column(do_transact_table->schema, > column_name); > + ovsdb_datum_unshare(&row->fields[column->index], &column->type); > row->fields[column->index].keys[0].integer = integer; > } > } _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
