diff options
author | Ilya Maximets <i.maximets@ovn.org> | 2021-09-22 09:28:50 +0200 |
---|---|---|
committer | Ilya Maximets <i.maximets@ovn.org> | 2021-09-24 15:53:46 +0200 |
commit | 429b114c5aadee24ccfb16ad7d824f45cdcea75a (patch) | |
tree | f8127a8d5164bcd9571442718b42e41632c09a58 /ovsdb | |
parent | 32b51326ef9c307b4acd0bacafb0218dd1372f3d (diff) | |
download | openvswitch-429b114c5aadee24ccfb16ad7d824f45cdcea75a.tar.gz |
ovsdb-data: Deduplicate string atoms.
ovsdb-server spends a lot of time cloning atoms for various reasons,
e.g. to create a diff of two rows or to clone a row to the transaction.
All atoms, except for strings, contains a simple value that could be
copied in efficient way, but duplicating strings every time has a
significant performance impact.
Introducing a new reference-counted structure 'ovsdb_atom_string'
that allows to not copy strings every time, but just increase a
reference counter.
This change allows to increase transaction throughput in benchmarks
up to 2x for standalone databases and 3x for clustered databases, i.e.
number of transactions that ovsdb-server can handle per second.
It also noticeably reduces memory consumption of ovsdb-server.
Next step will be to consolidate this structure with json strings,
so we will not need to duplicate strings while converting database
objects to json and back.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Mark D. Gray <mark.d.gray@redhat.com>
Diffstat (limited to 'ovsdb')
-rwxr-xr-x | ovsdb/ovsdb-idlc.in | 109 | ||||
-rw-r--r-- | ovsdb/ovsdb-server.c | 6 | ||||
-rw-r--r-- | ovsdb/ovsdb-util.c | 16 | ||||
-rw-r--r-- | ovsdb/rbac.c | 8 |
4 files changed, 67 insertions, 72 deletions
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in index f19e92915..e589c1bdf 100755 --- a/ovsdb/ovsdb-idlc.in +++ b/ovsdb/ovsdb-idlc.in @@ -577,20 +577,20 @@ static void print(" smap_init(&row->%s);" % columnName) print(" for (size_t i = 0; i < datum->n; i++) {") print(" smap_add(&row->%s," % columnName) - print(" datum->keys[i].string,") - print(" datum->values[i].string);") + print(" datum->keys[i].s->string,") + print(" datum->values[i].s->string);") print(" }") elif (type.n_min == 1 and type.n_max == 1) or type.is_optional_pointer(): print("") print(" if (datum->n >= 1) {") if not type.key.ref_table: - print(" %s = datum->keys[0].%s;" % (keyVar, type.key.type.to_string())) + print(" %s = datum->keys[0].%s;" % (keyVar, type.key.type.to_rvalue_string())) else: print(" %s = %s%s_cast(ovsdb_idl_get_row_arc(row_, &%stable_%s, &datum->keys[0].uuid));" % (keyVar, prefix, type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower())) if valueVar: if not type.value.ref_table: - print(" %s = datum->values[0].%s;" % (valueVar, type.value.type.to_string())) + print(" %s = datum->values[0].%s;" % (valueVar, type.value.type.to_rvalue_string())) else: print(" %s = %s%s_cast(ovsdb_idl_get_row_arc(row_, &%stable_%s, &datum->values[0].uuid));" % (valueVar, prefix, type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower())) print(" } else {") @@ -618,7 +618,7 @@ static void """ % (prefix, type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower())) keySrc = "keyRow" else: - keySrc = "datum->keys[i].%s" % type.key.type.to_string() + keySrc = "datum->keys[i].%s" % type.key.type.to_rvalue_string() if type.value and type.value.ref_table: print("""\ struct %s%s *valueRow = %s%s_cast(ovsdb_idl_get_row_arc(row_, &%stable_%s, &datum->values[i].uuid)); @@ -628,7 +628,7 @@ static void """ % (prefix, type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower())) valueSrc = "valueRow" elif valueVar: - valueSrc = "datum->values[i].%s" % type.value.type.to_string() + valueSrc = "datum->values[i].%s" % type.value.type.to_rvalue_string() print(" if (!row->n_%s) {" % (columnName)) print(" %s = xmalloc(%s * sizeof *%s);" % ( @@ -936,45 +936,45 @@ void 'args': ', '.join(['%(type)s%(name)s' % m for m in members])}) if type.n_min == 1 and type.n_max == 1: - print(" union ovsdb_atom key;") + print(" union ovsdb_atom *key = xmalloc(sizeof *key);") if type.value: - print(" union ovsdb_atom value;") + print(" union ovsdb_atom *value = xmalloc(sizeof *value);") print("") 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(" datum.keys = key;") + print(" " + type.key.copyCValue("key->%s" % type.key.type.to_lvalue_string(), keyVar)) if type.value: - print(" datum.values = &value;") - print(" "+ type.value.assign_c_value_casting_away_const("value.%s" % type.value.type.to_string(), valueVar)) + print(" datum.values = value;") + print(" " + type.value.copyCValue("value->%s" % type.value.type.to_lvalue_string(), valueVar)) else: print(" datum.values = NULL;") - txn_write_func = "ovsdb_idl_txn_write_clone" + txn_write_func = "ovsdb_idl_txn_write" elif type.is_optional_pointer(): - print(" union ovsdb_atom key;") print("") print(" if (%s) {" % keyVar) + print(" union ovsdb_atom *key = xmalloc(sizeof *key);") 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(" datum.keys = key;") + print(" " + type.key.copyCValue("key->%s" % type.key.type.to_lvalue_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" + txn_write_func = "ovsdb_idl_txn_write" elif type.n_max == 1: - print(" union ovsdb_atom key;") print("") print(" if (%s) {" % nVar) + print(" union ovsdb_atom *key = xmalloc(sizeof *key);") 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(" datum.keys = key;") + print(" " + type.key.copyCValue("key->%s" % type.key.type.to_lvalue_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" + txn_write_func = "ovsdb_idl_txn_write" else: print("") print(" datum.n = %s;" % nVar) @@ -984,9 +984,9 @@ void else: print(" datum.values = NULL;") print(" for (size_t i = 0; i < %s; i++) {" % nVar) - print(" " + type.key.copyCValue("datum.keys[i].%s" % type.key.type.to_string(), "%s[i]" % keyVar)) + print(" " + type.key.copyCValue("datum.keys[i].%s" % type.key.type.to_lvalue_string(), "%s[i]" % keyVar)) if type.value: - print(" " + type.value.copyCValue("datum.values[i].%s" % type.value.type.to_string(), "%s[i]" % valueVar)) + 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() @@ -1022,9 +1022,8 @@ void ''' % {'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}) - - print(" "+ type.key.copyCValue("datum->keys[0].%s" % type.key.type.to_string(), "new_key")) - print(" "+ type.value.copyCValue("datum->values[0].%s" % type.value.type.to_string(), "new_value")) + print(" " + type.key.copyCValue("datum->keys[0].%s" % type.key.type.to_lvalue_string(), "new_key")) + print(" " + type.value.copyCValue("datum->values[0].%s" % type.value.type.to_lvalue_string(), "new_value")) print(''' ovsdb_idl_txn_write_partial_map(&row->header_, &%(s)s_col_%(c)s, @@ -1048,8 +1047,7 @@ void ''' % {'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}) - - print(" "+ type.key.copyCValue("datum->keys[0].%s" % type.key.type.to_string(), "delete_key")) + print(" " + type.key.copyCValue("datum->keys[0].%s" % type.key.type.to_lvalue_string(), "delete_key")) print(''' ovsdb_idl_txn_delete_partial_map(&row->header_, &%(s)s_col_%(c)s, @@ -1075,8 +1073,7 @@ void datum->values = 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_string(), "new_value")) + print(" " + type.key.copyCValue("datum->keys[0].%s" % type.key.type.to_lvalue_string(), "new_value")) print(''' ovsdb_idl_txn_write_partial_set(&row->header_, &%(s)s_col_%(c)s, @@ -1100,8 +1097,7 @@ void ''' % {'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}) - - print(" "+ type.key.copyCValue("datum->keys[0].%s" % type.key.type.to_string(), "delete_value")) + print(" " + type.key.copyCValue("datum->keys[0].%s" % type.key.type.to_lvalue_string(), "delete_value")) print(''' ovsdb_idl_txn_delete_partial_set(&row->header_, &%(s)s_col_%(c)s, @@ -1169,37 +1165,36 @@ void print(" struct ovsdb_datum datum;") free = [] if type.n_min == 1 and type.n_max == 1: - print(" union ovsdb_atom key;") + print(" union ovsdb_atom *key = xmalloc(sizeof *key);") if type.value: - print(" union ovsdb_atom value;") + print(" union ovsdb_atom *value = xmalloc(sizeof *value);") print("") 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, refTable=False)) + print(" datum.keys = key;") + print(" " + type.key.copyCValue("key->%s" % type.key.type.to_lvalue_string(), keyVar, refTable=False)) if type.value: - print(" datum.values = &value;") - print(" "+ type.value.assign_c_value_casting_away_const("value.%s" % type.value.type.to_string(), valueVar, refTable=False)) + print(" " + type.value.copyCValue("value.%s" % type.value.type.to_lvalue_string(), valueVar, refTable=False)) else: print(" datum.values = NULL;") elif type.is_optional_pointer(): - print(" union ovsdb_atom key;") print("") print(" if (%s) {" % keyVar) + print(" union ovsdb_atom *key = xmalloc(sizeof *key);") 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, refTable=False)) + print(" datum.keys = key;") + print(" " + type.key.copyCValue("key->%s" % type.key.type.to_lvalue_string(), keyVar, refTable=False)) print(" } else {") print(" datum.n = 0;") print(" datum.keys = NULL;") print(" }") print(" datum.values = NULL;") elif type.n_max == 1: - print(" union ovsdb_atom key;") print("") print(" if (%s) {" % nVar) + print(" union ovsdb_atom *key = xmalloc(sizeof *key);") 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, refTable=False)) + print(" datum.keys = key;") + print(" " + type.key.copyCValue("key->%s" % type.key.type.to_lvalue_string(), "*" + keyVar, refTable=False)) print(" } else {") print(" datum.n = 0;") print(" datum.keys = NULL;") @@ -1208,16 +1203,14 @@ void else: print(" datum.n = %s;" % nVar) print(" datum.keys = %s ? xmalloc(%s * sizeof *datum.keys) : NULL;" % (nVar, nVar)) - free += ['datum.keys'] if type.value: print(" datum.values = xmalloc(%s * sizeof *datum.values);" % nVar) - free += ['datum.values'] else: print(" datum.values = NULL;") print(" for (size_t i = 0; i < %s; i++) {" % nVar) - print(" " + type.key.assign_c_value_casting_away_const("datum.keys[i].%s" % type.key.type.to_string(), "%s[i]" % keyVar, refTable=False)) + print(" " + type.key.copyCValue("datum.keys[i].%s" % type.key.type.to_lvalue_string(), "%s[i]" % keyVar, refTable=False)) if type.value: - print(" " + type.value.assign_c_value_casting_away_const("datum.values[i].%s" % type.value.type.to_string(), "%s[i]" % valueVar, refTable=False)) + 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() @@ -1237,8 +1230,8 @@ void 's': structName, 'S': structName.upper(), 'c': columnName}) - for var in free: - print(" free(%s);" % var) + print(" ovsdb_datum_destroy(&datum, &%(s)s_col_%(c)s.type);" \ + % {'s': structName, 'c': columnName}) print("}") # Index table related functions @@ -1335,8 +1328,8 @@ struct %(s)s * i = 0; SMAP_FOR_EACH (node, %(c)s) { - datum->keys[i].string = node->key; - datum->values[i].string = node->value; + datum->keys[i].s = ovsdb_atom_string_create(node->key); + datum->values[i].s = ovsdb_atom_string_create(node->value); i++; } ovsdb_datum_sort_unique(datum, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING); @@ -1385,10 +1378,10 @@ struct %(s)s * print() 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(" " + type.key.copyCValue("key->%s" % type.key.type.to_lvalue_string(), keyVar)) if type.value: print(" datum.values = value;") - print(" "+ type.value.assign_c_value_casting_away_const("value->%s" % type.value.type.to_string(), valueVar)) + print(" " + type.value.copyCValue("value->%s" % type.value.type.to_lvalue_string(), valueVar)) else: print(" datum.values = NULL;") txn_write_func = "ovsdb_idl_index_write" @@ -1399,7 +1392,7 @@ struct %(s)s * print(" key = xmalloc(sizeof (union ovsdb_atom));") 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(" " + type.key.copyCValue("key->%s" % type.key.type.to_lvalue_string(), keyVar)) print(" } else {") print(" datum.n = 0;") print(" datum.keys = NULL;") @@ -1413,7 +1406,7 @@ struct %(s)s * print(" key = xmalloc(sizeof(union ovsdb_atom));") 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(" " + type.key.copyCValue("key->%s" % type.key.type.to_lvalue_string(), "*" + keyVar)) print(" } else {") print(" datum.n = 0;") print(" datum.keys = NULL;") @@ -1430,9 +1423,9 @@ struct %(s)s * else: print(" datum.values = NULL;") print(" for (i = 0; i < %s; i++) {" % nVar) - print(" " + type.key.copyCValue("datum.keys[i].%s" % type.key.type.to_string(), "%s[i]" % keyVar)) + print(" " + type.key.copyCValue("datum.keys[i].%s" % type.key.type.to_lvalue_string(), "%s[i]" % keyVar)) if type.value: - print(" " + type.value.copyCValue("datum.values[i].%s" % type.value.type.to_string(), "%s[i]" % valueVar)) + 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() diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 0b3d2bb71..b34d97e29 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -904,8 +904,8 @@ query_db_string(const struct shash *all_dbs, const char *name, datum = &row->fields[column->index]; for (i = 0; i < datum->n; i++) { - if (datum->keys[i].string[0]) { - return datum->keys[i].string; + if (datum->keys[i].s->string[0]) { + return datum->keys[i].s->string; } } } @@ -1018,7 +1018,7 @@ query_db_remotes(const char *name, const struct shash *all_dbs, datum = &row->fields[column->index]; for (i = 0; i < datum->n; i++) { - add_remote(remotes, datum->keys[i].string); + add_remote(remotes, datum->keys[i].s->string); } } } else if (column->type.key.type == OVSDB_TYPE_UUID diff --git a/ovsdb/ovsdb-util.c b/ovsdb/ovsdb-util.c index c4075cdae..6d7be066b 100644 --- a/ovsdb/ovsdb-util.c +++ b/ovsdb/ovsdb-util.c @@ -111,13 +111,13 @@ ovsdb_util_read_map_string_column(const struct ovsdb_row *row, for (i = 0; i < datum->n; i++) { atom_key = &datum->keys[i]; - if (!strcmp(atom_key->string, key)) { + if (!strcmp(atom_key->s->string, key)) { atom_value = &datum->values[i]; break; } } - return atom_value ? atom_value->string : NULL; + return atom_value ? atom_value->s->string : NULL; } /* Read string-uuid key-values from a map. Returns the row associated with @@ -143,7 +143,7 @@ ovsdb_util_read_map_string_uuid_column(const struct ovsdb_row *row, const struct ovsdb_datum *datum = &row->fields[column->index]; for (size_t i = 0; i < datum->n; i++) { union ovsdb_atom *atom_key = &datum->keys[i]; - if (!strcmp(atom_key->string, key)) { + if (!strcmp(atom_key->s->string, key)) { const union ovsdb_atom *atom_value = &datum->values[i]; return ovsdb_table_get_row(ref_table, &atom_value->uuid); } @@ -181,7 +181,7 @@ ovsdb_util_read_string_column(const struct ovsdb_row *row, const union ovsdb_atom *atom; atom = ovsdb_util_read_column(row, column_name, OVSDB_TYPE_STRING); - *stringp = atom ? atom->string : NULL; + *stringp = atom ? atom->s->string : NULL; return atom != NULL; } @@ -269,8 +269,10 @@ ovsdb_util_write_string_column(struct ovsdb_row *row, const char *column_name, const char *string) { if (string) { - const union ovsdb_atom atom = { .string = CONST_CAST(char *, string) }; + union ovsdb_atom atom = { + .s = ovsdb_atom_string_create(CONST_CAST(char *, string)) }; ovsdb_util_write_singleton(row, column_name, &atom, OVSDB_TYPE_STRING); + ovsdb_atom_destroy(&atom, OVSDB_TYPE_STRING); } else { ovsdb_util_clear_column(row, column_name); } @@ -305,8 +307,8 @@ ovsdb_util_write_string_string_column(struct ovsdb_row *row, datum->values = xmalloc(n * sizeof *datum->values); for (i = 0; i < n; ++i) { - datum->keys[i].string = keys[i]; - datum->values[i].string = values[i]; + datum->keys[i].s = ovsdb_atom_string_create_nocopy(keys[i]); + datum->values[i].s = ovsdb_atom_string_create_nocopy(values[i]); } /* Sort and check constraints. */ diff --git a/ovsdb/rbac.c b/ovsdb/rbac.c index 2986027c9..ff411675f 100644 --- a/ovsdb/rbac.c +++ b/ovsdb/rbac.c @@ -53,8 +53,8 @@ ovsdb_find_row_by_string_key(const struct ovsdb_table *table, HMAP_FOR_EACH (row, hmap_node, &table->rows) { const struct ovsdb_datum *datum = &row->fields[column->index]; for (size_t i = 0; i < datum->n; i++) { - if (datum->keys[i].string[0] && - !strcmp(key, datum->keys[i].string)) { + if (datum->keys[i].s->string[0] && + !strcmp(key, datum->keys[i].s->string)) { return row; } } @@ -113,7 +113,7 @@ ovsdb_rbac_authorized(const struct ovsdb_row *perms, } for (i = 0; i < datum->n; i++) { - const char *name = datum->keys[i].string; + const char *name = datum->keys[i].s->string; const char *value = NULL; bool is_map; @@ -271,7 +271,7 @@ rbac_column_modification_permitted(const struct ovsdb_column *column, size_t i; for (i = 0; i < modifiable->n; i++) { - char *name = modifiable->keys[i].string; + char *name = modifiable->keys[i].s->string; if (!strcmp(name, column->name)) { return true; |