Browse Source

Speed up element definition retrieval by hashing tag and id.

Michael Ragazzon 6 years ago
parent
commit
3993ddd71d

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

@@ -74,7 +74,7 @@ class RMLUICORE_API StyleSheet : public NonCopyMoveable
 {
 public:
 	typedef std::vector< StyleSheetNode* > NodeList;
-	typedef UnorderedMap< String, NodeList > NodeIndex;
+	typedef UnorderedMap< size_t, NodeList > NodeIndex;
 
 	StyleSheet();
 	virtual ~StyleSheet();
@@ -107,6 +107,9 @@ public:
 	/// caller, so another should not be added. The definition should be released by removing the reference count.
 	SharedPtr<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);
+
 private:
 	// Root level node, attributes from special nodes like "body" get added to this node
 	UniquePtr<StyleSheetNode> root;

+ 35 - 9
Source/Core/StyleSheet.cpp

@@ -307,6 +307,15 @@ FontEffectListPtr StyleSheet::InstanceFontEffectsFromString(const String& font_e
 	return std::make_shared<FontEffectList>(std::move(font_effect_list));
 }
 
+size_t StyleSheet::NodeHash(const String& tag, const String& id)
+{
+	size_t seed = 0;
+	if (!tag.empty())
+		seed = std::hash<String>()(tag);
+	if(!id.empty())
+		Utilities::HashCombine(seed, id);
+	return seed;
+}
 
 // Returns the compiled element definition for a given element hierarchy.
 SharedPtr<ElementDefinition> StyleSheet::GetElementDefinition(const Element* element) const
