Re: [ovs-dev] [PATCH] json, ds, uuid: specialized uuid print functions
On Thu, Apr 17, 2025 at 04:42:28PM +, Dmitry Porokh via dev wrote:
> 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
>
Commit message is probably not conformant with the policy: it should
start with capital leter and end with dot.
Two minor comments below.
Thanks.
Adrián
> ---
> 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
> #include
> #include "timeval.h"
> +#include "uuid.h"
> #include "util.h
Re: [ovs-dev] [PATCH] json, ds, uuid: specialized uuid print functions
On Thu, Apr 17, 2025 at 04:42:28PM +, Dmitry Porokh via dev wrote: > 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 Hi Dmitry, It might be nice to add some numbers to the commit message regarding performance improvements. But regardless, this seems like a nice improvement in readability to me. Acked-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] json, ds, uuid: specialized uuid print functions
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
---
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
#include
#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'
