From f2d3d7e710e6a153551aba42676d1bfc5db6486a Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Mon, 20 Sep 2021 23:39:35 -0700 Subject: [PATCH 01/13] builder: Avoid double string lookup in precompile --- gtk/gtkbuilderprecompile.c | 62 +++++++++++++++----------------------- 1 file changed, 24 insertions(+), 38 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index 74551bb62e..d488094be5 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -32,6 +32,12 @@ typedef enum RECORD_TYPE_TEXT, } RecordTreeType; +typedef struct { + char *string; + int count; + int offset; +} RecordDataString; + typedef struct RecordDataTree RecordDataTree; /* All strings are owned by the string table */ @@ -39,22 +45,16 @@ struct RecordDataTree { RecordDataTree *parent; RecordTreeType type; int n_attributes; - const char *data; - const char **attributes; - const char **values; + RecordDataString *data; + RecordDataString **attributes; + RecordDataString **values; GList *children; }; -typedef struct { - char *string; - int count; - int offset; -} RecordDataString; - static RecordDataTree * -record_data_tree_new (RecordDataTree *parent, - RecordTreeType type, - const char *data) +record_data_tree_new (RecordDataTree *parent, + RecordTreeType type, + RecordDataString *data) { RecordDataTree *tree = g_slice_new0 (RecordDataTree); @@ -84,7 +84,7 @@ record_data_string_free (RecordDataString *s) g_slice_free (RecordDataString, s); } -static const char * +static RecordDataString * record_data_string_lookup (GHashTable *strings, const char *str, gssize len) @@ -104,7 +104,7 @@ record_data_string_lookup (GHashTable *strings, { g_free (copy); s->count++; - return s->string; + return s; } s = g_slice_new (RecordDataString); @@ -112,7 +112,7 @@ record_data_string_lookup (GHashTable *strings, s->count = 1; g_hash_table_insert (strings, s->string, s); - return s->string; + return s; } typedef struct { @@ -139,8 +139,8 @@ record_start_element (GMarkupParseContext *context, data->current = child; child->n_attributes = n_attrs; - child->attributes = g_new (const char *, n_attrs); - child->values = g_new (const char *, n_attrs); + child->attributes = g_new (RecordDataString *, n_attrs); + child->values = g_new (RecordDataString *, n_attrs); for (i = 0; i < n_attrs; i++) { @@ -240,22 +240,8 @@ marshal_uint32 (GString *str, } } -static void -marshal_string (GString *marshaled, - GHashTable *strings, - const char *string) -{ - RecordDataString *s; - - s = g_hash_table_lookup (strings, string); - g_assert (s != NULL); - - marshal_uint32 (marshaled, s->offset); -} - static void marshal_tree (GString *marshaled, - GHashTable *strings, RecordDataTree *tree) { GList *l; @@ -265,7 +251,7 @@ marshal_tree (GString *marshaled, if (tree->parent == NULL) { for (l = g_list_last (tree->children); l != NULL; l = l->prev) - marshal_tree (marshaled, strings, l->data); + marshal_tree (marshaled, l->data); return; } @@ -273,21 +259,21 @@ marshal_tree (GString *marshaled, { case RECORD_TYPE_ELEMENT: marshal_uint32 (marshaled, RECORD_TYPE_ELEMENT); - marshal_string (marshaled, strings, tree->data); + marshal_uint32 (marshaled, tree->data->offset); marshal_uint32 (marshaled, tree->n_attributes); for (i = 0; i < tree->n_attributes; i++) { - marshal_string (marshaled, strings, tree->attributes[i]); - marshal_string (marshaled, strings, tree->values[i]); + marshal_uint32 (marshaled, tree->attributes[i]->offset); + marshal_uint32 (marshaled, tree->values[i]->offset); } for (l = g_list_last (tree->children); l != NULL; l = l->prev) - marshal_tree (marshaled, strings, l->data); + marshal_tree (marshaled, l->data); marshal_uint32 (marshaled, RECORD_TYPE_END_ELEMENT); break; case RECORD_TYPE_TEXT: marshal_uint32 (marshaled, RECORD_TYPE_TEXT); - marshal_string (marshaled, strings, tree->data); + marshal_uint32 (marshaled, tree->data->offset); break; case RECORD_TYPE_END_ELEMENT: default: @@ -359,7 +345,7 @@ _gtk_buildable_parser_precompile (const char *text, g_list_free (string_table); - marshal_tree (marshaled, data.strings, data.root); + marshal_tree (marshaled, data.root); record_data_tree_free (data.root); g_hash_table_destroy (data.strings); From 83dc126565144c26f85de468c57f37a943d4659b Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Tue, 21 Sep 2021 13:57:08 -0700 Subject: [PATCH 02/13] builder: Use a string chunk for precompile Also use an explicit length and avoid g_strndup(). --- gtk/gtkbuilderprecompile.c | 116 +++++++++++++++++++++++-------------- 1 file changed, 72 insertions(+), 44 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index d488094be5..ac00cd19af 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -27,20 +27,21 @@ typedef enum { - RECORD_TYPE_ELEMENT, - RECORD_TYPE_END_ELEMENT, - RECORD_TYPE_TEXT, + RECORD_TYPE_ELEMENT, + RECORD_TYPE_END_ELEMENT, + RECORD_TYPE_TEXT, } RecordTreeType; +/* All strings are owned by the string chunk */ typedef struct { - char *string; + const char *string; + int len; int count; int offset; } RecordDataString; typedef struct RecordDataTree RecordDataTree; -/* All strings are owned by the string table */ struct RecordDataTree { RecordDataTree *parent; RecordTreeType type; @@ -51,6 +52,13 @@ struct RecordDataTree { GList *children; }; +typedef struct { + GHashTable *strings; + GStringChunk *chunks; + RecordDataTree *root; + RecordDataTree *current; +} RecordData; + static RecordDataTree * record_data_tree_new (RecordDataTree *parent, RecordTreeType type, @@ -80,47 +88,74 @@ record_data_tree_free (RecordDataTree *tree) static void record_data_string_free (RecordDataString *s) { - g_free (s->string); g_slice_free (RecordDataString, s); } +static gboolean +record_data_string_equal (gconstpointer _a, + gconstpointer _b) +{ + const RecordDataString *a = _a; + const RecordDataString *b = _b; + + return a->len == b->len && + memcmp (a->string, b->string, a->len) == 0; +} + +/* Copied from g_bytes_hash() */ +static guint +record_data_string_hash (gconstpointer _a) +{ + const RecordDataString *a = _a; + const signed char *p, *e; + guint32 h = 5381; + + for (p = (signed char *)a->string, e = (signed char *)a->string + a->len; p != e; p++) + h = (h << 5) + h + *p; + + return h; +} + +static int +record_data_string_compare (gconstpointer _a, + gconstpointer _b) +{ + const RecordDataString *a = _a; + const RecordDataString *b = _b; + + return b->count - a->count; +} + static RecordDataString * -record_data_string_lookup (GHashTable *strings, +record_data_string_lookup (RecordData *data, const char *str, gssize len) { - char *copy = NULL; - RecordDataString *s; + RecordDataString *s, tmp; - if (len >= 0) - { - /* Ensure str is zero terminated */ - copy = g_strndup (str, len); - str = copy; - } + if (len < 0) + len = strlen (str); - s = g_hash_table_lookup (strings, str); + tmp.string = str; + tmp.len = len; + + s = g_hash_table_lookup (data->strings, &tmp); if (s) { - g_free (copy); s->count++; return s; } s = g_slice_new (RecordDataString); - s->string = copy ? copy : g_strdup (str); + /* The string is zero terminated */ + s->string = g_string_chunk_insert_len (data->chunks, str, len); + s->len = len; s->count = 1; - g_hash_table_insert (strings, s->string, s); + g_hash_table_add (data->strings, s); return s; } -typedef struct { - GHashTable *strings; - RecordDataTree *root; - RecordDataTree *current; -} RecordData; - static void record_start_element (GMarkupParseContext *context, const char *element_name, @@ -135,7 +170,7 @@ record_start_element (GMarkupParseContext *context, int i; child = record_data_tree_new (data->current, RECORD_TYPE_ELEMENT, - record_data_string_lookup (data->strings, element_name, -1)); + record_data_string_lookup (data, element_name, -1)); data->current = child; child->n_attributes = n_attrs; @@ -144,8 +179,8 @@ record_start_element (GMarkupParseContext *context, for (i = 0; i < n_attrs; i++) { - child->attributes[i] = record_data_string_lookup (data->strings, names[i], -1); - child->values[i] = record_data_string_lookup (data->strings, values[i], -1); + child->attributes[i] = record_data_string_lookup (data, names[i], -1); + child->values[i] = record_data_string_lookup (data, values[i], -1); } } @@ -170,7 +205,7 @@ record_text (GMarkupParseContext *context, RecordData *data = user_data; record_data_tree_new (data->current, RECORD_TYPE_TEXT, - record_data_string_lookup (data->strings, text, text_len)); + record_data_string_lookup (data, text, text_len)); } static const GMarkupParser record_parser = @@ -182,16 +217,6 @@ static const GMarkupParser record_parser = NULL, // error, fails immediately }; -static int -compare_string (gconstpointer _a, - gconstpointer _b) -{ - const RecordDataString *a = _a; - const RecordDataString *b = _b; - - return b->count - a->count; -} - static void marshal_uint32 (GString *str, guint32 v) @@ -303,7 +328,9 @@ _gtk_buildable_parser_precompile (const char *text, GString *marshaled; int offset; - data.strings = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, (GDestroyNotify)record_data_string_free); + data.strings = g_hash_table_new_full (record_data_string_hash, record_data_string_equal, + (GDestroyNotify)record_data_string_free, NULL); + data.chunks = g_string_chunk_new (512); data.root = record_data_tree_new (NULL, RECORD_TYPE_ELEMENT, NULL); data.current = data.root; @@ -313,6 +340,7 @@ _gtk_buildable_parser_precompile (const char *text, !g_markup_parse_context_end_parse (ctx, error)) { record_data_tree_free (data.root); + g_string_chunk_free (data.chunks); g_hash_table_destroy (data.strings); g_markup_parse_context_free (ctx); return NULL; @@ -321,15 +349,14 @@ _gtk_buildable_parser_precompile (const char *text, g_markup_parse_context_free (ctx); string_table = g_hash_table_get_values (data.strings); - - string_table = g_list_sort (string_table, compare_string); + string_table = g_list_sort (string_table, record_data_string_compare); offset = 0; for (l = string_table; l != NULL; l = l->next) { RecordDataString *s = l->data; s->offset = offset; - offset += strlen (s->string) + 1; + offset += s->len + 1; } marshaled = g_string_new (""); @@ -340,7 +367,7 @@ _gtk_buildable_parser_precompile (const char *text, for (l = string_table; l != NULL; l = l->next) { RecordDataString *s = l->data; - g_string_append_len (marshaled, s->string, strlen (s->string) + 1); + g_string_append_len (marshaled, s->string, s->len + 1); } g_list_free (string_table); @@ -348,6 +375,7 @@ _gtk_buildable_parser_precompile (const char *text, marshal_tree (marshaled, data.root); record_data_tree_free (data.root); + g_string_chunk_free (data.chunks); g_hash_table_destroy (data.strings); return g_string_free_to_bytes (marshaled); From 8b7d4b423c93f5c2c2d6b43a88533a0a33d19e67 Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Tue, 21 Sep 2021 14:07:06 -0700 Subject: [PATCH 03/13] builder: Combine attribute name and value allocations --- gtk/gtkbuilderprecompile.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index ac00cd19af..994beb7e32 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -48,7 +48,6 @@ struct RecordDataTree { int n_attributes; RecordDataString *data; RecordDataString **attributes; - RecordDataString **values; GList *children; }; @@ -81,7 +80,6 @@ record_data_tree_free (RecordDataTree *tree) { g_list_free_full (tree->children, (GDestroyNotify)record_data_tree_free); g_free (tree->attributes); - g_free (tree->values); g_slice_free (RecordDataTree, tree); } @@ -167,6 +165,7 @@ record_start_element (GMarkupParseContext *context, gsize n_attrs = g_strv_length ((char **)names); RecordData *data = user_data; RecordDataTree *child; + RecordDataString **attr_names, **attr_values; int i; child = record_data_tree_new (data->current, RECORD_TYPE_ELEMENT, @@ -174,13 +173,14 @@ record_start_element (GMarkupParseContext *context, data->current = child; child->n_attributes = n_attrs; - child->attributes = g_new (RecordDataString *, n_attrs); - child->values = g_new (RecordDataString *, n_attrs); + child->attributes = g_new (RecordDataString *, n_attrs * 2); + attr_names = &child->attributes[0]; + attr_values = &child->attributes[n_attrs]; for (i = 0; i < n_attrs; i++) { - child->attributes[i] = record_data_string_lookup (data, names[i], -1); - child->values[i] = record_data_string_lookup (data, values[i], -1); + attr_names[i] = record_data_string_lookup (data, names[i], -1); + attr_values[i] = record_data_string_lookup (data, values[i], -1); } } @@ -271,6 +271,7 @@ marshal_tree (GString *marshaled, { GList *l; int i; + RecordDataString **attr_names, **attr_values; /* Special case the root */ if (tree->parent == NULL) @@ -286,10 +287,13 @@ marshal_tree (GString *marshaled, marshal_uint32 (marshaled, RECORD_TYPE_ELEMENT); marshal_uint32 (marshaled, tree->data->offset); marshal_uint32 (marshaled, tree->n_attributes); + + attr_names = &tree->attributes[0]; + attr_values = &tree->attributes[tree->n_attributes]; for (i = 0; i < tree->n_attributes; i++) { - marshal_uint32 (marshaled, tree->attributes[i]->offset); - marshal_uint32 (marshaled, tree->values[i]->offset); + marshal_uint32 (marshaled, attr_names[i]->offset); + marshal_uint32 (marshaled, attr_values[i]->offset); } for (l = g_list_last (tree->children); l != NULL; l = l->prev) marshal_tree (marshaled, l->data); From c6ecf02a072fa95e3226e9127967387685733bea Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Tue, 21 Sep 2021 14:23:34 -0700 Subject: [PATCH 04/13] builder: Embed text length in precompile --- gtk/gtkbuilderprecompile.c | 53 +++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index 994beb7e32..ebce99ddef 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -38,6 +38,8 @@ typedef struct { int len; int count; int offset; + int text_offset; + gboolean include_len; } RecordDataString; typedef struct RecordDataTree RecordDataTree; @@ -130,6 +132,7 @@ record_data_string_lookup (RecordData *data, gssize len) { RecordDataString *s, tmp; + gboolean include_len = len >= 0; if (len < 0) len = strlen (str); @@ -141,6 +144,7 @@ record_data_string_lookup (RecordData *data, if (s) { s->count++; + s->include_len |= include_len; return s; } @@ -149,6 +153,7 @@ record_data_string_lookup (RecordData *data, s->string = g_string_chunk_insert_len (data->chunks, str, len); s->len = len; s->count = 1; + s->include_len = include_len; g_hash_table_add (data->strings, s); return s; @@ -265,6 +270,24 @@ marshal_uint32 (GString *str, } } +static int +marshal_uint32_len (guint32 v) +{ + if (v < 128) + return 1; + + if (v < (1<<14)) + return 2; + + if (v < (1<<21)) + return 3; + + if (v < (1<<28)) + return 4; + + return 5; +} + static void marshal_tree (GString *marshaled, RecordDataTree *tree) @@ -302,7 +325,7 @@ marshal_tree (GString *marshaled, break; case RECORD_TYPE_TEXT: marshal_uint32 (marshaled, RECORD_TYPE_TEXT); - marshal_uint32 (marshaled, tree->data->offset); + marshal_uint32 (marshaled, tree->data->text_offset); break; case RECORD_TYPE_END_ELEMENT: default: @@ -359,6 +382,13 @@ _gtk_buildable_parser_precompile (const char *text, for (l = string_table; l != NULL; l = l->next) { RecordDataString *s = l->data; + + if (s->include_len) + { + s->text_offset = offset; + offset += marshal_uint32_len (s->len); + } + s->offset = offset; offset += s->len + 1; } @@ -371,6 +401,10 @@ _gtk_buildable_parser_precompile (const char *text, for (l = string_table; l != NULL; l = l->next) { RecordDataString *s = l->data; + + if (s->include_len) + marshal_uint32 (marshaled, s->len); + g_string_append_len (marshaled, s->string, s->len + 1); } @@ -430,6 +464,18 @@ demarshal_string (const char **tree, return strings + offset; } +static const char * +demarshal_text (const char **tree, + const char *strings, + guint32 *len) +{ + guint32 offset = demarshal_uint32 (tree); + const char *str = strings + offset; + + *len = demarshal_uint32 (&str); + return str; +} + static void propagate_error (GtkBuildableParseContext *context, GError **dest, @@ -507,14 +553,15 @@ replay_text (GtkBuildableParseContext *context, const char *strings, GError **error) { + guint32 len; const char *text; GError *tmp_error = NULL; - text = demarshal_string (tree, strings); + text = demarshal_text (tree, strings, &len); (*context->internal_callbacks->text) (NULL, text, - strlen (text), + len, context, &tmp_error); From b962d37f791539c36108e50f5dcc0409e7e7e8a0 Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Tue, 21 Sep 2021 14:26:24 -0700 Subject: [PATCH 05/13] builder: Use a reasonable default string size in precompile --- gtk/gtkbuilderprecompile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index ebce99ddef..56f78476ff 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -393,7 +393,7 @@ _gtk_buildable_parser_precompile (const char *text, offset += s->len + 1; } - marshaled = g_string_new (""); + marshaled = g_string_sized_new (4 + offset + 32); /* Magic marker */ g_string_append_len (marshaled, "GBU\0", 4); marshal_uint32 (marshaled, offset); From 1bfd0e5e3817eaa069663984c897b3d03c1f8726 Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Tue, 21 Sep 2021 14:36:40 -0700 Subject: [PATCH 06/13] builder: Use a GQueue in precompile This avoids g_list_last() and embeds the GList link in the RecordDataTree. --- gtk/gtkbuilderprecompile.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index 56f78476ff..00f2a796ac 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -50,7 +50,8 @@ struct RecordDataTree { int n_attributes; RecordDataString *data; RecordDataString **attributes; - GList *children; + GList link; + GQueue children; }; typedef struct { @@ -70,9 +71,10 @@ record_data_tree_new (RecordDataTree *parent, tree->parent = parent; tree->type = type; tree->data = data; + tree->link.data = tree; if (parent) - parent->children = g_list_prepend (parent->children, tree); + g_queue_push_tail_link (&parent->children, &tree->link); return tree; } @@ -80,7 +82,16 @@ record_data_tree_new (RecordDataTree *parent, static void record_data_tree_free (RecordDataTree *tree) { - g_list_free_full (tree->children, (GDestroyNotify)record_data_tree_free); + GList *l, *next; + + l = tree->children.head; + while (l) + { + next = l->next; + record_data_tree_free (l->data); + l = next; + } + g_free (tree->attributes); g_slice_free (RecordDataTree, tree); } @@ -299,7 +310,7 @@ marshal_tree (GString *marshaled, /* Special case the root */ if (tree->parent == NULL) { - for (l = g_list_last (tree->children); l != NULL; l = l->prev) + for (l = tree->children.head; l != NULL; l = l->next) marshal_tree (marshaled, l->data); return; } @@ -318,7 +329,7 @@ marshal_tree (GString *marshaled, marshal_uint32 (marshaled, attr_names[i]->offset); marshal_uint32 (marshaled, attr_values[i]->offset); } - for (l = g_list_last (tree->children); l != NULL; l = l->prev) + for (l = tree->children.head; l != NULL; l = l->next) marshal_tree (marshaled, l->data); marshal_uint32 (marshaled, RECORD_TYPE_END_ELEMENT); From 4ce07f41ca9235f27bff6a83c082177067679aba Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Tue, 21 Sep 2021 15:13:34 -0700 Subject: [PATCH 07/13] builder: Reduce memory usage in precompile Split the Element and Text nodes into separate structures. --- gtk/gtkbuilderprecompile.c | 180 ++++++++++++++++++++++++------------- 1 file changed, 118 insertions(+), 62 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index 00f2a796ac..6c910d79be 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -30,7 +30,7 @@ typedef enum RECORD_TYPE_ELEMENT, RECORD_TYPE_END_ELEMENT, RECORD_TYPE_TEXT, -} RecordTreeType; +} RecordDataType; /* All strings are owned by the string chunk */ typedef struct { @@ -42,58 +42,110 @@ typedef struct { gboolean include_len; } RecordDataString; -typedef struct RecordDataTree RecordDataTree; - -struct RecordDataTree { - RecordDataTree *parent; - RecordTreeType type; - int n_attributes; - RecordDataString *data; - RecordDataString **attributes; +typedef struct { + RecordDataType type; GList link; +} RecordDataNode; + +typedef struct RecordDataElement RecordDataElement; + +struct RecordDataElement { + RecordDataNode base; + + RecordDataElement *parent; + int n_attributes; + RecordDataString *name; + RecordDataString **attributes; GQueue children; }; +typedef struct { + RecordDataNode base; + + RecordDataString *string; +} RecordDataText; + typedef struct { GHashTable *strings; GStringChunk *chunks; - RecordDataTree *root; - RecordDataTree *current; + RecordDataElement *root; + RecordDataElement *current; } RecordData; -static RecordDataTree * -record_data_tree_new (RecordDataTree *parent, - RecordTreeType type, - RecordDataString *data) +static gpointer +record_data_node_new (RecordDataElement *parent, + RecordDataType type, + gsize size) { - RecordDataTree *tree = g_slice_new0 (RecordDataTree); + RecordDataNode *node = g_slice_alloc0 (size); - tree->parent = parent; - tree->type = type; - tree->data = data; - tree->link.data = tree; + node->type = type; + node->link.data = node; if (parent) - g_queue_push_tail_link (&parent->children, &tree->link); + g_queue_push_tail_link (&parent->children, &node->link); - return tree; + return node; +} + +static RecordDataElement * +record_data_element_new (RecordDataElement *parent, + RecordDataString *name) +{ + RecordDataElement *element; + + element = record_data_node_new (parent, + RECORD_TYPE_ELEMENT, + sizeof (RecordDataElement)); + element->parent = parent; + element->name = name; + + return element; } static void -record_data_tree_free (RecordDataTree *tree) +record_data_element_append_text (RecordDataElement *parent, + RecordDataString *string) +{ + RecordDataText *text; + + text = record_data_node_new (parent, + RECORD_TYPE_TEXT, + sizeof (RecordDataText)); + text->string = string; +} + +static void +record_data_node_free (RecordDataNode *node) { GList *l, *next; + RecordDataText *text; + RecordDataElement *element; - l = tree->children.head; - while (l) + switch (node->type) { - next = l->next; - record_data_tree_free (l->data); - l = next; - } + case RECORD_TYPE_ELEMENT: + element = (RecordDataElement *)node; - g_free (tree->attributes); - g_slice_free (RecordDataTree, tree); + l = element->children.head; + while (l) + { + next = l->next; + record_data_node_free (l->data); + l = next; + } + + g_free (element->attributes); + g_slice_free (RecordDataElement, element); + break; + case RECORD_TYPE_TEXT: + text = (RecordDataText *)node; + g_slice_free (RecordDataText, text); + break; + case RECORD_TYPE_END_ELEMENT: + default: + g_assert_not_reached (); + } } static void @@ -180,12 +232,12 @@ record_start_element (GMarkupParseContext *context, { gsize n_attrs = g_strv_length ((char **)names); RecordData *data = user_data; - RecordDataTree *child; - RecordDataString **attr_names, **attr_values; + RecordDataElement *child; + RecordDataString *name, **attr_names, **attr_values; int i; - child = record_data_tree_new (data->current, RECORD_TYPE_ELEMENT, - record_data_string_lookup (data, element_name, -1)); + name = record_data_string_lookup (data, element_name, -1); + child = record_data_element_new (data->current, name); data->current = child; child->n_attributes = n_attrs; @@ -219,9 +271,10 @@ record_text (GMarkupParseContext *context, GError **error) { RecordData *data = user_data; + RecordDataString *string; - record_data_tree_new (data->current, RECORD_TYPE_TEXT, - record_data_string_lookup (data, text, text_len)); + string = record_data_string_lookup (data, text, text_len); + record_data_element_append_text (data->current, string); } static const GMarkupParser record_parser = @@ -301,42 +354,45 @@ marshal_uint32_len (guint32 v) static void marshal_tree (GString *marshaled, - RecordDataTree *tree) + RecordDataNode *node) { GList *l; int i; + RecordDataText *text; + RecordDataElement *element; RecordDataString **attr_names, **attr_values; - /* Special case the root */ - if (tree->parent == NULL) - { - for (l = tree->children.head; l != NULL; l = l->next) - marshal_tree (marshaled, l->data); - return; - } - - switch (tree->type) + switch (node->type) { case RECORD_TYPE_ELEMENT: - marshal_uint32 (marshaled, RECORD_TYPE_ELEMENT); - marshal_uint32 (marshaled, tree->data->offset); - marshal_uint32 (marshaled, tree->n_attributes); + element = (RecordDataElement *)node; - attr_names = &tree->attributes[0]; - attr_values = &tree->attributes[tree->n_attributes]; - for (i = 0; i < tree->n_attributes; i++) + /* Special case the root */ + if (element->parent != NULL) { - marshal_uint32 (marshaled, attr_names[i]->offset); - marshal_uint32 (marshaled, attr_values[i]->offset); + marshal_uint32 (marshaled, RECORD_TYPE_ELEMENT); + marshal_uint32 (marshaled, element->name->offset); + marshal_uint32 (marshaled, element->n_attributes); + + attr_names = &element->attributes[0]; + attr_values = &element->attributes[element->n_attributes]; + for (i = 0; i < element->n_attributes; i++) + { + marshal_uint32 (marshaled, attr_names[i]->offset); + marshal_uint32 (marshaled, attr_values[i]->offset); + } } - for (l = tree->children.head; l != NULL; l = l->next) + + for (l = element->children.head; l != NULL; l = l->next) marshal_tree (marshaled, l->data); - marshal_uint32 (marshaled, RECORD_TYPE_END_ELEMENT); + if (element->parent != NULL) + marshal_uint32 (marshaled, RECORD_TYPE_END_ELEMENT); break; case RECORD_TYPE_TEXT: + text = (RecordDataText *)node; marshal_uint32 (marshaled, RECORD_TYPE_TEXT); - marshal_uint32 (marshaled, tree->data->text_offset); + marshal_uint32 (marshaled, text->string->text_offset); break; case RECORD_TYPE_END_ELEMENT: default: @@ -369,7 +425,7 @@ _gtk_buildable_parser_precompile (const char *text, data.strings = g_hash_table_new_full (record_data_string_hash, record_data_string_equal, (GDestroyNotify)record_data_string_free, NULL); data.chunks = g_string_chunk_new (512); - data.root = record_data_tree_new (NULL, RECORD_TYPE_ELEMENT, NULL); + data.root = record_data_element_new (NULL, NULL); data.current = data.root; ctx = g_markup_parse_context_new (&record_parser, G_MARKUP_TREAT_CDATA_AS_TEXT, &data, NULL); @@ -377,7 +433,7 @@ _gtk_buildable_parser_precompile (const char *text, if (!g_markup_parse_context_parse (ctx, text, text_len, error) || !g_markup_parse_context_end_parse (ctx, error)) { - record_data_tree_free (data.root); + record_data_node_free (&data.root->base); g_string_chunk_free (data.chunks); g_hash_table_destroy (data.strings); g_markup_parse_context_free (ctx); @@ -421,9 +477,9 @@ _gtk_buildable_parser_precompile (const char *text, g_list_free (string_table); - marshal_tree (marshaled, data.root); + marshal_tree (marshaled, &data.root->base); - record_data_tree_free (data.root); + record_data_node_free (&data.root->base); g_string_chunk_free (data.chunks); g_hash_table_destroy (data.strings); From 8b228e7471d22c4e57885dab8a0bd59fd5599183 Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Tue, 21 Sep 2021 15:26:45 -0700 Subject: [PATCH 08/13] builder: Use a flexible array for attributes in precompile --- gtk/gtkbuilderprecompile.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index 6c910d79be..9467ce5c14 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -53,10 +53,10 @@ struct RecordDataElement { RecordDataNode base; RecordDataElement *parent; + GQueue children; int n_attributes; RecordDataString *name; - RecordDataString **attributes; - GQueue children; + RecordDataString *attributes[]; }; typedef struct { @@ -90,15 +90,18 @@ record_data_node_new (RecordDataElement *parent, static RecordDataElement * record_data_element_new (RecordDataElement *parent, - RecordDataString *name) + RecordDataString *name, + gsize n_attributes) { RecordDataElement *element; element = record_data_node_new (parent, RECORD_TYPE_ELEMENT, - sizeof (RecordDataElement)); + sizeof (RecordDataElement) + + sizeof (RecordDataString) * n_attributes); element->parent = parent; element->name = name; + element->n_attributes = n_attributes; return element; } @@ -135,8 +138,8 @@ record_data_node_free (RecordDataNode *node) l = next; } - g_free (element->attributes); - g_slice_free (RecordDataElement, element); + g_slice_free1 (sizeof (RecordDataElement) + + sizeof (RecordDataString) * element->n_attributes, element); break; case RECORD_TYPE_TEXT: text = (RecordDataText *)node; @@ -237,12 +240,9 @@ record_start_element (GMarkupParseContext *context, int i; name = record_data_string_lookup (data, element_name, -1); - child = record_data_element_new (data->current, name); + child = record_data_element_new (data->current, name, n_attrs); data->current = child; - child->n_attributes = n_attrs; - child->attributes = g_new (RecordDataString *, n_attrs * 2); - attr_names = &child->attributes[0]; attr_values = &child->attributes[n_attrs]; for (i = 0; i < n_attrs; i++) @@ -425,7 +425,7 @@ _gtk_buildable_parser_precompile (const char *text, data.strings = g_hash_table_new_full (record_data_string_hash, record_data_string_equal, (GDestroyNotify)record_data_string_free, NULL); data.chunks = g_string_chunk_new (512); - data.root = record_data_element_new (NULL, NULL); + data.root = record_data_element_new (NULL, NULL, 0); data.current = data.root; ctx = g_markup_parse_context_new (&record_parser, G_MARKUP_TREAT_CDATA_AS_TEXT, &data, NULL); From 6c8b505f9314d578e37afc1a821f50973dff7151 Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Tue, 21 Sep 2021 18:01:54 -0700 Subject: [PATCH 09/13] builder: Avoid g_hash_table_get_values() in precompile Embed the GList link in the RecordDataString. --- gtk/gtkbuilderprecompile.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index 9467ce5c14..ec2741cdf3 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -40,6 +40,7 @@ typedef struct { int offset; int text_offset; gboolean include_len; + GList link; } RecordDataString; typedef struct { @@ -68,6 +69,7 @@ typedef struct { typedef struct { GHashTable *strings; GStringChunk *chunks; + GQueue string_list; RecordDataElement *root; RecordDataElement *current; } RecordData; @@ -184,7 +186,8 @@ record_data_string_hash (gconstpointer _a) static int record_data_string_compare (gconstpointer _a, - gconstpointer _b) + gconstpointer _b, + gpointer user_data) { const RecordDataString *a = _a; const RecordDataString *b = _b; @@ -220,8 +223,12 @@ record_data_string_lookup (RecordData *data, s->len = len; s->count = 1; s->include_len = include_len; + s->link.data = s; + s->link.next = NULL; + s->link.prev = NULL; g_hash_table_add (data->strings, s); + g_queue_push_tail_link (&data->string_list, &s->link); return s; } @@ -418,7 +425,7 @@ _gtk_buildable_parser_precompile (const char *text, { GMarkupParseContext *ctx; RecordData data = { 0 }; - GList *string_table, *l; + GList *l; GString *marshaled; int offset; @@ -442,11 +449,10 @@ _gtk_buildable_parser_precompile (const char *text, g_markup_parse_context_free (ctx); - string_table = g_hash_table_get_values (data.strings); - string_table = g_list_sort (string_table, record_data_string_compare); + g_queue_sort (&data.string_list, record_data_string_compare, NULL); offset = 0; - for (l = string_table; l != NULL; l = l->next) + for (l = data.string_list.head; l != NULL; l = l->next) { RecordDataString *s = l->data; @@ -465,7 +471,7 @@ _gtk_buildable_parser_precompile (const char *text, g_string_append_len (marshaled, "GBU\0", 4); marshal_uint32 (marshaled, offset); - for (l = string_table; l != NULL; l = l->next) + for (l = data.string_list.head; l != NULL; l = l->next) { RecordDataString *s = l->data; @@ -475,8 +481,6 @@ _gtk_buildable_parser_precompile (const char *text, g_string_append_len (marshaled, s->string, s->len + 1); } - g_list_free (string_table); - marshal_tree (marshaled, &data.root->base); record_data_node_free (&data.root->base); From 9c12b58e3275ebdb79ba5faeda1bd278fe3229e1 Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Thu, 23 Sep 2021 15:21:53 -0700 Subject: [PATCH 10/13] builder: Remove root special case from precompile --- gtk/gtkbuilderprecompile.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index ec2741cdf3..b75a0023a7 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -374,27 +374,22 @@ marshal_tree (GString *marshaled, case RECORD_TYPE_ELEMENT: element = (RecordDataElement *)node; - /* Special case the root */ - if (element->parent != NULL) - { - marshal_uint32 (marshaled, RECORD_TYPE_ELEMENT); - marshal_uint32 (marshaled, element->name->offset); - marshal_uint32 (marshaled, element->n_attributes); + marshal_uint32 (marshaled, RECORD_TYPE_ELEMENT); + marshal_uint32 (marshaled, element->name->offset); + marshal_uint32 (marshaled, element->n_attributes); - attr_names = &element->attributes[0]; - attr_values = &element->attributes[element->n_attributes]; - for (i = 0; i < element->n_attributes; i++) - { - marshal_uint32 (marshaled, attr_names[i]->offset); - marshal_uint32 (marshaled, attr_values[i]->offset); - } + attr_names = &element->attributes[0]; + attr_values = &element->attributes[element->n_attributes]; + for (i = 0; i < element->n_attributes; i++) + { + marshal_uint32 (marshaled, attr_names[i]->offset); + marshal_uint32 (marshaled, attr_values[i]->offset); } for (l = element->children.head; l != NULL; l = l->next) marshal_tree (marshaled, l->data); - if (element->parent != NULL) - marshal_uint32 (marshaled, RECORD_TYPE_END_ELEMENT); + marshal_uint32 (marshaled, RECORD_TYPE_END_ELEMENT); break; case RECORD_TYPE_TEXT: text = (RecordDataText *)node; @@ -407,6 +402,17 @@ marshal_tree (GString *marshaled, } } +static void +marshal_root (GString *marshaled, + RecordDataNode *node) +{ + GList *l; + RecordDataElement *element = (RecordDataElement *)node; + + for (l = element->children.head; l != NULL; l = l->next) + marshal_tree (marshaled, l->data); +} + /** * _gtk_buildable_parser_precompile: * @text: chunk of text to parse @@ -481,7 +487,7 @@ _gtk_buildable_parser_precompile (const char *text, g_string_append_len (marshaled, s->string, s->len + 1); } - marshal_tree (marshaled, &data.root->base); + marshal_root (marshaled, &data.root->base); record_data_node_free (&data.root->base); g_string_chunk_free (data.chunks); From fcb6adaaaa66e859c8404807455c8f33c41ccf30 Mon Sep 17 00:00:00 2001 From: Garrett Regier Date: Thu, 23 Sep 2021 15:26:56 -0700 Subject: [PATCH 11/13] builder: Use g_slice_free_chain() for strings in precompile --- gtk/gtkbuilderprecompile.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index b75a0023a7..7c8dac9a5a 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -34,13 +34,15 @@ typedef enum /* All strings are owned by the string chunk */ typedef struct { + /* Must be first for g_slice_free_chain() */ + GList link; + const char *string; int len; int count; int offset; int text_offset; gboolean include_len; - GList link; } RecordDataString; typedef struct { @@ -153,12 +155,6 @@ record_data_node_free (RecordDataNode *node) } } -static void -record_data_string_free (RecordDataString *s) -{ - g_slice_free (RecordDataString, s); -} - static gboolean record_data_string_equal (gconstpointer _a, gconstpointer _b) @@ -435,8 +431,7 @@ _gtk_buildable_parser_precompile (const char *text, GString *marshaled; int offset; - data.strings = g_hash_table_new_full (record_data_string_hash, record_data_string_equal, - (GDestroyNotify)record_data_string_free, NULL); + data.strings = g_hash_table_new (record_data_string_hash, record_data_string_equal); data.chunks = g_string_chunk_new (512); data.root = record_data_element_new (NULL, NULL, 0); data.current = data.root; @@ -489,6 +484,9 @@ _gtk_buildable_parser_precompile (const char *text, marshal_root (marshaled, &data.root->base); + g_slice_free_chain (RecordDataString, + (RecordDataString *)data.string_list.head, + link.next); record_data_node_free (&data.root->base); g_string_chunk_free (data.chunks); g_hash_table_destroy (data.strings); From c7afa5452b2f32096d4dda4693e1a948ac1c8f42 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 14 Dec 2021 00:12:37 -0500 Subject: [PATCH 12/13] builder: Drop irrelevant whitespace in precompile Drop text nodes that won't contribute to the end result. This gets rid of a lot of text nodes in the replay. --- gtk/gtkbuilderprecompile.c | 41 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index 7c8dac9a5a..5128f3ab4b 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -58,6 +58,7 @@ struct RecordDataElement { RecordDataElement *parent; GQueue children; int n_attributes; + gboolean preserve_whitespace; RecordDataString *name; RecordDataString *attributes[]; }; @@ -92,6 +93,25 @@ record_data_node_new (RecordDataElement *parent, return node; } +static gboolean +text_is_important (const char *name) +{ + const char *elements[] = { + "property", + "attribute", + "col", + "action-widget", + "item", + "mime-type", + "pattern", + "suffix", + "mark", + NULL + }; + + return g_strv_contains (elements, name); +} + static RecordDataElement * record_data_element_new (RecordDataElement *parent, RecordDataString *name, @@ -105,6 +125,7 @@ record_data_element_new (RecordDataElement *parent, sizeof (RecordDataString) * n_attributes); element->parent = parent; element->name = name; + element->preserve_whitespace = name && text_is_important (name->string); element->n_attributes = n_attributes; return element; @@ -266,6 +287,23 @@ record_end_element (GMarkupParseContext *context, data->current = data->current->parent; } +static gboolean +is_whitespace (const char *text, + gsize text_len) +{ + const char *end; + const char *p; + + end = text + text_len; + for (p = text; p < end; p = g_utf8_next_char (p)) + { + if (!g_unichar_isspace (g_utf8_get_char (p))) + return FALSE; + } + + return TRUE; +} + static void record_text (GMarkupParseContext *context, const char *text, @@ -276,6 +314,9 @@ record_text (GMarkupParseContext *context, RecordData *data = user_data; RecordDataString *string; + if (!data->current->preserve_whitespace && is_whitespace (text, text_len)) + return; + string = record_data_string_lookup (data, text, text_len); record_data_element_append_text (data->current, string); } From c5bffb9fb55d25714251b5ef7b9f6e67b7bffb65 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 14 Dec 2021 01:06:05 -0500 Subject: [PATCH 13/13] builder: Drop empty text chunks when precompiling These don't add any value either. --- gtk/gtkbuilderprecompile.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gtk/gtkbuilderprecompile.c b/gtk/gtkbuilderprecompile.c index 5128f3ab4b..398fd75eb2 100644 --- a/gtk/gtkbuilderprecompile.c +++ b/gtk/gtkbuilderprecompile.c @@ -314,6 +314,9 @@ record_text (GMarkupParseContext *context, RecordData *data = user_data; RecordDataString *string; + if (text_len == 0) + return; + if (!data->current->preserve_whitespace && is_whitespace (text, text_len)) return;