Browse Source

Update style sheet index cache lookup for faster retrieval of element style definition, see #293.

- Particularly, performance improvements when using style rules with class names.
- Also fixes hash collisions in tag/id or node lists, previously they could result in the wrong styles being applied.
Michael Ragazzon 3 years ago
parent
commit
aba05b4a8e

+ 4 - 12
Include/RmlUi/Core/StyleSheet.h

@@ -29,10 +29,10 @@
 #ifndef RMLUI_CORE_STYLESHEET_H
 #define RMLUI_CORE_STYLESHEET_H
 
-#include "Traits.h"
 #include "PropertyDictionary.h"
 #include "Spritesheet.h"
 #include "StyleSheetTypes.h"
+#include "Traits.h"
 
 namespace Rml {
 
@@ -40,14 +40,11 @@ class Element;
 class ElementDefinition;
 class StyleSheetNode;
 class Decorator;
-class FontEffect;
 class SpritesheetList;
-class Stream;
 class StyleSheetContainer;
 class StyleSheetParser;
 struct PropertySource;
 struct Sprite;
-struct Spritesheet;
 
 /**
 	StyleSheet maintains a single stylesheet definition. A stylesheet can be combined with another stylesheet to create
@@ -61,9 +58,6 @@ class RMLUICORE_API StyleSheet final : public NonCopyMoveable
 public:
 	~StyleSheet();
 
-	using NodeList = Vector<const StyleSheetNode*>;
-	using NodeIndex = UnorderedMap<size_t, NodeList>;
-
 	/// Combines this style sheet with another one, producing a new sheet.
 	UniquePtr<StyleSheet> CombineStyleSheet(const StyleSheet& sheet) const;
 	/// Merges another style sheet into this.
@@ -83,9 +77,6 @@ public:
 	/// Returns the compiled element definition for a given element and its hierarchy.
 	SharedPtr<const ElementDefinition> GetElementDefinition(const Element* element) const;
 
-	/// Retrieve the hash key used to look-up applicable nodes in the node index.
-	static size_t NodeHash(const String& tag, const String& id);
-
 	/// Returns a list of instanced decorators from the declarations. The instances are cached for faster future retrieval.
 	const Vector<SharedPtr<const Decorator>>& InstanceDecorators(const DecoratorDeclarationList& declaration_list, const PropertySource* decorator_source) const;
 
@@ -112,10 +103,10 @@ private:
 	SpritesheetList spritesheet_list;
 
 	// Map of all styled nodes, that is, they have one or more properties.
-	NodeIndex styled_node_index;
+	StyleSheetIndex styled_node_index;
 
 	// Index of node sets to element definitions.
-	using ElementDefinitionCache = UnorderedMap<size_t, SharedPtr<const ElementDefinition>>;
+	using ElementDefinitionCache = UnorderedMap<StyleSheetIndex::NodeList, SharedPtr<const ElementDefinition>>;
 	mutable ElementDefinitionCache node_cache;
 
 	// Cached decorator instances.
@@ -127,4 +118,5 @@ private:
 };
 
 } // namespace Rml
+
 #endif

+ 34 - 4
Include/RmlUi/Core/StyleSheetTypes.h

@@ -15,7 +15,7 @@
  *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -29,18 +29,20 @@
 #ifndef RMLUI_CORE_STYLESHEETTYPES_H
 #define RMLUI_CORE_STYLESHEETTYPES_H
 
-#include "Types.h"
 #include "PropertyDictionary.h"
+#include "Types.h"
+#include "Utilities.h"
 
 namespace Rml {
 
 class Decorator;
 class DecoratorInstancer;
 class StyleSheet;
+class StyleSheetNode;
 
 struct KeyframeBlock {
 	KeyframeBlock(float normalized_time) : normalized_time(normalized_time) {}
-	float normalized_time;  // [0, 1]
+	float normalized_time; // [0, 1]
 	PropertyDictionary properties;
 };
 struct Keyframes {
@@ -68,12 +70,40 @@ struct DecoratorDeclarationList {
 
 struct MediaBlock {
 	MediaBlock() {}
-	MediaBlock(PropertyDictionary _properties, SharedPtr<StyleSheet> _stylesheet) : properties(std::move(_properties)), stylesheet(std::move(_stylesheet)) {}
+	MediaBlock(PropertyDictionary _properties, SharedPtr<StyleSheet> _stylesheet) :
+		properties(std::move(_properties)), stylesheet(std::move(_stylesheet))
+	{}
 
 	PropertyDictionary properties; // Media query properties
 	SharedPtr<StyleSheet> stylesheet;
 };
 using MediaBlockList = Vector<MediaBlock>;
 
+/**
+   StyleSheetIndex contains a cached index of all styled nodes for quick lookup when finding applicable style nodes for the current state of a given
+   element.
+ */
+struct StyleSheetIndex {
+	using NodeList = Vector<const StyleSheetNode*>;
+	using NodeIndex = UnorderedMap<std::size_t, NodeList>;
+
+	// The following objects are given in prioritized order. Any nodes in the first object will not be contained in the next one and so on.
+	NodeIndex ids, classes, tags;
+	NodeList other;
+};
 } // namespace Rml