@@ -318,16 +327,36 @@ SharedPtr<ElementDefinition> StyleSheet::GetElementDefinition(const Element* ele
 	static std::vector< const StyleSheetNode* > applicable_nodes;
 	applicable_nodes.clear();
 
-	String tags[] = {element->GetTagName(), ""};
-	for (int i = 0; i < 2; i++)
+	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.
+	std::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())
 	{
-		auto it_nodes = styled_node_index.find(tags[i]);
+		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())
 		{
 			const NodeList& nodes = it_nodes->second;
 
-			// There are! Now see if we satisfy all of their parenting requirements. What this involves is traversing the style
-			// nodes backwards, trying to match nodes in the element's hierarchy to nodes in the style hierarchy.
+			// 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 (StyleSheetNode* node : nodes)
 			{
 				if (node->IsApplicable(element))
@@ -344,8 +373,7 @@ SharedPtr<ElementDefinition> StyleSheet::GetElementDefinition(const Element* ele
 	if (applicable_nodes.empty())
 		return nullptr;
 
-	// Check if this puppy has already been cached in the node index; it may be that it has already been created by an
-	// element with a different address but an identical output definition.
+	// 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);
@@ -359,8 +387,6 @@ SharedPtr<ElementDefinition> StyleSheet::GetElementDefinition(const Element* ele
 
 	// Create the new definition and add it to our cache.
 	auto new_definition = std::make_shared<ElementDefinition>(applicable_nodes);
-
-	// Add to the node cache.
 	node_cache[seed] = new_definition;
 
 	return new_definition;

+ 3 - 3
Source/Core/StyleSheetFactory.cpp

@@ -164,12 +164,12 @@ void StyleSheetFactory::ClearStyleSheetCache()
 }
 
 // Returns one of the available node selectors.
-NodeSelector StyleSheetFactory::GetSelector(const String& name)
+StructuralSelector StyleSheetFactory::GetSelector(const String& name)
 {
 	size_t index = name.find("(");
 	auto it = instance->selectors.find(name.substr(0, index));
 	if (it == instance->selectors.end())
-		return NodeSelector(nullptr, 0, 0);
+		return StructuralSelector(nullptr, 0, 0);
 
 	// Parse the 'a' and 'b' values.
 	int a = 1;
@@ -224,7 +224,7 @@ NodeSelector StyleSheetFactory::GetSelector(const String& name)
 		}
 	}
 
-	return NodeSelector(it->second, a, b);
+	return StructuralSelector(it->second, a, b);
 }
 
 SharedPtr<StyleSheet> StyleSheetFactory::LoadStyleSheet(const String& sheet)

+ 2 - 2
Source/Core/StyleSheetFactory.h

@@ -36,7 +36,7 @@ namespace Core {
 
 class StyleSheet;
 class StyleSheetNodeSelector;
-struct NodeSelector;
+struct StructuralSelector;
 
 /**
 	Creates stylesheets on the fly as needed. The factory keeps a cache of built sheets for optimisation.
@@ -67,7 +67,7 @@ public:
 	/// Returns one of the available node selectors.
 	/// @param name[in] The name of the desired selector.
 	/// @return The selector registered with the given name, or nullptr if none exists.
-	static NodeSelector GetSelector(const String& name);
+	static StructuralSelector GetSelector(const String& name);
 
 private:
 	StyleSheetFactory();

+ 47 - 41
Source/Core/StyleSheetNode.cpp

@@ -42,16 +42,15 @@ StyleSheetNode::StyleSheetNode() : parent(nullptr)
 	is_structurally_volatile = true;
 }
 
-
-StyleSheetNode::StyleSheetNode(StyleSheetNode* parent, const String& tag, const String& id, const StringList& classes, const StringList& pseudo_classes, const NodeSelectorList& structural_pseudo_classes)
-	: parent(parent), tag(tag), id(id), class_names(classes), pseudo_class_names(pseudo_classes)
+StyleSheetNode::StyleSheetNode(StyleSheetNode* parent, const String& tag, const String& id, const StringList& classes, const StringList& pseudo_classes, const StructuralSelectorList& structural_selectors)
+	: parent(parent), tag(tag), id(id), class_names(classes), pseudo_class_names(pseudo_classes), structural_selectors(structural_selectors)
 {
 	specificity = CalculateSpecificity();
 	is_structurally_volatile = true;
 }
 
-StyleSheetNode::StyleSheetNode(StyleSheetNode* parent, String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes, NodeSelectorList&& structural_pseudo_classes) 
-	: parent(parent), tag(std::move(tag)), id(std::move(id)), class_names(std::move(classes)), pseudo_class_names(std::move(pseudo_classes))
+StyleSheetNode::StyleSheetNode(StyleSheetNode* parent, String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes, StructuralSelectorList&& structural_selectors)
+	: parent(parent), tag(std::move(tag)), id(std::move(id)), class_names(std::move(classes)), pseudo_class_names(std::move(pseudo_classes)), structural_selectors(std::move(structural_selectors))
 {
 	specificity = CalculateSpecificity();
 	is_structurally_volatile = true;
@@ -75,10 +74,8 @@ StyleSheetNode* StyleSheetNode::GetOrCreateChildNode(const StyleSheetNode& other
 	return result;
 }
 
-StyleSheetNode* StyleSheetNode::GetOrCreateChildNode(String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes, NodeSelectorList&& structural_pseudo_classes)
+StyleSheetNode* StyleSheetNode::GetOrCreateChildNode(String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes, StructuralSelectorList&& structural_pseudo_classes)
 {
-	// @performance: Maybe sort children by tag,id or something else?
-
 	// See if we match an existing child
 	for (const auto& child : children)
 	{
@@ -116,7 +113,9 @@ void StyleSheetNode::BuildIndexAndOptimizeProperties(StyleSheet::NodeIndex& styl
 	// If this has properties defined, then we insert it into the styled node index.
 	if(properties.GetNumProperties() > 0)
 	{
-		StyleSheet::NodeList& nodes = styled_node_index[tag];
+		// 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);
@@ -186,7 +185,7 @@ bool StyleSheetNode::SetStructurallyVolatileRecursive(bool ancestor_is_structura
 	return (self_is_structural_pseudo_class || descendant_is_structural_pseudo_class);
 }
 
-bool StyleSheetNode::IsEquivalent(const String& _tag, const String& _id, const StringList& _class_names, const StringList& _pseudo_class_names, const NodeSelectorList& _structural_selectors) const
+bool StyleSheetNode::IsEquivalent(const String& _tag, const String& _id, const StringList& _class_names, const StringList& _pseudo_class_names, const StructuralSelectorList& _structural_selectors) const
 {
 	if (tag != _tag)
 		return false;
@@ -202,13 +201,6 @@ bool StyleSheetNode::IsEquivalent(const String& _tag, const String& _id, const S
 	return true;
 }
 
-
-// Returns the name of this node.
-const String& StyleSheetNode::GetTag() const
-{
-	return tag;
-}
-
 // Returns the specificity of this node.
 int StyleSheetNode::GetSpecificity() const
 {
@@ -228,27 +220,43 @@ const PropertyDictionary& StyleSheetNode::GetProperties() const
 	return properties;
 }
 
-bool StyleSheetNode::Match(const Element* element, const StyleSheetNode* node)
+inline bool StyleSheetNode::Match(const Element* element) const
 {
-	if (!node->tag.empty() && node->tag != element->GetTagName())
+	if (!tag.empty() && tag != element->GetTagName())
 		return false;
 
-	if (!node->id.empty() && node->id != element->GetId())
+	if (!id.empty() && id != element->GetId())
 		return false;
 
-	for (auto& name : node->class_names)
+	if (!MatchClassPseudoClass(element))
+		return false;
+
+	if (!MatchStructuralSelector(element))
+		return false;
+
+	return true;
+}
+
+inline bool StyleSheetNode::MatchClassPseudoClass(const Element* element) const
+{
+	for (auto& name : class_names)
 	{
 		if (!element->IsClassSet(name))
 			return false;
 	}
 
-	for (auto& name : node->pseudo_class_names)
+	for (auto& name : pseudo_class_names)
 	{
 		if (!element->IsPseudoClassSet(name))
 			return false;
 	}
 
-	for (auto& node_selector : node->structural_selectors)
+	return true;
+}
+
+inline bool StyleSheetNode::MatchStructuralSelector(const Element* element) const
+{
+	for (auto& node_selector : structural_selectors)
 	{
 		if (!node_selector.selector->IsApplicable(element, node_selector.a, node_selector.b))
 			return false;
@@ -258,37 +266,36 @@ bool StyleSheetNode::Match(const Element* element, const StyleSheetNode* node)
 }
 
 // Returns true if this node is applicable to the given element, given its IDs, classes and heritage.
-bool StyleSheetNode::IsApplicable(const Element* element) const
+bool StyleSheetNode::IsApplicable(const Element* const in_element) const
 {
-	// This function is called with an element that matches a style node only with the tag name. We have to determine
+	// This function is called with an element that matches a style node only with the tag name and id. We have to determine
 	// here whether or not it also matches the required hierarchy.
-
-	// We must have a parent; if not, something's amok with the style tree.
-	if (parent == nullptr)
-	{
-		RMLUI_ERRORMSG("Invalid RCSS hierarchy.");
-		return false;
-	}
-
-	const StyleSheetNode* node = this;
 	
-	// Check for matching local requirements
-	if (!Match(element, node))
+	// Check for matching local class, pseudo class, and structural requirements. Id and tag have already been checked in StyleSheet.
+	if (!MatchClassPseudoClass(in_element))
 		return false;
 
-	// Then match each parent node
-	for(node = node->parent; node && node->parent; node = node->parent)
+	const Element* element = in_element;
+
+	// Then we must match every parent node.
+	for(const StyleSheetNode* node = parent; node && node->parent; node = node->parent)
 	{
+		// Try a match on every element ancestor.
 		for(element = element->GetParentNode(); element; element = element->GetParentNode())
 		{
-			if (Match(element, node))
+			if (node->Match(element))
 				break;
 		}
 
+		// We have run out of element ancestors before we matched every node. Bail out.
 		if (!element)
 			return false;
 	}
 
+	// Check for matching local class, pseudo class, and structural requirements. Id and tag have already been checked in StyleSheet.
+	if (!MatchStructuralSelector(in_element))
+		return false;
+
 	return true;
 }
 
@@ -316,9 +323,8 @@ int StyleSheetNode::CalculateSpecificity()
 	specificity += 100000*(int)structural_selectors.size();
 
 	// Add our parent's specificity onto ours.
-	// @performance: Replace with parent->specificity
 	if (parent)
-		specificity += parent->CalculateSpecificity();
+		specificity += parent->specificity;
 
 	return specificity;
 }

+ 24 - 23
Source/Core/StyleSheetNode.h

@@ -32,24 +32,26 @@
 #include "../../Include/RmlUi/Core/PropertyDictionary.h"
 #include "../../Include/RmlUi/Core/StyleSheet.h"
 #include "../../Include/RmlUi/Core/Types.h"
+#include <tuple>
 
 namespace Rml {
 namespace Core {
 
 class StyleSheetNodeSelector;
-struct NodeSelector {
-	NodeSelector(StyleSheetNodeSelector* selector, int a, int b) : selector(selector), a(a), b(b) {}
+
+struct StructuralSelector {
+	StructuralSelector(StyleSheetNodeSelector* selector, int a, int b) : selector(selector), a(a), b(b) {}
 	StyleSheetNodeSelector* selector;
 	int a;
 	int b;
 };
+inline bool operator==(const StructuralSelector& a, const StructuralSelector& b) { return a.selector == b.selector && a.a == b.a && a.b == b.b; }
+inline bool operator<(const StructuralSelector& a, const StructuralSelector& b) { return std::tie(a.selector, a.a, a.b) < std::tie(b.selector, b.a, b.b); }
 
-inline bool operator==(const NodeSelector& a, const NodeSelector& b) { return a.selector == b.selector && a.a == b.a && a.b == b.b; }
-inline bool operator<(const NodeSelector& a, const NodeSelector& b) { return std::tie(a.selector, a.a, a.b) < std::tie(b.selector, b.a, b.b); }
-
-using NodeSelectorList = std::vector< NodeSelector >;
+using StructuralSelectorList = std::vector< StructuralSelector >;
 using StyleSheetNodeList = std::vector< UniquePtr<StyleSheetNode> >;
 
+
 /**
 	A style sheet is composed of a tree of nodes.
 
@@ -60,24 +62,21 @@ class StyleSheetNode
 {
 public:
 	StyleSheetNode();
-	StyleSheetNode(StyleSheetNode* parent, const String& tag, const String& id, const StringList& classes, const StringList& pseudo_classes, const NodeSelectorList& structural_pseudo_classes);
-	StyleSheetNode(StyleSheetNode* parent, String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes, NodeSelectorList&& structural_pseudo_classes);
+	StyleSheetNode(StyleSheetNode* parent, const String& tag, const String& id, const StringList& classes, const StringList& pseudo_classes, const StructuralSelectorList& structural_selectors);
+	StyleSheetNode(StyleSheetNode* parent, String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes, StructuralSelectorList&& structural_selectors);
 
+	/// Retrieves a child node with the given properties if they match an existing node, or else creates a new one.
+	StyleSheetNode* GetOrCreateChildNode(String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes, StructuralSelectorList&& structural_selectors);
+	/// Retrieves or creates a child node with properties equivalent to the 'other' node.
 	StyleSheetNode* GetOrCreateChildNode(const StyleSheetNode& other);
-	StyleSheetNode* GetOrCreateChildNode(String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes, NodeSelectorList&& structural_pseudo_classes);
 
 	/// Merges an entire tree hierarchy into our hierarchy.
 	bool MergeHierarchy(StyleSheetNode* node, int specificity_offset = 0);
-	/// Recursively set structural volatility
+	/// Recursively set structural volatility.
 	bool SetStructurallyVolatileRecursive(bool ancestor_is_structurally_volatile);
 	/// Builds up a style sheet's index recursively and optimizes some properties for faster retrieval.
 	void BuildIndexAndOptimizeProperties(StyleSheet::NodeIndex& styled_node_index, const StyleSheet& style_sheet);
 
-	/// Returns the name of this node.
-	const String& GetTag() const;
-	/// Returns the specificity of this node.
-	int GetSpecificity() 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
 	/// will be overwritten if they are of a lower specificity.
@@ -87,35 +86,37 @@ public:
 	/// Returns the node's default properties.
 	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) const;
 
+	/// Returns the specificity of this node.
+	int GetSpecificity() const;
 	/// Returns true if this node employs a structural selector, and therefore generates element definitions that are
 	/// sensitive to sibling changes. 
 	/// @warning Result is only valid if structural volatility is set since any changes to the node tree.
-	/// @return True if this node uses a structural selector.
 	bool IsStructurallyVolatile() const;
 
-
 private:
-	static bool Match(const Element* element, const StyleSheetNode* node);
-
-	bool IsEquivalent(const String& tag, const String& id, const StringList& classes, const StringList& pseudo_classes, const NodeSelectorList& structural_pseudo_classes) const;
+	// Returns true if the properties of this node equals the given arguments.
+	bool IsEquivalent(const String& tag, const String& id, const StringList& classes, const StringList& pseudo_classes, const StructuralSelectorList& structural_pseudo_classes) const;
 
 	int CalculateSpecificity();
 
+	// Match an element to the local node requirements.
+	inline bool Match(const Element* element) const;
+	inline bool MatchClassPseudoClass(const Element* element) const;
+	inline bool MatchStructuralSelector(const Element* element) const;
+
 	// The parent of this node; is nullptr for the root node.
 	StyleSheetNode* parent;
 	//bool child_selector; // The '>' CSS selector: Parent node must be applicable to the element parent.
 
-	// The name.
 	String tag;
 	String id;
 
 	StringList class_names;
 	StringList pseudo_class_names;
-	NodeSelectorList structural_selectors;
+	StructuralSelectorList structural_selectors; // Represents structural pseudo classes
 
 	// True if any ancestor, descendent, or self is a structural pseudo class.
 	bool is_structurally_volatile;

+ 2 - 2
Source/Core/StyleSheetParser.cpp

@@ -605,7 +605,7 @@ bool StyleSheetParser::ImportProperties(StyleSheetNode* node, const String& rule
 		String id;
 		StringList classes;
 		StringList pseudo_classes;
-		NodeSelectorList structural_pseudo_classes;
+		StructuralSelectorList structural_pseudo_classes;
 
 		size_t index = 0;
 		while (index < name.size())
@@ -630,7 +630,7 @@ bool StyleSheetParser::ImportProperties(StyleSheetNode* node, const String& rule
 					case ':':
 					{
 						String pseudo_class_name = identifier.substr(1);
-						NodeSelector node_selector = StyleSheetFactory::GetSelector(pseudo_class_name);
+						StructuralSelector node_selector = StyleSheetFactory::GetSelector(pseudo_class_name);
 						if (node_selector.selector)
 							structural_pseudo_classes.push_back(node_selector);
 						else