According to profiling data, converting UUIDs to strings is a frequent
operation in some workloads. This typically results in a call to
xasprintf(), which internally calls vsnprintf() twice, first to
calculate the required buffer size, and then to format the string.
This patch introduces specialized functions for printing UUIDs, which
both reduces code duplication and improves performance.
For example, on my laptop, 10,000,000 calls to the new uuid_to_string()
function takes 1296 ms, while the same number of xasprintf() calls using
UUID_FMT take 2498 ms.
Signed-off-by: Dmitry Porokh <dpor...@nvidia.com>
---
v4:
- sad mistake of snprintf usage in ds_put_uuid fixed
v3:
- More clear commit message & summary
- Zero-copy version of ds_put_uuid
- snprintf instead of sprintf
- Comments and code cosmetic fixes
include/openvswitch/dynamic-string.h | 3 +++
include/openvswitch/json.h | 3 +++
lib/db-ctl-base.c | 6 +++---
lib/dynamic-string.c | 14 ++++++++++++++
lib/json.c | 11 +++++++++++
lib/ovsdb-cs.c | 4 +---
lib/ovsdb-data.c | 5 ++---
lib/ovsdb-idl.c | 6 ++----
lib/uuid.h | 15 +++++++++++++++
ovsdb/jsonrpc-server.c | 21 +++++++--------------
ovsdb/ovsdb-client.c | 3 +--
ovsdb/raft-private.c | 17 +++++++----------
ovsdb/raft-rpc.c | 23 ++++++++++-------------
ovsdb/raft.c | 6 +++---
14 files changed, 82 insertions(+), 55 deletions(-)
diff --git a/include/openvswitch/dynamic-string.h
b/include/openvswitch/dynamic-string.h
index 1c262b049..a1d341d48 100644
--- a/include/openvswitch/dynamic-string.h
+++ b/include/openvswitch/dynamic-string.h
@@ -28,6 +28,8 @@
extern "C" {
#endif
+struct uuid;
+
/* A "dynamic string", that is, a buffer that can be used to construct a
* string across a series of operations that extend or modify it.
*
@@ -58,6 +60,7 @@ void ds_put_format(struct ds *, const char *, ...)
OVS_PRINTF_FORMAT(2, 3);
void ds_put_format_valist(struct ds *, const char *, va_list)
OVS_PRINTF_FORMAT(2, 0);
void ds_put_printable(struct ds *, const char *, size_t);
+void ds_put_uuid(struct ds *, const struct uuid *);
void ds_put_hex(struct ds *ds, const void *buf, size_t size);
void ds_put_hex_dump(struct ds *ds, const void *buf_, size_t size,
uintptr_t ofs, bool ascii);
diff --git a/include/openvswitch/json.h b/include/openvswitch/json.h
index 555440760..097bd057d 100644
--- a/include/openvswitch/json.h
+++ b/include/openvswitch/json.h
@@ -80,6 +80,7 @@ struct json *json_null_create(void);
struct json *json_boolean_create(bool);
struct json *json_string_create(const char *);
struct json *json_string_create_nocopy(char *);
+struct json *json_string_create_uuid(const struct uuid *);
struct json *json_serialized_object_create(const struct json *);
struct json *json_integer_create(long long int);
struct json *json_real_create(double);
@@ -101,6 +102,8 @@ void json_object_put_string(struct json *,
void json_object_put_format(struct json *,
const char *name, const char *format, ...)
OVS_PRINTF_FORMAT(3, 4);
+void json_object_put_uuid(struct json *, const char *name,
+ const struct uuid *);
const char *json_string(const struct json *);
const char *json_serialized_object(const struct json *);
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index de046a4ed..1f157e46c 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1785,7 +1785,7 @@ cmd_create(struct ctl_context *ctx)
return;
}
}
- ds_put_format(&ctx->output, UUID_FMT, UUID_ARGS(&row->uuid));
+ ds_put_uuid(&ctx->output, &row->uuid);
}
/* This function may be used as the 'postprocess' function for commands that
@@ -1809,7 +1809,7 @@ post_create(struct ctl_context *ctx)
real = ovsdb_idl_txn_get_insert_uuid(ctx->txn, &dummy);
if (real) {
ds_clear(&ctx->output);
- ds_put_format(&ctx->output, UUID_FMT, UUID_ARGS(real));
+ ds_put_uuid(&ctx->output, real);
}
ds_put_char(&ctx->output, '\n');
}
@@ -2153,7 +2153,7 @@ cmd_show_row(struct ctl_context *ctx, const struct
ovsdb_idl_row *row,
datum = ovsdb_idl_read(row, show->name_column);
ovsdb_datum_to_string(datum, &show->name_column->type, &ctx->output);
} else {
- ds_put_format(&ctx->output, UUID_FMT, UUID_ARGS(&row->uuid));
+ ds_put_uuid(&ctx->output, &row->uuid);
}
ds_put_char(&ctx->output, '\n');
diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c
index 8e9555a63..37b513966 100644
--- a/lib/dynamic-string.c
+++ b/lib/dynamic-string.c
@@ -21,6 +21,7 @@
#include <string.h>
#include <time.h>
#include "timeval.h"
+#include "uuid.h"
#include "util.h"
/* Initializes 'ds' as an empty string buffer. */
@@ -188,6 +189,19 @@ ds_put_printable(struct ds *ds, const char *s, size_t n)
}
}
+void
+ds_put_uuid(struct ds *ds, const struct uuid *uuid)
+{
+ /* We do not use dp_put_format() here to avoid two calls to
+ * vsnprintf().
+ *
+ * We CAN use UUID_LEN + 1 in snprintf because ds->string always
+ * contains one more byte for '\0'
+ */
+ snprintf(ds_put_uninit(ds, UUID_LEN), UUID_LEN + 1,
+ UUID_FMT, UUID_ARGS(uuid));
+}
+
/* Writes the current time with optional millisecond resolution to 'string'
* based on 'template'.
* The current time is either localtime or UTC based on 'utc'. */
diff --git a/lib/json.c b/lib/json.c
index 001f6e6ab..2649e8e12 100644
--- a/lib/json.c
+++ b/lib/json.c
@@ -189,6 +189,11 @@ json_string_create(const char *s)
return json_string_create_nocopy(xstrdup(s));
}
+struct json *json_string_create_uuid(const struct uuid *uuid)
+{
+ return json_string_create_nocopy(uuid_to_string(uuid));
+}
+
struct json *
json_serialized_object_create(const struct json *src)
{
@@ -342,6 +347,12 @@ json_object_put_format(struct json *json,
va_end(args);
}
+void json_object_put_uuid(struct json *json,
+ const char *name, const struct uuid *uuid)
+{
+ json_object_put(json, name, json_string_create_uuid(uuid));
+}
+
const char *
json_string(const struct json *json)
{
diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
index b5eda88ad..87c6211bf 100644
--- a/lib/ovsdb-cs.c
+++ b/lib/ovsdb-cs.c
@@ -1546,9 +1546,7 @@ ovsdb_cs_send_monitor_request(struct ovsdb_cs *cs, struct
ovsdb_cs_db *db,
json_clone(db->monitor_id),
mrs);
if (version == 3) {
- struct json *json_last_id = json_string_create_nocopy(
- xasprintf(UUID_FMT, UUID_ARGS(&db->last_id)));
- json_array_add(params, json_last_id);
+ json_array_add(params, json_string_create_uuid(&db->last_id));
}
ovsdb_cs_send_request(cs, jsonrpc_create_request(method, params, NULL));
}
diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index aadac6bb2..e4149401f 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -483,8 +483,7 @@ ovsdb_atom_to_json__(const union ovsdb_atom *atom, enum
ovsdb_atomic_type type,
: json_deep_clone(atom->s);
case OVSDB_TYPE_UUID:
- return wrap_json("uuid", json_string_create_nocopy(
- xasprintf(UUID_FMT, UUID_ARGS(&atom->uuid))));
+ return wrap_json("uuid", json_string_create_uuid(&atom->uuid));
case OVSDB_N_TYPES:
default:
@@ -753,7 +752,7 @@ ovsdb_atom_to_string(const union ovsdb_atom *atom, enum
ovsdb_atomic_type type,
break;
case OVSDB_TYPE_UUID:
- ds_put_format(out, UUID_FMT, UUID_ARGS(&atom->uuid));
+ ds_put_uuid(out, &atom->uuid);
break;
case OVSDB_N_TYPES:
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 4c2a3e3aa..226a4b475 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2838,8 +2838,7 @@ where_uuid_equals(const struct uuid *uuid)
json_string_create("=="),
json_array_create_2(
json_string_create("uuid"),
- json_string_create_nocopy(
- xasprintf(UUID_FMT, UUID_ARGS(uuid))))));
+ json_string_create_uuid(uuid))));
}
static const struct ovsdb_idl_row *
@@ -3308,8 +3307,7 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
struct json *value;
if (row->persist_uuid) {
uuid_json = "uuid";
- value = json_string_create_nocopy(
- xasprintf(UUID_FMT, UUID_ARGS(&row->uuid)));
+ value = json_string_create_uuid(&row->uuid);
} else {
uuid_json = "uuid-name";
value = json_string_create_nocopy(
diff --git a/lib/uuid.h b/lib/uuid.h
index fa49354f6..0115241d1 100644
--- a/lib/uuid.h
+++ b/lib/uuid.h
@@ -16,6 +16,7 @@
#ifndef UUID_H
#define UUID_H 1
+#include "util.h"
#include "openvswitch/uuid.h"
#ifdef __cplusplus
@@ -73,6 +74,20 @@ uuid_prefix(const struct uuid *uuid, int digits)
return (uuid->parts[0] >> (32 - 4 * digits));
}
+/* Returns a string representation of a UUID.
+ *
+ * The string is allocated on the heap. Ownership of the string is
+ * transferred to the caller.
+ */
+static inline char *
+uuid_to_string(const struct uuid *uuid)
+{
+ char *data = xmalloc(UUID_LEN + 1);
+
+ snprintf(data, UUID_LEN + 1, UUID_FMT, UUID_ARGS(uuid));
+ return data;
+}
+
void uuid_init(void);
void uuid_generate(struct uuid *);
struct uuid uuid_random(void);
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 26a53898f..ba5a4211b 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1157,8 +1157,7 @@ ovsdb_jsonrpc_session_got_request(struct
ovsdb_jsonrpc_session *s,
const struct uuid *uuid = &s->up.server->uuid;
struct json *result;
- result = json_string_create_nocopy(xasprintf(UUID_FMT,
- UUID_ARGS(uuid)));
+ result = json_string_create_uuid(uuid);
reply = jsonrpc_create_reply(result, request->id);
} else if (!strcmp(request->method, "lock")) {
reply = ovsdb_jsonrpc_session_lock(s, request, OVSDB_LOCK_WAIT);
@@ -1609,10 +1608,8 @@ ovsdb_jsonrpc_monitor_create(struct
ovsdb_jsonrpc_session *s, struct ovsdb *db,
json = json ? json : json_object_create();
if (m->version == OVSDB_MONITOR_V3) {
- struct json *json_last_id = json_string_create_nocopy(
- xasprintf(UUID_FMT,
- UUID_ARGS(ovsdb_monitor_get_last_txnid(
- m->dbmon))));
+ struct json *json_last_id = json_string_create_uuid(
+ ovsdb_monitor_get_last_txnid(m->dbmon));
struct json *json_found = json_boolean_create(!initial);
json = json_array_create_3(json_found, json_last_id, json);
@@ -1752,10 +1749,8 @@ ovsdb_jsonrpc_monitor_cond_change(struct
ovsdb_jsonrpc_session *s,
struct jsonrpc_msg *msg;
struct json *p;
if (m->version == OVSDB_MONITOR_V3) {
- struct json *json_last_id = json_string_create_nocopy(
- xasprintf(UUID_FMT,
- UUID_ARGS(ovsdb_monitor_get_last_txnid(
- m->dbmon))));
+ struct json *json_last_id = json_string_create_uuid(
+ ovsdb_monitor_get_last_txnid(m->dbmon));
p = json_array_create_3(json_clone(m->monitor_id), json_last_id,
update_json);
@@ -1912,10 +1907,8 @@ ovsdb_jsonrpc_monitor_flush_all(struct
ovsdb_jsonrpc_session *s)
struct jsonrpc_msg *msg;
struct json *params;
if (m->version == OVSDB_MONITOR_V3) {
- struct json *json_last_id = json_string_create_nocopy(
- xasprintf(UUID_FMT,
- UUID_ARGS(ovsdb_monitor_get_last_txnid(
- m->dbmon))));
+ struct json *json_last_id = json_string_create_uuid(
+ ovsdb_monitor_get_last_txnid(m->dbmon));
params = json_array_create_3(json_clone(m->monitor_id),
json_last_id, json);
} else {
diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index 3fa1d9afc..5258eccaa 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -1469,8 +1469,7 @@ do_monitor__(struct jsonrpc *rpc, const char *database,
break;
case OVSDB_MONITOR_V3:
method = "monitor_cond_since";
- struct json *json_last_id = json_string_create_nocopy(
- xasprintf(UUID_FMT, UUID_ARGS(last_id)));
+ struct json *json_last_id = json_string_create_uuid(last_id);
json_array_add(monitor, json_last_id);
break;
case OVSDB_MONITOR_VERSION_MAX:
diff --git a/ovsdb/raft-private.c b/ovsdb/raft-private.c
index e685c8103..16ae6d572 100644
--- a/ovsdb/raft-private.c
+++ b/ovsdb/raft-private.c
@@ -312,7 +312,7 @@ raft_entry_to_json(const struct raft_entry *e)
if (raft_entry_has_data(e)) {
json_object_put(json, "data",
json_clone(raft_entry_get_serialized_data(e)));
- json_object_put_format(json, "eid", UUID_FMT, UUID_ARGS(&e->eid));
+ json_object_put_uuid(json, "eid", &e->eid);
}
if (e->servers) {
json_object_put(json, "servers", json_clone(e->servers));
@@ -509,10 +509,9 @@ raft_header_to_json(const struct raft_header *h)
{
struct json *json = json_object_create();
- json_object_put_format(json, "server_id", UUID_FMT, UUID_ARGS(&h->sid));
+ json_object_put_uuid(json, "server_id", &h->sid);
if (!uuid_is_zero(&h->cid)) {
- json_object_put_format(json, "cluster_id",
- UUID_FMT, UUID_ARGS(&h->cid));
+ json_object_put_uuid(json, "cluster_id", &h->cid);
}
json_object_put_string(json, "local_address", h->local_address);
json_object_put_string(json, "name", h->name);
@@ -532,8 +531,7 @@ raft_header_to_json(const struct raft_header *h)
json_object_put(json, "prev_data",
json_clone(raft_entry_get_serialized_data(&h->snap)));
}
- json_object_put_format(json, "prev_eid",
- UUID_FMT, UUID_ARGS(&h->snap.eid));
+ json_object_put_uuid(json, "prev_eid", &h->snap.eid);
if (h->snap.election_timer) {
raft_put_uint64(json, "prev_election_timer",
h->snap.election_timer);
@@ -689,8 +687,7 @@ raft_record_to_json(const struct raft_record *r)
raft_put_uint64(json, "election_timer", r->entry.election_timer);
}
if (!uuid_is_zero(&r->entry.eid)) {
- json_object_put_format(json, "eid",
- UUID_FMT, UUID_ARGS(&r->entry.eid));
+ json_object_put_uuid(json, "eid", &r->entry.eid);
}
break;
@@ -700,7 +697,7 @@ raft_record_to_json(const struct raft_record *r)
case RAFT_REC_VOTE:
raft_put_uint64(json, "term", r->term);
- json_object_put_format(json, "vote", UUID_FMT, UUID_ARGS(&r->sid));
+ json_object_put_uuid(json, "vote", &r->sid);
break;
case RAFT_REC_NOTE:
@@ -713,7 +710,7 @@ raft_record_to_json(const struct raft_record *r)
case RAFT_REC_LEADER:
raft_put_uint64(json, "term", r->term);
- json_object_put_format(json, "leader", UUID_FMT, UUID_ARGS(&r->sid));
+ json_object_put_uuid(json, "leader", &r->sid);
break;
default:
diff --git a/ovsdb/raft-rpc.c b/ovsdb/raft-rpc.c
index 27c3aad99..846727c0a 100644
--- a/ovsdb/raft-rpc.c
+++ b/ovsdb/raft-rpc.c
@@ -333,7 +333,7 @@ raft_vote_reply_to_jsonrpc(const struct raft_vote_reply
*rpy,
struct json *args)
{
raft_put_uint64(args, "term", rpy->term);
- json_object_put_format(args, "vote", UUID_FMT, UUID_ARGS(&rpy->vote));
+ json_object_put_uuid(args, "vote", &rpy->vote);
if (rpy->is_prevote) {
json_object_put(args, "is_prevote", json_boolean_create(true));
}
@@ -476,8 +476,7 @@ raft_remove_server_reply_to_jsonrpc(const struct
raft_remove_server_reply *rpy,
struct json *args)
{
if (!uuid_is_zero(&rpy->target_sid)) {
- json_object_put_format(args, "target_server",
- UUID_FMT, UUID_ARGS(&rpy->target_sid));
+ json_object_put_uuid(args, "target_server", &rpy->target_sid);
}
json_object_put(args, "success", json_boolean_create(rpy->success));
}
@@ -524,8 +523,7 @@ raft_install_snapshot_request_to_jsonrpc(
raft_put_uint64(args, "last_index", rq->last_index);
raft_put_uint64(args, "last_term", rq->last_term);
json_object_put(args, "last_servers", json_clone(rq->last_servers));
- json_object_put_format(args, "last_eid",
- UUID_FMT, UUID_ARGS(&rq->last_eid));
+ json_object_put_uuid(args, "last_eid", &rq->last_eid);
raft_put_uint64(args, "election_timer", rq->election_timer);
json_object_put(args, "data", json_clone(rq->data));
@@ -636,7 +634,7 @@ static void
raft_remove_server_request_to_jsonrpc(
const struct raft_remove_server_request *rq, struct json *args)
{
- json_object_put_format(args, "server_id", UUID_FMT, UUID_ARGS(&rq->sid));
+ json_object_put_uuid(args, "server_id", &rq->sid);
}
static void
@@ -708,8 +706,8 @@ raft_execute_command_request_to_jsonrpc(
const struct raft_execute_command_request *rq, struct json *args)
{
json_object_put(args, "data", json_clone(rq->data));
- json_object_put_format(args, "prereq", UUID_FMT, UUID_ARGS(&rq->prereq));
- json_object_put_format(args, "result", UUID_FMT, UUID_ARGS(&rq->result));
+ json_object_put_uuid(args, "prereq", &rq->prereq);
+ json_object_put_uuid(args, "result", &rq->result);
}
static void
@@ -751,7 +749,7 @@ static void
raft_execute_command_reply_to_jsonrpc(
const struct raft_execute_command_reply *rpy, struct json *args)
{
- json_object_put_format(args, "result", UUID_FMT, UUID_ARGS(&rpy->result));
+ json_object_put_uuid(args, "result", &rpy->result);
json_object_put_string(args, "status",
raft_command_status_to_string(rpy->status));
if (rpy->commit_index) {
@@ -833,13 +831,12 @@ raft_rpc_to_jsonrpc(const struct uuid *cid,
{
struct json *args = json_object_create();
if (!uuid_is_zero(cid)) {
- json_object_put_format(args, "cluster", UUID_FMT, UUID_ARGS(cid));
+ json_object_put_uuid(args, "cluster", cid);
}
if (!uuid_is_zero(&rpc->common.sid)) {
- json_object_put_format(args, "to", UUID_FMT,
- UUID_ARGS(&rpc->common.sid));
+ json_object_put_uuid(args, "to", &rpc->common.sid);
}
- json_object_put_format(args, "from", UUID_FMT, UUID_ARGS(sid));
+ json_object_put_uuid(args, "from", sid);
if (rpc->common.comment) {
json_object_put_string(args, "comment", rpc->common.comment);
}
diff --git a/ovsdb/raft.c b/ovsdb/raft.c
index 7d78f710e..78ae39e84 100644
--- a/ovsdb/raft.c
+++ b/ovsdb/raft.c
@@ -527,7 +527,7 @@ raft_create_cluster(const char *file_name, const char *name,
};
raft_entry_set_parsed_data(&h.snap, data);
shash_add_nocopy(json_object(h.snap.servers),
- xasprintf(UUID_FMT, UUID_ARGS(&h.sid)),
+ uuid_to_string(&h.sid),
json_string_create(local_address));
error = ovsdb_log_write_and_free(log, raft_header_to_json(&h));
raft_header_uninit(&h);
@@ -4846,7 +4846,7 @@ raft_unixctl_cid(struct unixctl_conn *conn,
} else if (uuid_is_zero(&raft->cid)) {
unixctl_command_reply_error(conn, "cluster id not yet known");
} else {
- char *uuid = xasprintf(UUID_FMT, UUID_ARGS(&raft->cid));
+ char *uuid = uuid_to_string(&raft->cid);
unixctl_command_reply(conn, uuid);
free(uuid);
}
@@ -4861,7 +4861,7 @@ raft_unixctl_sid(struct unixctl_conn *conn,
if (!raft) {
unixctl_command_reply_error(conn, "unknown cluster");
} else {
- char *uuid = xasprintf(UUID_FMT, UUID_ARGS(&raft->sid));
+ char *uuid = uuid_to_string(&raft->sid);
unixctl_command_reply(conn, uuid);
free(uuid);
}
--
2.43.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
