According to profiler on some loads UUID to string is pretty common
operation and almost always it calls xasprintf in result which calls
vsnprintf twice 1. to calculate length of resulting string 2. to print
result to string. This patch introduces specialized function for UUID
printing and both reduces code duplication and improves performance
exploiting fixed size of output result.

Signed-off-by: Dmitry Porokh <dpor...@nvidia.com>

---
 include/openvswitch/dynamic-string.h |    3 +++
 include/openvswitch/json.h           |    3 +++
 lib/db-ctl-base.c                    |    6 +++---
 lib/dynamic-string.c                 |    8 ++++++++
 lib/json.c                           |   12 ++++++++++++
 lib/ovsdb-cs.c                       |    3 +--
 lib/ovsdb-data.c                     |    5 ++---
 lib/ovsdb-idl.c                      |    6 ++----
 lib/uuid.h                           |   14 ++++++++++++++
 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, 76 insertions(+), 54 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..43db272ce 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 *json, const char *name,
+                          const struct uuid *value);
 
 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..988a85a48 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,13 @@ ds_put_printable(struct ds *ds, const char *s, size_t n)
     }
 }
 
+void
+ds_put_uuid(struct ds *ds, const struct uuid * uuid) {
+    char uuid_str[UUID_LEN + 1];
+    sprintf(uuid_str, UUID_FMT, UUID_ARGS(uuid));
+    ds_put_buffer(ds, uuid_str, UUID_LEN);
+}
+
 /* 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..fa2e25c3a 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,13 @@ 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..206722fbd 100644
--- a/lib/ovsdb-cs.c
+++ b/lib/ovsdb-cs.c
@@ -1546,8 +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)));
+        struct json *json_last_id = json_string_create_uuid(&db->last_id);
         json_array_add(params, json_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..14b9589a1 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,19 @@ uuid_prefix(const struct uuid *uuid, int digits)
     return (uuid->parts[0] >> (32 - 4 * digits));
 }
 
+/* Returns a string representation of UUID.
+ *
+ * String is allocated on heap. Ownership of string
+ * goes to the caller.
+ */
+static inline char *
+uuid_to_string(const struct uuid *uuid)
+{
+    char *data = xmalloc(UUID_LEN+1);
+    sprintf(data, 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);
     }
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to