diff options
author | Ilya Maximets <i.maximets@ovn.org> | 2022-07-01 01:34:06 +0200 |
---|---|---|
committer | Ilya Maximets <i.maximets@ovn.org> | 2022-07-13 20:33:07 +0200 |
commit | 485ac63d10f8af22030c8b71de77094eee4f0672 (patch) | |
tree | 031b10b7142be06187882b40d33ddc1d0c8f9f2f /ovsdb | |
parent | 02dabb21f24313cba7669d898b00d2d875eb513f (diff) | |
download | openvswitch-485ac63d10f8af22030c8b71de77094eee4f0672.tar.gz |
ovsdb: Add lazy-copy support for ovsdb_datum objects.
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.
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 ovsdb-server.c and column mutations are accessing
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
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Diffstat (limited to 'ovsdb')
-rw-r--r-- | ovsdb/condition.c | 4 | ||||
-rw-r--r-- | ovsdb/monitor.c | 5 | ||||
-rw-r--r-- | ovsdb/mutation.c | 4 | ||||
-rwxr-xr-x | ovsdb/ovsdb-idlc.in | 35 | ||||
-rw-r--r-- | ovsdb/ovsdb-util.c | 6 | ||||
-rw-r--r-- | ovsdb/row.c | 6 | ||||
-rw-r--r-- | ovsdb/transaction.c | 8 |
7 files changed, 34 insertions, 34 deletions
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); |