From 4d4b475dfb526d016e8162c3079320f4bfe741e7 Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Wed, 6 Jun 2018 20:45:08 -0400 Subject: [PATCH] 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 --- include/babeltrace/ctf-ir/fields-internal.h | 85 ++++++++++----------- lib/ctf-ir/fields.c | 42 ++++++---- lib/ctf-writer/fields.c | 6 +- lib/lib-logging.c | 6 +- 4 files changed, 74 insertions(+), 65 deletions(-) diff --git a/include/babeltrace/ctf-ir/fields-internal.h b/include/babeltrace/ctf-ir/fields-internal.h index 25310430..2a4c78b0 100644 --- a/include/babeltrace/ctf-ir/fields-internal.h +++ b/include/babeltrace/ctf-ir/fields-internal.h @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -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 5c9ff0b6..db2c93b0 100644 --- 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 8d92cb6a..749494cf 100644 --- 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 eedcae71..67454601 100644 --- 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; -- 2.34.1