From 65050a1c8112997d38283ca8f09e89801dacb498 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Tue, 28 Jan 2020 17:49:37 +0100 Subject: [PATCH 1/3] css: Don't do the "all set" test We have so many properties that it is basically impossible that all of them are set and the time spent checking is higher than the time saved if it does indeed happen. --- gtk/gtkcsslookup.c | 7 ------- gtk/gtkcsslookupprivate.h | 2 -- gtk/gtkcssprovider.c | 3 --- 3 files changed, 12 deletions(-) diff --git a/gtk/gtkcsslookup.c b/gtk/gtkcsslookup.c index cfe5bfc5c5..8b0c33b0df 100644 --- a/gtk/gtkcsslookup.c +++ b/gtk/gtkcsslookup.c @@ -44,12 +44,6 @@ _gtk_css_lookup_is_missing (const GtkCssLookup *lookup, return lookup->values[id].value == NULL; } -gboolean -_gtk_css_lookup_all_set (const GtkCssLookup *lookup) -{ - return lookup->n_set_values == GTK_CSS_PROPERTY_N_PROPERTIES; -} - /** * _gtk_css_lookup_set: * @lookup: the lookup @@ -75,7 +69,6 @@ _gtk_css_lookup_set (GtkCssLookup *lookup, lookup->values[id].value = value; lookup->values[id].section = section; - lookup->n_set_values ++; } /** diff --git a/gtk/gtkcsslookupprivate.h b/gtk/gtkcsslookupprivate.h index 34acd504c3..ef29c4e9b3 100644 --- a/gtk/gtkcsslookupprivate.h +++ b/gtk/gtkcsslookupprivate.h @@ -36,7 +36,6 @@ typedef struct { } GtkCssLookupValue; struct _GtkCssLookup { - guint n_set_values; GtkCssLookupValue values[GTK_CSS_PROPERTY_N_PROPERTIES]; }; @@ -44,7 +43,6 @@ void _gtk_css_lookup_init (GtkCssLookup void _gtk_css_lookup_destroy (GtkCssLookup *lookup); gboolean _gtk_css_lookup_is_missing (const GtkCssLookup *lookup, guint id); -gboolean _gtk_css_lookup_all_set (const GtkCssLookup *lookup); void _gtk_css_lookup_set (GtkCssLookup *lookup, guint id, GtkCssSection *section, diff --git a/gtk/gtkcssprovider.c b/gtk/gtkcssprovider.c index 6db8d88ba6..e2388de1e1 100644 --- a/gtk/gtkcssprovider.c +++ b/gtk/gtkcssprovider.c @@ -489,9 +489,6 @@ gtk_css_style_provider_lookup (GtkStyleProvider *provider, ruleset->styles[j].section, ruleset->styles[j].value); } - - if (_gtk_css_lookup_all_set (lookup)) - break; } g_ptr_array_free (tree_rules, TRUE); From c6158f1684418d9796aecf871a16c345ba3caaa0 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Tue, 28 Jan 2020 18:28:32 +0100 Subject: [PATCH 2/3] cssselector: Reorder functions This just changes the order of functions in the source code in preparation for the next commit. --- gtk/gtkcssselector.c | 136 +++++++++++++++++++++---------------------- 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/gtk/gtkcssselector.c b/gtk/gtkcssselector.c index b08e3c7a2b..6bf6748541 100644 --- a/gtk/gtkcssselector.c +++ b/gtk/gtkcssselector.c @@ -1815,6 +1815,74 @@ gtk_css_selectors_skip_initial_selector (GtkCssSelector *selector, const GtkCssS return (GtkCssSelector *)gtk_css_selector_previous (selector); } +/* When checking for changes via the tree we need to know if a rule further + down the tree matched, because if so we need to add "our bit" to the + Change. For instance in a match like *.class:active we'll + get a tree that first checks :active, if that matches we continue down + to the tree, and if we get a match we add CHANGE_CLASS. However, the + end of the tree where we have a match is an ANY which doesn't actually + modify the change, so we don't know if we have a match or not. We fix + this by setting GTK_CSS_CHANGE_GOT_MATCH which lets us guarantee + that change != 0 on any match. */ +#define GTK_CSS_CHANGE_GOT_MATCH GTK_CSS_CHANGE_RESERVED_BIT + +/* The code for collecting matches assumes that the name, id and classes + * of a node remain unchanged, and anything else can change. This needs to + * be kept in sync with the definition of 'radical change' in gtkcssnode.c. + */ + +static GtkCssChange +gtk_css_selector_tree_get_change (const GtkCssSelectorTree *tree, + const GtkCountingBloomFilter *filter, + GtkCssNode *node, + gboolean skipping) +{ + GtkCssChange change = 0; + const GtkCssSelectorTree *prev; + + switch (tree->selector.class->category) + { + case GTK_CSS_SELECTOR_CATEGORY_SIMPLE: + break; + case GTK_CSS_SELECTOR_CATEGORY_SIMPLE_RADICAL: + if (skipping) + break; + if (node) + { + if (!tree->selector.class->match_one (&tree->selector, node)) + return 0; + } + else if (filter) + { + if (!gtk_counting_bloom_filter_may_contain (filter, + gtk_css_selector_hash_one (&tree->selector))) + return 0; + } + break; + case GTK_CSS_SELECTOR_CATEGORY_PARENT: + skipping = FALSE; + node = NULL; + break; + case GTK_CSS_SELECTOR_CATEGORY_SIBLING: + skipping = TRUE; + node = NULL; + break; + default: + g_assert_not_reached (); + return 0; + } + + for (prev = gtk_css_selector_tree_get_previous (tree); + prev != NULL; + prev = gtk_css_selector_tree_get_sibling (prev)) + change |= gtk_css_selector_tree_get_change (prev, filter, node, skipping); + + if (change || gtk_css_selector_tree_get_matches (tree)) + change = tree->selector.class->get_change (&tree->selector, change & ~GTK_CSS_CHANGE_GOT_MATCH) | GTK_CSS_CHANGE_GOT_MATCH; + + return change; +} + static gboolean gtk_css_selector_tree_match_bloom (const GtkCssSelectorTree *tree, const GtkCountingBloomFilter *filter, @@ -1929,74 +1997,6 @@ _gtk_css_selector_tree_match_all (const GtkCssSelectorTree *tree, return results; } -/* When checking for changes via the tree we need to know if a rule further - down the tree matched, because if so we need to add "our bit" to the - Change. For instance in a match like *.class:active we'll - get a tree that first checks :active, if that matches we continue down - to the tree, and if we get a match we add CHANGE_CLASS. However, the - end of the tree where we have a match is an ANY which doesn't actually - modify the change, so we don't know if we have a match or not. We fix - this by setting GTK_CSS_CHANGE_GOT_MATCH which lets us guarantee - that change != 0 on any match. */ -#define GTK_CSS_CHANGE_GOT_MATCH GTK_CSS_CHANGE_RESERVED_BIT - -/* The code for collecting matches assumes that the name, id and classes - * of a node remain unchanged, and anything else can change. This needs to - * be kept in sync with the definition of 'radical change' in gtkcssnode.c. - */ - -static GtkCssChange -gtk_css_selector_tree_get_change (const GtkCssSelectorTree *tree, - const GtkCountingBloomFilter *filter, - GtkCssNode *node, - gboolean skipping) -{ - GtkCssChange change = 0; - const GtkCssSelectorTree *prev; - - switch (tree->selector.class->category) - { - case GTK_CSS_SELECTOR_CATEGORY_SIMPLE: - break; - case GTK_CSS_SELECTOR_CATEGORY_SIMPLE_RADICAL: - if (skipping) - break; - if (node) - { - if (!tree->selector.class->match_one (&tree->selector, node)) - return 0; - } - else if (filter) - { - if (!gtk_counting_bloom_filter_may_contain (filter, - gtk_css_selector_hash_one (&tree->selector))) - return 0; - } - break; - case GTK_CSS_SELECTOR_CATEGORY_PARENT: - skipping = FALSE; - node = NULL; - break; - case GTK_CSS_SELECTOR_CATEGORY_SIBLING: - skipping = TRUE; - node = NULL; - break; - default: - g_assert_not_reached (); - return 0; - } - - for (prev = gtk_css_selector_tree_get_previous (tree); - prev != NULL; - prev = gtk_css_selector_tree_get_sibling (prev)) - change |= gtk_css_selector_tree_get_change (prev, filter, node, skipping); - - if (change || gtk_css_selector_tree_get_matches (tree)) - change = tree->selector.class->get_change (&tree->selector, change & ~GTK_CSS_CHANGE_GOT_MATCH) | GTK_CSS_CHANGE_GOT_MATCH; - - return change; -} - gboolean _gtk_css_selector_tree_is_empty (const GtkCssSelectorTree *tree) { From 5e3cbff8d23a26ae5b09d7e81507048ec4e69764 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Wed, 29 Jan 2020 04:20:47 +0100 Subject: [PATCH 3/3] cssselector: Rework how we handle the bloom filter Instead of foreaching through all the previous selectors every time we bloom-filter, just bloom-filter the current element and return a special value if that filter fails (FALSE). If that happens, don't try filter-matching more nodes in the caller as we know it's an abort. --- gtk/gtkcssselector.c | 74 ++++++++++---------------------------------- 1 file changed, 17 insertions(+), 57 deletions(-) diff --git a/gtk/gtkcssselector.c b/gtk/gtkcssselector.c index 6bf6748541..802ae9a395 100644 --- a/gtk/gtkcssselector.c +++ b/gtk/gtkcssselector.c @@ -1883,49 +1883,6 @@ gtk_css_selector_tree_get_change (const GtkCssSelectorTree *tree, return change; } -static gboolean -gtk_css_selector_tree_match_bloom (const GtkCssSelectorTree *tree, - const GtkCountingBloomFilter *filter, - gboolean skipping) -{ - const GtkCssSelectorTree *prev; - - switch (tree->selector.class->category) - { - case GTK_CSS_SELECTOR_CATEGORY_SIMPLE: - break; - case GTK_CSS_SELECTOR_CATEGORY_SIMPLE_RADICAL: - if (skipping) - break; - if (!gtk_counting_bloom_filter_may_contain (filter, - gtk_css_selector_hash_one (&tree->selector))) - return FALSE; - break; - case GTK_CSS_SELECTOR_CATEGORY_PARENT: - skipping = FALSE; - break; - case GTK_CSS_SELECTOR_CATEGORY_SIBLING: - skipping = TRUE; - break; - default: - g_assert_not_reached (); - return FALSE; - } - - if (gtk_css_selector_tree_get_matches (tree)) - return TRUE; - - for (prev = gtk_css_selector_tree_get_previous (tree); - prev != NULL; - prev = gtk_css_selector_tree_get_sibling (prev)) - { - if (gtk_css_selector_tree_match_bloom (prev, filter, skipping)) - return TRUE; - } - - return FALSE; -} - static void gtk_css_selector_tree_found_match (const GtkCssSelectorTree *tree, GPtrArray **results) @@ -1944,27 +1901,27 @@ gtk_css_selector_tree_found_match (const GtkCssSelectorTree *tree, g_ptr_array_insert_sorted (*results, matches[i]); } -static void +static gboolean gtk_css_selector_tree_match (const GtkCssSelectorTree *tree, const GtkCountingBloomFilter *filter, + gboolean match_filter, GtkCssNode *node, GPtrArray **results) { const GtkCssSelectorTree *prev; GtkCssNode *child; + if (match_filter && tree->selector.class->category == GTK_CSS_SELECTOR_CATEGORY_SIMPLE_RADICAL && + !gtk_counting_bloom_filter_may_contain (filter, gtk_css_selector_hash_one (&tree->selector))) + return FALSE; + if (!gtk_css_selector_match_one (&tree->selector, node)) - return; + return TRUE; gtk_css_selector_tree_found_match (tree, results); if (filter && !gtk_css_selector_is_simple (&tree->selector)) - { - /* We can pass both TRUE or FALSE for skipping here, because the - * function will immediately update it. */ - if (!gtk_css_selector_tree_match_bloom (tree, filter, FALSE)) - return; - } + match_filter = tree->selector.class->category == GTK_CSS_SELECTOR_CATEGORY_PARENT; for (prev = gtk_css_selector_tree_get_previous (tree); prev != NULL; @@ -1974,11 +1931,12 @@ gtk_css_selector_tree_match (const GtkCssSelectorTree *tree, child; child = gtk_css_selector_iterator (&tree->selector, node, child)) { - gtk_css_selector_tree_match (prev, filter, child, results); + if (!gtk_css_selector_tree_match (prev, filter, match_filter, child, results)) + break; } } - return; + return TRUE; } GPtrArray * @@ -1986,12 +1944,14 @@ _gtk_css_selector_tree_match_all (const GtkCssSelectorTree *tree, const GtkCountingBloomFilter *filter, GtkCssNode *node) { + const GtkCssSelectorTree *iter; GPtrArray *results = NULL; - for (; tree != NULL; - tree = gtk_css_selector_tree_get_sibling (tree)) + for (iter = tree; + iter != NULL; + iter = gtk_css_selector_tree_get_sibling (iter)) { - gtk_css_selector_tree_match (tree, filter, node, &results); + gtk_css_selector_tree_match (iter, filter, FALSE, node, &results); } return results; @@ -2020,7 +1980,7 @@ gtk_css_selector_tree_get_change_all (const GtkCssSelectorTree *tree, #ifdef PRINT_TREE static void -_gtk_css_selector_tree_print (const GtkCssSelectorTree *tree, GString *str, char *prefix) +_gtk_css_selector_tree_print (const GtkCssSelectorTree *tree, GString *str, const char *prefix) { gboolean first = TRUE; int len, i;