Re: [ovs-dev] [PATCH v2] json, ds, uuid: Specialized uuid print functions.
On 4/24/25 1:32 AM, Dmitry Porokh via dev wrote:
> According to profiler on some loads UUID to string is pretty common
> operation and almost always it in result calls xasprintf 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. For example, on my laptop
> 10'000'000 calls of uuid_to_string function take 1296 ms when
> 10'000'000 calls of xasprintf with UUID_FMT take 2498 ms.
>
> Signed-off-by: Dmitry Porokh
> ---
> v2 fixes:
> * Commit summary message.
> * Micro-benchmark numbers of performance improvement claims.
> * Two coding style fixes (spaces).
>
> 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(-)
>
I n addition to what Eelco already mentioned, see a few more small
comments below.
Best regards, Ilya Maximets.
> 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);
Remove the 'json' and 'value'. Variables with descriptive types do
not need names in prototypes.
>
> 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);
>
Re: [ovs-dev] [PATCH v2] json, ds, uuid: Specialized uuid print functions.
On 24 Apr 2025, at 1:32, Dmitry Porokh via dev wrote:
Hi Dmitry,
Thanks for the patch! I have some small comments below.
I would change the patch subject a bit, something like:
ovsdb: Introduce and use specialized uuid print functions.
> According to profiler on some loads UUID to string is pretty common
> operation and almost always it in result calls xasprintf 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. For example, on my laptop
> 10'000'000 calls of uuid_to_string function take 1296 ms when
> 10'000'000 calls of xasprintf with UUID_FMT take 2498 ms.
What about rewriting the commit message to be a bit more readable?
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
> ---
> v2 fixes:
> * Commit summary message.
> * Micro-benchmark numbers of performance improvement claims.
> * Two coding style fixes (spaces).
>
> 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->tx
