From patchwork Mon Nov 22 00:09:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1557826 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4Hy71n2bC5z9t1r for ; Mon, 22 Nov 2021 11:09:49 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id C881541BFF; Mon, 22 Nov 2021 00:09:46 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8-k6VX4pQlHF; Mon, 22 Nov 2021 00:09:45 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id BEE3F409F7; Mon, 22 Nov 2021 00:09:44 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E2205C0037; Mon, 22 Nov 2021 00:09:43 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9867FC0012 for ; Mon, 22 Nov 2021 00:09:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 7540240153 for ; Mon, 22 Nov 2021 00:09:42 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Fmz79hVeqWSP for ; Mon, 22 Nov 2021 00:09:41 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp2.osuosl.org (Postfix) with ESMTPS id 41D10400C1 for ; Mon, 22 Nov 2021 00:09:41 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id EA18FFF804; Mon, 22 Nov 2021 00:09:38 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Mon, 22 Nov 2021 01:09:31 +0100 Message-Id: <20211122000932.3041390-2-i.maximets@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211122000932.3041390-1-i.maximets@ovn.org> References: <20211122000932.3041390-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH 1/2] json: Inline clone and destroy functions. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" With the next commit reference counting of json objects will take significant part of the CPU time for ovsdb-server. Inlining them to reduce the cost of a function call. Signed-off-by: Ilya Maximets Acked-by: Dumitru Ceara --- include/openvswitch/json.h | 26 +++++++++++++++++-- lib/json.c | 53 +++++++++++++++----------------------- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/include/openvswitch/json.h b/include/openvswitch/json.h index 0831a9cee..35b403c29 100644 --- a/include/openvswitch/json.h +++ b/include/openvswitch/json.h @@ -110,9 +110,9 @@ double json_real(const struct json *); int64_t json_integer(const struct json *); struct json *json_deep_clone(const struct json *); -struct json *json_clone(const struct json *); +static inline struct json *json_clone(const struct json *); struct json *json_nullable_clone(const struct json *); -void json_destroy(struct json *); +static inline void json_destroy(struct json *); size_t json_hash(const struct json *, size_t basis); bool json_equal(const struct json *, const struct json *); @@ -146,6 +146,28 @@ void json_to_ds(const struct json *, int flags, struct ds *); bool json_string_unescape(const char *in, size_t in_len, char **outp); void json_string_escape(const char *in, struct ds *out); + +/* Inline functions. */ + +/* Returns 'json', with the reference count incremented. */ +static inline struct json * +json_clone(const struct json *json_) +{ + struct json *json = CONST_CAST(struct json *, json_); + json->count++; + return json; +} + +void json_destroy__(struct json *json); + +/* Frees 'json' and everything it points to, recursively. */ +static inline void +json_destroy(struct json *json) +{ + if (json && !--json->count) { + json_destroy__(json); + } +} #ifdef __cplusplus } diff --git a/lib/json.c b/lib/json.c index 0baf7c622..720c73d94 100644 --- a/lib/json.c +++ b/lib/json.c @@ -365,35 +365,33 @@ static void json_destroy_array(struct json_array *array); /* Frees 'json' and everything it points to, recursively. */ void -json_destroy(struct json *json) +json_destroy__(struct json *json) { - if (json && !--json->count) { - switch (json->type) { - case JSON_OBJECT: - json_destroy_object(json->object); - break; + switch (json->type) { + case JSON_OBJECT: + json_destroy_object(json->object); + break; - case JSON_ARRAY: - json_destroy_array(&json->array); - break; + case JSON_ARRAY: + json_destroy_array(&json->array); + break; - case JSON_STRING: - case JSON_SERIALIZED_OBJECT: - free(json->string); - break; + case JSON_STRING: + case JSON_SERIALIZED_OBJECT: + free(json->string); + break; - case JSON_NULL: - case JSON_FALSE: - case JSON_TRUE: - case JSON_INTEGER: - case JSON_REAL: - break; + case JSON_NULL: + case JSON_FALSE: + case JSON_TRUE: + case JSON_INTEGER: + case JSON_REAL: + break; - case JSON_N_TYPES: - OVS_NOT_REACHED(); - } - free(json); + case JSON_N_TYPES: + OVS_NOT_REACHED(); } + free(json); } static void @@ -459,15 +457,6 @@ json_deep_clone(const struct json *json) } } -/* Returns 'json', with the reference count incremented. */ -struct json * -json_clone(const struct json *json_) -{ - struct json *json = CONST_CAST(struct json *, json_); - json->count++; - return json; -} - struct json * json_nullable_clone(const struct json *json) { From patchwork Mon Nov 22 00:09:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1557827 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4Hy71p56xVz9t1r for ; Mon, 22 Nov 2021 11:09:50 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id DD4856061F; Mon, 22 Nov 2021 00:09:48 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cKQQ1dU5Nbx4; Mon, 22 Nov 2021 00:09:47 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id CEAE8607D0; Mon, 22 Nov 2021 00:09:46 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id A2D76C002E; Mon, 22 Nov 2021 00:09:46 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 23801C0012 for ; Mon, 22 Nov 2021 00:09:46 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 0F88B405FF for ; Mon, 22 Nov 2021 00:09:46 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0rJHpmPH0JZd for ; Mon, 22 Nov 2021 00:09:45 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by smtp4.osuosl.org (Postfix) with ESMTPS id 7A8AA40462 for ; Mon, 22 Nov 2021 00:09:44 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 5382BFF804; Mon, 22 Nov 2021 00:09:42 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Mon, 22 Nov 2021 01:09:32 +0100 Message-Id: <20211122000932.3041390-3-i.maximets@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211122000932.3041390-1-i.maximets@ovn.org> References: <20211122000932.3041390-1-i.maximets@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH 2/2] ovsdb-data: Consolidate ovsdb atom and json strings. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" 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 Acked-by: Dumitru Ceara --- 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 #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 #include #include +#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);