Use "growing" `GArray` to store string field's payload

Minimize the number of required operations for functions such as
bt_field_string_set_value(), bt_field_string_append(),
bt_field_string_append_len(), and bt_field_string_clear().

This patch makes the string field object use the same principle that the
sequence field uses, but with a `GArray` of characters instead of a
`GPtrArray` of fields.

Preliminary benchmarks show that this approach is faster than using a
`GString` object.

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/include/babeltrace/ctf-ir/fields-internal.h b/include/babeltrace/ctf-ir/fields-internal.h
index 25310430eba36e65e7700f5f6ad12106f85456fa..2a4c78b0b8bfe4617756ac1291a5d6b853ff9ca6 100644 (file)
--- a/include/babeltrace/ctf-ir/fields-internal.h
+++ b/include/babeltrace/ctf-ir/fields-internal.h
@@ -35,6 +35,7 @@
 #include <babeltrace/babeltrace-internal.h>
 #include <babeltrace/types.h>
 #include <stdint.h>
+#include <string.h>
 #include <inttypes.h>
 #include <stdbool.h>
 #include <glib.h>
@@ -149,7 +150,8 @@ struct bt_field_common_sequence {
 
 struct bt_field_common_string {
        struct bt_field_common common;
-       GString *payload;
+       GArray *buf;
+       size_t size;
 };
 
 struct bt_field_enumeration {
@@ -190,6 +192,12 @@ int bt_field_common_variant_initialize(struct bt_field_common *field,
                bt_field_common_create_func field_create_func,
                GDestroyNotify field_release_func);
 
+BT_HIDDEN
+int bt_field_common_string_initialize(struct bt_field_common *field,
+               struct bt_field_type_common *type,
+               bt_object_release_func release_func,
+               struct bt_field_common_methods *methods);
+
 BT_HIDDEN
 int bt_field_common_generic_validate(struct bt_field_common *field);
 
@@ -220,9 +228,6 @@ void bt_field_common_array_reset_recursive(struct bt_field_common *field);
 BT_HIDDEN
 void bt_field_common_sequence_reset_recursive(struct bt_field_common *field);
 
-BT_HIDDEN
-void bt_field_common_string_reset(struct bt_field_common *field);
-
 BT_HIDDEN
 void bt_field_common_generic_set_is_frozen(struct bt_field_common *field,
                bool is_frozen);
@@ -645,7 +650,7 @@ const char *bt_field_common_string_get_value(struct bt_field_common *field)
        BT_ASSERT_PRE_FIELD_COMMON_IS_SET(field, "String field");
        BT_ASSERT_PRE_FIELD_COMMON_HAS_TYPE_ID(field,
                BT_FIELD_TYPE_ID_STRING, "Field");
-       return string->payload->str;
+       return (const char *) string->buf->data;
 }
 
 static inline
@@ -653,6 +658,7 @@ int bt_field_common_string_set_value(struct bt_field_common *field,
                const char *value)
 {
        struct bt_field_common_string *string = BT_FROM_COMMON(field);
+       size_t str_len;
 
        BT_ASSERT_PRE_NON_NULL(field, "String field");
        BT_ASSERT_PRE_NON_NULL(value, "Value");
@@ -660,34 +666,15 @@ int bt_field_common_string_set_value(struct bt_field_common *field,
        BT_ASSERT_PRE_FIELD_COMMON_HAS_TYPE_ID(field,
                BT_FIELD_TYPE_ID_STRING, "Field");
 
-       if (string->payload) {
-               g_string_assign(string->payload, value);
-       } else {
-               string->payload = g_string_new(value);
-       }
-
-       bt_field_common_set(field, true);
-       return 0;
-}
-
-static inline
-int bt_field_common_string_append(struct bt_field_common *field,
-               const char *value)
-{
-       struct bt_field_common_string *string_field = BT_FROM_COMMON(field);
+       str_len = strlen(value);
 
-       BT_ASSERT_PRE_NON_NULL(field, "String field");
-       BT_ASSERT_PRE_NON_NULL(value, "Value");
-       BT_ASSERT_PRE_FIELD_COMMON_HOT(field, "String field");
-       BT_ASSERT_PRE_FIELD_COMMON_HAS_TYPE_ID(field,
-               BT_FIELD_TYPE_ID_STRING, "Field");
-
-       if (string_field->payload) {
-               g_string_append(string_field->payload, value);
-       } else {
-               string_field->payload = g_string_new(value);
+       if (str_len + 1 > string->buf->len) {
+               g_array_set_size(string->buf, str_len + 1);
        }
 
+       memcpy(string->buf->data, value, str_len);
+       ((char *) string->buf->data)[str_len] = '\0';
+       string->size = str_len;
        bt_field_common_set(field, true);
        return 0;
 }
@@ -697,6 +684,8 @@ int bt_field_common_string_append_len(struct bt_field_common *field,
                const char *value, unsigned int length)
 {
        struct bt_field_common_string *string_field = BT_FROM_COMMON(field);
+       char *data;
+       size_t new_size;
 
        BT_ASSERT_PRE_NON_NULL(field, "String field");
        BT_ASSERT_PRE_NON_NULL(value, "Value");
@@ -704,21 +693,35 @@ int bt_field_common_string_append_len(struct bt_field_common *field,
        BT_ASSERT_PRE_FIELD_COMMON_HAS_TYPE_ID(field,
                BT_FIELD_TYPE_ID_STRING, "Field");
 
-       /* make sure no null bytes are appended */
+       /* Make sure no null bytes are appended */
        BT_ASSERT_PRE(memchr(value, '\0', length) == NULL,
                "String value to append contains a null character: "
                "partial-value=\"%.32s\", length=%u", value, length);
 
-       if (string_field->payload) {
-               g_string_append_len(string_field->payload, value, length);
-       } else {
-               string_field->payload = g_string_new_len(value, length);
+       new_size = string_field->size + length;
+
+       if (new_size + 1 > string_field->buf->len) {
+               g_array_set_size(string_field->buf, new_size + 1);
        }
 
+       data = string_field->buf->data;
+       memcpy(data + string_field->size, value, length);
+       ((char *) string_field->buf->data)[new_size] = '\0';
+       string_field->size = new_size;
        bt_field_common_set(field, true);
        return 0;
 }
 
+static inline
+int bt_field_common_string_append(struct bt_field_common *field,
+               const char *value)
+{
+       BT_ASSERT_PRE_NON_NULL(value, "Value");
+
+       return bt_field_common_string_append_len(field, value,
+               strlen(value));
+}
+
 static inline
 int bt_field_common_string_clear(struct bt_field_common *field)
 {
@@ -728,13 +731,7 @@ int bt_field_common_string_clear(struct bt_field_common *field)
        BT_ASSERT_PRE_FIELD_COMMON_HOT(field, "String field");
        BT_ASSERT_PRE_FIELD_COMMON_HAS_TYPE_ID(field,
                BT_FIELD_TYPE_ID_STRING, "Field");
-
-       if (string_field->payload) {
-               g_string_set_size(string_field->payload, 0);
-       } else {
-               string_field->payload = g_string_new(NULL);
-       }
-
+       string_field->size = 0;
        bt_field_common_set(field, true);
        return 0;
 }
@@ -859,8 +856,8 @@ void bt_field_common_string_finalize(struct bt_field_common *field)
        BT_LOGD("Finalizing common string field object: addr=%p", field);
        bt_field_common_finalize(field);
 
-       if (string->payload) {
-               g_string_free(string->payload, TRUE);
+       if (string->buf) {
+               g_array_free(string->buf, TRUE);
        }
 }
 

diff --git a/lib/ctf-ir/fields.c b/lib/ctf-ir/fields.c
index 5c9ff0b667cd4ae7e482f0b983a042bf42134b19..db2c93b0b252ebe77b25cdc6aac08131ab72bca7 100644 (file)
--- a/lib/ctf-ir/fields.c
+++ b/lib/ctf-ir/fields.c
@@ -75,7 +75,7 @@ static struct bt_field_common_methods bt_field_string_methods = {
        .validate = bt_field_common_generic_validate,
        .copy = NULL,
        .is_set = bt_field_common_generic_is_set,
-       .reset = bt_field_common_string_reset,
+       .reset = bt_field_common_generic_reset,
 };
 
 static struct bt_field_common_methods bt_field_structure_methods = {
@@ -756,6 +756,31 @@ end:
        return ret;
 }
 
+BT_HIDDEN
+int bt_field_common_string_initialize(struct bt_field_common *field,
+               struct bt_field_type_common *type,
+               bt_object_release_func release_func,
+               struct bt_field_common_methods *methods)
+{
+       int ret = 0;
+       struct bt_field_common_string *string = BT_FROM_COMMON(field);
+
+       BT_LOGD("Initializing common string field object: ft-addr=%p", type);
+       bt_field_common_initialize(field, type, release_func, methods);
+       string->buf = g_array_sized_new(FALSE, FALSE, sizeof(char), 1);
+       if (!string->buf) {
+               ret = -1;
+               goto end;
+       }
+
+       g_array_index(string->buf, char, 0) = '\0';
+       BT_LOGD("Initialized common string field object: addr=%p, ft-addr=%p",
+               field, type);
+
+end:
+       return ret;
+}
+
 static
 struct bt_field *bt_field_variant_create(struct bt_field_type *type)
 {
@@ -926,7 +951,7 @@ struct bt_field *bt_field_string_create(struct bt_field_type *type)
        BT_LOGD("Creating string field object: ft-addr=%p", type);
 
        if (string) {
-               bt_field_common_initialize(BT_TO_COMMON(string),
+               bt_field_common_string_initialize(BT_TO_COMMON(string),
                        (void *) type, NULL, &bt_field_string_methods);
                BT_LOGD("Created string field object: addr=%p, ft-addr=%p",
                        string, type);
@@ -1123,19 +1148,6 @@ void bt_field_common_sequence_reset_recursive(struct bt_field_common *field)
        sequence->length = 0;
 }
 
-BT_HIDDEN
-void bt_field_common_string_reset(struct bt_field_common *field)
-{
-       struct bt_field_common_string *string = BT_FROM_COMMON(field);
-
-       BT_ASSERT(field);
-       bt_field_common_generic_reset(field);
-
-       if (string->payload) {
-               g_string_truncate(string->payload, 0);
-       }
-}
-
 BT_HIDDEN
 void bt_field_common_generic_set_is_frozen(struct bt_field_common *field,
                bool is_frozen)

diff --git a/lib/ctf-writer/fields.c b/lib/ctf-writer/fields.c
index 8d92cb6ab7e17d1eaafc009241fdc9e0ee470448..749494cf20da88454b55a283fb4a359cc10a8e60 100644 (file)
--- a/lib/ctf-writer/fields.c
+++ b/lib/ctf-writer/fields.c
@@ -557,8 +557,8 @@ int bt_ctf_field_string_serialize(struct bt_field_common *field,
        BT_LOGV_STR("Creating character field from string field's character field type.");
        character = bt_ctf_field_create(character_type);
 
-       for (i = 0; i < string->payload->len + 1; i++) {
-               const uint64_t chr = (uint64_t) string->payload->str[i];
+       for (i = 0; i < string->size + 1; i++) {
+               const uint64_t chr = (uint64_t) ((char *) string->buf->data)[i];
 
                ret = bt_ctf_field_integer_unsigned_set_value(character, chr);
                BT_ASSERT(ret == 0);
@@ -1097,7 +1097,7 @@ struct bt_ctf_field *bt_ctf_field_string_create(struct bt_ctf_field_type *type)
        BT_LOGD("Creating CTF writer string field object: ft-addr=%p", type);
 
        if (string) {
-               bt_field_common_initialize(BT_TO_COMMON(string),
+               bt_field_common_string_initialize(BT_TO_COMMON(string),
                        (void *) type,
                        (bt_object_release_func)
                                bt_ctf_field_string_destroy,

diff --git a/lib/lib-logging.c b/lib/lib-logging.c
index eedcae710854dc6f12c428ace7c6e7b43d09bcfe..674546011f799d253e0e156f98e9848c8e6f7fab 100644 (file)
--- a/lib/lib-logging.c
+++ b/lib/lib-logging.c
@@ -334,10 +334,10 @@ static inline void format_field_common(char **buf_ch, bool extended,
                struct bt_field_common_string *str =
                        BT_FROM_COMMON(field);
 
-               if (str->payload) {
-                       BT_ASSERT(str->payload);
+               if (str->buf) {
+                       BT_ASSERT(str->buf->data);
                        BUF_APPEND(", %spartial-value=\"%.32s\"",
-                               PRFIELD(str->payload->str));
+                               PRFIELD(str->buf->data));
 
                }
                break;