[ovs-dev] [PATCH 2/2] ovsdb-data: Consolidate ovsdb atom and json strings.
Ilya Maximets
i.maximets at ovn.org
Mon Nov 22 00:09:32 UTC 2021
ovsdb_atom_string and json_string are basically the same data structure
and ovsdb-server frequently needs to convert one to another. We can
avoid that by using json_string from the beginning for all ovsdb
strings. So, the conversion turns into simple json_clone(), i.e.
increment of a reference counter. This change doesn't give any
significant performance boost, but it improves the code clarity and
may be useful for future development.
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
---
lib/ovsdb-data.c | 27 +++++++++++----------------
lib/ovsdb-data.h | 25 ++++++++-----------------
ovsdb/ovsdb-idlc.in | 5 +++--
ovsdb/ovsdb-server.c | 7 ++++---
ovsdb/rbac.c | 8 ++++----
python/ovs/db/data.py | 10 ++++++----
tests/test-ovsdb.c | 9 +++++----
7 files changed, 41 insertions(+), 50 deletions(-)
diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index 6654ed6de..2832e94ea 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -136,7 +136,7 @@ ovsdb_atom_is_default(const union ovsdb_atom *atom,
return atom->boolean == false;
case OVSDB_TYPE_STRING:
- return atom->s->string[0] == '\0';
+ return json_string(atom->s)[0] == '\0';
case OVSDB_TYPE_UUID:
return uuid_is_zero(&atom->uuid);
@@ -172,8 +172,7 @@ ovsdb_atom_clone(union ovsdb_atom *new, const union ovsdb_atom *old,
break;
case OVSDB_TYPE_STRING:
- new->s = old->s;
- new->s->n_refs++;
+ new->s = json_clone(old->s);
break;
case OVSDB_TYPE_UUID:
@@ -215,7 +214,7 @@ ovsdb_atom_hash(const union ovsdb_atom *atom, enum ovsdb_atomic_type type,
return hash_boolean(atom->boolean, basis);
case OVSDB_TYPE_STRING:
- return hash_string(atom->s->string, basis);
+ return json_hash(atom->s, basis);
case OVSDB_TYPE_UUID:
return hash_int(uuid_hash(&atom->uuid), basis);
@@ -247,7 +246,7 @@ ovsdb_atom_compare_3way(const union ovsdb_atom *a,
return a->boolean - b->boolean;
case OVSDB_TYPE_STRING:
- return a->s == b->s ? 0 : strcmp(a->s->string, b->s->string);
+ return a->s == b->s ? 0 : strcmp(json_string(a->s), json_string(b->s));
case OVSDB_TYPE_UUID:
return uuid_compare_3way(&a->uuid, &b->uuid);
@@ -405,7 +404,7 @@ ovsdb_atom_from_json__(union ovsdb_atom *atom,
case OVSDB_TYPE_STRING:
if (json->type == JSON_STRING) {
- atom->s = ovsdb_atom_string_create(json->string);
+ atom->s = json_clone(json);
return NULL;
}
break;
@@ -474,7 +473,7 @@ ovsdb_atom_to_json(const union ovsdb_atom *atom, enum ovsdb_atomic_type type)
return json_boolean_create(atom->boolean);
case OVSDB_TYPE_STRING:
- return json_string_create(atom->s->string);
+ return json_clone(atom->s);
case OVSDB_TYPE_UUID:
return wrap_json("uuid", json_string_create_nocopy(
@@ -726,14 +725,10 @@ ovsdb_atom_to_string(const union ovsdb_atom *atom, enum ovsdb_atomic_type type,
break;
case OVSDB_TYPE_STRING:
- if (string_needs_quotes(atom->s->string)) {
- struct json json;
-
- json.type = JSON_STRING;
- json.string = atom->s->string;
- json_to_ds(&json, 0, out);
+ if (string_needs_quotes(json_string(atom->s))) {
+ json_to_ds(atom->s, 0, out);
} else {
- ds_put_cstr(out, atom->s->string);
+ ds_put_cstr(out, json_string(atom->s));
}
break;
@@ -755,7 +750,7 @@ ovsdb_atom_to_bare(const union ovsdb_atom *atom, enum ovsdb_atomic_type type,
struct ds *out)
{
if (type == OVSDB_TYPE_STRING) {
- ds_put_cstr(out, atom->s->string);
+ ds_put_cstr(out, json_string(atom->s));
} else {
ovsdb_atom_to_string(atom, type, out);
}
@@ -882,7 +877,7 @@ ovsdb_atom_check_constraints(const union ovsdb_atom *atom,
return NULL;
case OVSDB_TYPE_STRING:
- return check_string_constraints(atom->s->string, &base->string);
+ return check_string_constraints(json_string(atom->s), &base->string);
case OVSDB_TYPE_UUID:
return NULL;
diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h
index f66ed3472..47115a7b8 100644
--- a/lib/ovsdb-data.h
+++ b/lib/ovsdb-data.h
@@ -19,6 +19,7 @@
#include <stdlib.h>
#include "compiler.h"
#include "ovsdb-types.h"
+#include "openvswitch/json.h"
#include "openvswitch/shash.h"
#include "util.h"
@@ -32,25 +33,16 @@ struct ds;
struct ovsdb_symbol_table;
struct smap;
-struct ovsdb_atom_string {
- char *string;
- size_t n_refs;
-};
-
-static inline struct ovsdb_atom_string *
+static inline struct json *
ovsdb_atom_string_create_nocopy(char *str)
{
- struct ovsdb_atom_string *s = xzalloc(sizeof *s);
-
- s->string = str;
- s->n_refs = 1;
- return s;
+ return json_string_create_nocopy(str);
}
-static inline struct ovsdb_atom_string *
+static inline struct json *
ovsdb_atom_string_create(const char *str)
{
- return ovsdb_atom_string_create_nocopy(xstrdup(str));
+ return json_string_create(str);
}
/* One value of an atomic type (given by enum ovs_atomic_type). */
@@ -58,7 +50,7 @@ union ovsdb_atom {
int64_t integer;
double real;
bool boolean;
- struct ovsdb_atom_string *s;
+ struct json *s;
struct uuid uuid;
};
@@ -88,9 +80,8 @@ ovsdb_atom_needs_destruction(enum ovsdb_atomic_type type)
static inline void
ovsdb_atom_destroy(union ovsdb_atom *atom, enum ovsdb_atomic_type type)
{
- if (type == OVSDB_TYPE_STRING && !--atom->s->n_refs) {
- free(atom->s->string);
- free(atom->s);
+ if (type == OVSDB_TYPE_STRING) {
+ json_destroy(atom->s);
}
}
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index e589c1bdf..5a02c8f93 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -192,6 +192,7 @@ def printCIDLHeader(schemaFile):
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
+#include "openvswitch/json.h"
#include "ovsdb-data.h"
#include "ovsdb-idl-provider.h"
#include "smap.h"
@@ -577,8 +578,8 @@ 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].s->string,")
- print(" datum->values[i].s->string);")
+ print(" json_string(datum->keys[i].s),")
+ print(" json_string(datum->values[i].s));")
print(" }")
elif (type.n_min == 1 and type.n_max == 1) or type.is_optional_pointer():
print("")
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index b34d97e29..9fe90592e 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -904,8 +904,9 @@ 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].s->string[0]) {
- return datum->keys[i].s->string;
+ const char *key = json_string(datum->keys[i].s);
+ if (key[0]) {
+ return key;
}
}
}
@@ -1018,7 +1019,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].s->string);
+ add_remote(remotes, json_string(datum->keys[i].s));
}
}
} else if (column->type.key.type == OVSDB_TYPE_UUID
diff --git a/ovsdb/rbac.c b/ovsdb/rbac.c
index ff411675f..a3fe97120 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].s->string[0] &&
- !strcmp(key, datum->keys[i].s->string)) {
+ const char *row_key = json_string(datum->keys[i].s);
+ if (row_key[0] && !strcmp(key, row_key)) {
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].s->string;
+ const char *name = json_string(datum->keys[i].s);
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].s->string;
+ const char *name = json_string(modifiable->keys[i].s);
if (!strcmp(name, column->name)) {
return true;
diff --git a/python/ovs/db/data.py b/python/ovs/db/data.py
index 8db21b837..3e9c5049f 100644
--- a/python/ovs/db/data.py
+++ b/python/ovs/db/data.py
@@ -569,10 +569,11 @@ class Datum(object):
s = []
if self.type.key.type == ovs.db.types.StringType:
- s += ["static struct ovsdb_atom_string %s_key_strings[%d] = {"
+ s += ["static struct json %s_key_strings[%d] = {"
% (name, n)]
for key in sorted(self.values):
- s += [' { .string = "%s", .n_refs = 2 },'
+ s += [' { .type = JSON_STRING, '
+ '.string = "%s", .count = 2 },'
% escapeCString(key.value)]
s += ["};"]
s += ["static union ovsdb_atom %s_keys[%d] = {" % (name, n)]
@@ -587,10 +588,11 @@ class Datum(object):
if self.type.value:
if self.type.value.type == ovs.db.types.StringType:
- s += ["static struct ovsdb_atom_string %s_val_strings[%d] = {"
+ s += ["static struct json %s_val_strings[%d] = {"
% (name, n)]
for k, v in sorted(self.values):
- s += [' { .string = "%s", .n_refs = 2 },'
+ s += [' { .type = JSON_STRING, '
+ '.string = "%s", .count = 2 },'
% escapeCString(v.value)]
s += ["};"]
s += ["static union ovsdb_atom %s_values[%d] = {" % (name, n)]
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 432977159..a40318c7d 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2745,13 +2745,13 @@ print_idl_row_simple2(const struct idltest_simple2 *s, int step)
step, s->name);
for (i = 0; i < smap->n; i++) {
printf("[%s : %s]%s",
- smap->keys[i].s->string, smap->values[i].s->string,
+ json_string(smap->keys[i].s), json_string(smap->values[i].s),
i < smap->n - 1 ? "," : "");
}
printf("] imap=[");
for (i = 0; i < imap->n; i++) {
printf("[%"PRId64" : %s]%s",
- imap->keys[i].integer, imap->values[i].s->string,
+ imap->keys[i].integer, json_string(imap->values[i].s),
i < imap->n - 1 ? "," : "");
}
printf("]\n");
@@ -2821,8 +2821,9 @@ do_idl_partial_update_map_column(struct ovs_cmdl_context *ctx)
myTxn = ovsdb_idl_txn_create(idl);
smap = idltest_simple2_get_smap(myRow, OVSDB_TYPE_STRING,
OVSDB_TYPE_STRING);
- ovs_strlcpy(key_to_delete, smap->keys[0].s->string, sizeof key_to_delete);
- idltest_simple2_update_smap_delkey(myRow, smap->keys[0].s->string);
+ ovs_strlcpy(key_to_delete,
+ json_string(smap->keys[0].s), sizeof key_to_delete);
+ idltest_simple2_update_smap_delkey(myRow, json_string(smap->keys[0].s));
ovsdb_idl_txn_commit_block(myTxn);
ovsdb_idl_txn_destroy(myTxn);
ovsdb_idl_get_initial_snapshot(idl);
--
2.31.1
More information about the dev
mailing list