+
+namespace std {
+// Hash specialization for the node list, so it can be used as key in UnorderedMap.
+template <>
+struct hash<::Rml::StyleSheetIndex::NodeList> {
+	std::size_t operator()(const ::Rml::StyleSheetIndex::NodeList& nodes) const
+	{
+		std::size_t seed = 0;
+		for (const ::Rml::StyleSheetNode* node : nodes)
+			::Rml::Utilities::HashCombine(seed, node);
+		return seed;
+	}
+};
+} // namespace std
 #endif

+ 4 - 3
Source/Core/Element.cpp

@@ -40,6 +40,7 @@
 #include "../../Include/RmlUi/Core/PropertyIdSet.h"
 #include "../../Include/RmlUi/Core/PropertiesIteratorView.h"
 #include "../../Include/RmlUi/Core/PropertyDefinition.h"
+#include "../../Include/RmlUi/Core/StyleSheet.h"
 #include "../../Include/RmlUi/Core/StyleSheetSpecification.h"
 #include "../../Include/RmlUi/Core/TransformPrimitive.h"
 #include "Clock.h"
@@ -1048,7 +1049,7 @@ Element* Element::Closest(const String& selectors) const
 	{
 		for (const StyleSheetNode* node : leaf_nodes)
 		{
-			if (node->IsApplicable(parent, false))
+			if (node->IsApplicable(parent))
 			{
 				return parent;
 			}
@@ -1512,7 +1513,7 @@ static Element* QuerySelectorMatchRecursive(const StyleSheetNodeListRaw& nodes,
 
 		for (const StyleSheetNode* node : nodes)
 		{
-			if (node->IsApplicable(child, false))
+			if (node->IsApplicable(child))
 				return child;
 		}
 
@@ -1534,7 +1535,7 @@ static void QuerySelectorAllMatchRecursive(ElementList& matching_elements, const
 
 		for (const StyleSheetNode* node : nodes)
 		{
-			if (node->IsApplicable(child, false))
+			if (node->IsApplicable(child))
 			{
 				matching_elements.push_back(child);
 				break;

+ 5 - 0
Source/Core/ElementStyle.cpp

@@ -319,6 +319,11 @@ String ElementStyle::GetClassNames() const
 	return class_names;
 }
 
+const StringList& ElementStyle::GetClassNameList() const
+{
+	return classes;
+}
+
 // Sets a local property override on the element to a pre-parsed value.
 bool ElementStyle::SetProperty(PropertyId id, const Property& property)
 {

+ 2 - 0
Source/Core/ElementStyle.h

@@ -87,6 +87,8 @@ public:
 	/// Return the active class list.
 	/// @return A string containing all the classes on the element, separated by spaces.
 	String GetClassNames() const;
+	/// Return the active class list.
+	const StringList& GetClassNameList() const;
 
 	/// Sets a local property override on the element to a pre-parsed value.
 	/// @param[in] name The name of the new property.

+ 45 - 65
Source/Core/StyleSheet.cpp

@@ -32,19 +32,13 @@
 #include "../../Include/RmlUi/Core/Profiling.h"
 #include "../../Include/RmlUi/Core/PropertyDefinition.h"
 #include "../../Include/RmlUi/Core/StyleSheetSpecification.h"
-#include "../../Include/RmlUi/Core/Utilities.h"
 #include "ElementDefinition.h"
+#include "ElementStyle.h"
 #include "StyleSheetNode.h"
 #include <algorithm>
 
 namespace Rml {
 
-// Sorts style nodes based on specificity.
-inline static bool StyleSheetNodeSort(const StyleSheetNode* lhs, const StyleSheetNode* rhs)
-{
-	return lhs->GetSpecificity() < rhs->GetSpecificity();
-}
-
 StyleSheet::StyleSheet()
 {
 	root = MakeUnique<StyleSheetNode>();
@@ -105,7 +99,7 @@ void StyleSheet::MergeStyleSheet(const StyleSheet& other_sheet)
 void StyleSheet::BuildNodeIndex()
 {
 	RMLUI_ZoneScoped;
-	styled_node_index.clear();
+	styled_node_index = {};
 	root->BuildIndex(styled_node_index);
 	root->SetStructurallyVolatileRecursive(false);
 }
@@ -170,89 +164,75 @@ const Sprite* StyleSheet::GetSprite(const String& name) const
 	return spritesheet_list.GetSprite(name);
 }
 
-size_t StyleSheet::NodeHash(const String& tag, const String& id)
-{
-	size_t seed = 0;
-	if (!tag.empty())
-		seed = Hash<String>()(tag);
-	if(!id.empty())
-		Utilities::HashCombine(seed, id);
-	return seed;
-}
-
 // Returns the compiled element definition for a given element hierarchy.
 SharedPtr<const ElementDefinition> StyleSheet::GetElementDefinition(const Element* element) const
 {
 	RMLUI_ASSERT_NONRECURSIVE;
 
-	// See if there are any styles defined for this element.
 	// Using static to avoid allocations. Make sure we don't call this function recursively.
 	static Vector< const StyleSheetNode* > applicable_nodes;
 	applicable_nodes.clear();
 
-	const String& tag = element->GetTagName();
-	const String& id = element->GetId();
-
-	// The styled_node_index is hashed with the tag and id of the RCSS rule. However, we must also check
-	// the rules which don't have them defined, because they apply regardless of tag and id.
-	Array<size_t, 4> node_hash;
-	int num_hashes = 2;
-
-	node_hash[0] = 0;
-	node_hash[1] = NodeHash(tag, String());
-
-	// If we don't have an id, we can safely skip nodes that define an id. Otherwise, we also check the id nodes.
-	if (!id.empty())
-	{
-		num_hashes = 4;
-		node_hash[2] = NodeHash(String(), id);
-		node_hash[3] = NodeHash(tag, id);
-	}
-
-	// The hashes are keys into a set of applicable nodes (given tag and id).
-	for (int i = 0; i < num_hashes; i++)
-	{
-		auto it_nodes = styled_node_index.find(node_hash[i]);
-		if (it_nodes != styled_node_index.end())
+	auto AddApplicableNodes = [element](const StyleSheetIndex::NodeIndex& node_index, const String& key) {
+		auto it_nodes = node_index.find(Hash<String>()(key));
+		if (it_nodes != node_index.end())
 		{
-			const NodeList& nodes = it_nodes->second;
+			const StyleSheetIndex::NodeList& nodes = it_nodes->second;
 
-			// Now see if we satisfy all of the requirements not yet tested: classes, pseudo classes, structural selectors, 
-			// and the full requirements of parent nodes. What this involves is traversing the style nodes backwards, 
-			// trying to match nodes in the element's hierarchy to nodes in the style hierarchy.
 			for (const StyleSheetNode* node : nodes)
 			{
-				if (node->IsApplicable(element, true))
-				{
+				// We found a node that has at least one requirement matching the element. Now see if we satisfy the remaining requirements of the
+				// node, including all ancestor nodes. What this involves is traversing the style nodes backwards, trying to match nodes in the
+				// element's hierarchy to nodes in the style hierarchy.
+				if (node->IsApplicable(element))
 					applicable_nodes.push_back(node);
-				}
 			}
 		}
-	}
+	};
+
+	// See if there are any styles defined for this element.
+	const String& tag = element->GetTagName();
+	const String& id = element->GetId();
+	const StringList& class_names = element->GetStyle()->GetClassNameList();
+
+	// First, look up the indexed requirements. 
+	if (!id.empty())
+		AddApplicableNodes(styled_node_index.ids, id);
 
-	std::sort(applicable_nodes.begin(), applicable_nodes.end(), StyleSheetNodeSort);
+	for (const String& name : class_names)
+		AddApplicableNodes(styled_node_index.classes, name);
+
+	AddApplicableNodes(styled_node_index.tags, tag);
+
+	// Also check all remaining nodes that don't contain any indexed requirements.
+	for (const StyleSheetNode* node : styled_node_index.other)
+	{
+		if (node->IsApplicable(element))
+			applicable_nodes.push_back(node);
+	}
 
 	// If this element definition won't actually store any information, don't bother with it.
 	if (applicable_nodes.empty())
 		return nullptr;
 
-	// Check if this puppy has already been cached in the node index.
-	size_t seed = 0;
-	for (const StyleSheetNode* node : applicable_nodes)
-		Utilities::HashCombine(seed, node);
+	// Sort the applicable nodes by specificity first, then by pointer value in case we have duplicate specificities.
+	std::sort(applicable_nodes.begin(), applicable_nodes.end(), [](const StyleSheetNode* a, const StyleSheetNode* b) {
+		const int a_specificity = a->GetSpecificity();
+		const int b_specificity = b->GetSpecificity();
+		if (a_specificity == b_specificity)
+			return a < b;
+		return a_specificity < b_specificity;
+	});
 
-	auto cache_iterator = node_cache.find(seed);
-	if (cache_iterator != node_cache.end())
+	// Check if this puppy has already been cached in the node index.
+	SharedPtr<const ElementDefinition>& definition = node_cache[applicable_nodes];
+	if (!definition)
 	{
-		SharedPtr<const ElementDefinition>& definition = (*cache_iterator).second;
-		return definition;
+		// Otherwise, create a new definition and add it to our cache.
+		definition = MakeShared<const ElementDefinition>(applicable_nodes);
 	}
 
-	// Create the new definition and add it to our cache.
-	auto new_definition = MakeShared<const ElementDefinition>(applicable_nodes);
-	node_cache[seed] = new_definition;
-
-	return new_definition;
+	return definition;
 }
 
 } // namespace Rml

+ 42 - 17
Source/Core/StyleSheetNode.cpp

@@ -29,6 +29,7 @@
 #include "StyleSheetNode.h"
 #include "../../Include/RmlUi/Core/Element.h"
 #include "../../Include/RmlUi/Core/Profiling.h"
+#include "../../Include/RmlUi/Core/StyleSheet.h"
 #include "StyleSheetFactory.h"
 #include "StyleSheetNodeSelector.h"
 #include <algorithm>
@@ -121,23 +122,42 @@ UniquePtr<StyleSheetNode> StyleSheetNode::DeepCopy(StyleSheetNode* in_parent) co
 }
 
 // Builds up a style sheet's index recursively.
-void StyleSheetNode::BuildIndex(StyleSheet::NodeIndex& styled_node_index) const
+void StyleSheetNode::BuildIndex(StyleSheetIndex& styled_node_index) const
 {
 	// If this has properties defined, then we insert it into the styled node index.
-	if(properties.GetNumProperties() > 0)
+	if (properties.GetNumProperties() > 0)
 	{
-		// The keys of the node index is a hashed combination of tag and id. These are used for fast lookup of applicable nodes.
-		size_t node_hash = StyleSheet::NodeHash(tag, id);
-		StyleSheet::NodeList& nodes = styled_node_index[node_hash];
-		auto it = std::find(nodes.begin(), nodes.end(), this);
-		if(it == nodes.end())
-			nodes.push_back(this);
+		auto IndexInsertNode = [](StyleSheetIndex::NodeIndex& node_index, const String& key, const StyleSheetNode* node) {
+			StyleSheetIndex::NodeList& nodes = node_index[Hash<String>()(key)];
+			auto it = std::find(nodes.begin(), nodes.end(), node);
+			if (it == nodes.end())
+				nodes.push_back(node);
+		};
+
+		// Add this node to the appropriate index for looking up applicable nodes later. Prioritize the most unique requirement first and the most
+		// general requirement last. This way we are able to rule out as many nodes as possible as quickly as possible.
+		if (!id.empty())
+		{
+			IndexInsertNode(styled_node_index.ids, id, this);
+		}
+		else if (!class_names.empty())
+		{
+			// @performance Right now we just use the first class for simplicity. Later we may want to devise a better strategy to try to add the
+			// class with the most unique name. For example by adding the class from this node's list that has the fewest existing matches.
+			IndexInsertNode(styled_node_index.classes, class_names.front(), this);
+		}
+		else if (!tag.empty())
+		{
+			IndexInsertNode(styled_node_index.tags, tag, this);
+		}
+		else
+		{
+			styled_node_index.other.push_back(this);
+		}
 	}
 
 	for (auto& child : children)
-	{
 		child->BuildIndex(styled_node_index);
-	}
 }
 
 bool StyleSheetNode::SetStructurallyVolatileRecursive(bool ancestor_is_structural_pseudo_class)
@@ -241,24 +261,29 @@ inline bool StyleSheetNode::MatchStructuralSelector(const Element* element) cons
 }
 
 // Returns true if this node is applicable to the given element, given its IDs, classes and heritage.
-bool StyleSheetNode::IsApplicable(const Element* const in_element, bool skip_id_tag) const
+bool StyleSheetNode::IsApplicable(const Element* const in_element) const
 {
 	// Determine whether the element matches the current node and its entire lineage. The entire hierarchy of
 	// the element's document will be considered during the match as necessary.
 
-	if (skip_id_tag)
+	for (const String& name : pseudo_class_names)
 	{
-		// Id and tag have already been checked, only check class and pseudo class.
-		if (!MatchClassPseudoClass(in_element))
+		if (!in_element->IsPseudoClassSet(name))
 			return false;
 	}
-	else
+
+	if (!tag.empty() && tag != in_element->GetTagName())
+		return false;
+
+	for (const String& name : class_names)
 	{
-		// Id and tag have not already been matched, match everything.
-		if (!Match(in_element))
+		if (!in_element->IsClassSet(name))
 			return false;
 	}
 
+	if (!id.empty() && id != in_element->GetId())
+		return false;
+
 	const Element* element = in_element;
 
 	// Walk up through all our parent nodes, each one of them must be matched by some ancestor element.

+ 4 - 3
Source/Core/StyleSheetNode.h

@@ -30,12 +30,13 @@
 #define RMLUI_CORE_STYLESHEETNODE_H
 
 #include "../../Include/RmlUi/Core/PropertyDictionary.h"
-#include "../../Include/RmlUi/Core/StyleSheet.h"
 #include "../../Include/RmlUi/Core/Types.h"
 #include <tuple>
 
 namespace Rml {
 
+struct StyleSheetIndex;
+class StyleSheetNode;
 class StyleSheetNodeSelector;
 
 struct StructuralSelector {
@@ -76,7 +77,7 @@ public:
 	/// Recursively set structural volatility.
 	bool SetStructurallyVolatileRecursive(bool ancestor_is_structurally_volatile);
 	/// Builds up a style sheet's index recursively.
-	void BuildIndex(StyleSheet::NodeIndex& styled_node_index) const;
+	void BuildIndex(StyleSheetIndex& styled_node_index) const;
 
 	/// Imports properties from a single rule definition into the node's properties and sets the
 	/// appropriate specificity on them. Any existing attributes sharing a key with a new attribute
@@ -88,7 +89,7 @@ public:
 	const PropertyDictionary& GetProperties() const;
 
 	/// Returns true if this node is applicable to the given element, given its IDs, classes and heritage.
-	bool IsApplicable(const Element* element, bool skip_id_tag) const;
+	bool IsApplicable(const Element* element) const;
 
 	/// Returns the specificity of this node.
 	int GetSpecificity() const;