Refactor: Depsgraph: Use StringRef, inline some simple methods
While profiling a scene with many objects, I noticed some unexpected functions taking a significant time of depsgraph creation/evaluation. String length calculation and equality comparison was taking longer than it should, and some simple methods were appearing in profiles that should be inlined instead. There are more places where this sort of change would be helpful, this commit just changes places using `OperationIDKey` and `ComponentIDKey`. Pull Request: https://projects.blender.org/blender/blender/pulls/136795
This commit is contained in:
@@ -9,10 +9,6 @@
|
||||
#include "intern/node/deg_node_component.hh"
|
||||
|
||||
#include <cstdio>
|
||||
#include <cstring> /* required for STREQ later on. */
|
||||
|
||||
#include "BLI_ghash.h"
|
||||
#include "BLI_utildefines.h"
|
||||
|
||||
#include "DNA_object_types.h"
|
||||
|
||||
@@ -31,41 +27,12 @@ namespace blender::deg {
|
||||
/** \name Standard Component Methods
|
||||
* \{ */
|
||||
|
||||
ComponentNode::OperationIDKey::OperationIDKey()
|
||||
: opcode(OperationCode::OPERATION), name(""), name_tag(-1)
|
||||
{
|
||||
}
|
||||
|
||||
ComponentNode::OperationIDKey::OperationIDKey(OperationCode opcode)
|
||||
: opcode(opcode), name(""), name_tag(-1)
|
||||
{
|
||||
}
|
||||
|
||||
ComponentNode::OperationIDKey::OperationIDKey(OperationCode opcode, const char *name, int name_tag)
|
||||
: opcode(opcode), name(name), name_tag(name_tag)
|
||||
{
|
||||
}
|
||||
|
||||
std::string ComponentNode::OperationIDKey::identifier() const
|
||||
{
|
||||
const std::string codebuf = std::to_string(int(opcode));
|
||||
return "OperationIDKey(" + codebuf + ", " + name + ")";
|
||||
}
|
||||
|
||||
bool ComponentNode::OperationIDKey::operator==(const OperationIDKey &other) const
|
||||
{
|
||||
return (opcode == other.opcode) && STREQ(name, other.name) && (name_tag == other.name_tag);
|
||||
}
|
||||
|
||||
uint64_t ComponentNode::OperationIDKey::hash() const
|
||||
{
|
||||
const int opcode_as_int = int(opcode);
|
||||
return BLI_ghashutil_combine_hash(
|
||||
name_tag,
|
||||
BLI_ghashutil_combine_hash(BLI_ghashutil_uinthash(opcode_as_int),
|
||||
BLI_ghashutil_strhash_p(name)));
|
||||
}
|
||||
|
||||
ComponentNode::ComponentNode()
|
||||
: entry_operation(nullptr),
|
||||
exit_operation(nullptr),
|
||||
@@ -106,7 +73,7 @@ OperationNode *ComponentNode::find_operation(OperationIDKey key) const
|
||||
else {
|
||||
for (OperationNode *op_node : operations) {
|
||||
if (op_node->opcode == key.opcode && op_node->name_tag == key.name_tag &&
|
||||
STREQ(op_node->name.c_str(), key.name))
|
||||
op_node->name == key.name)
|
||||
{
|
||||
node = op_node;
|
||||
break;
|
||||
@@ -117,7 +84,7 @@ OperationNode *ComponentNode::find_operation(OperationIDKey key) const
|
||||
}
|
||||
|
||||
OperationNode *ComponentNode::find_operation(OperationCode opcode,
|
||||
const char *name,
|
||||
const StringRef name,
|
||||
int name_tag) const
|
||||
{
|
||||
OperationIDKey key(opcode, name, name_tag);
|
||||
@@ -139,7 +106,7 @@ OperationNode *ComponentNode::get_operation(OperationIDKey key) const
|
||||
}
|
||||
|
||||
OperationNode *ComponentNode::get_operation(OperationCode opcode,
|
||||
const char *name,
|
||||
const StringRef name,
|
||||
int name_tag) const
|
||||
{
|
||||
OperationIDKey key(opcode, name, name_tag);
|
||||
@@ -151,7 +118,7 @@ bool ComponentNode::has_operation(OperationIDKey key) const
|
||||
return find_operation(key) != nullptr;
|
||||
}
|
||||
|
||||
bool ComponentNode::has_operation(OperationCode opcode, const char *name, int name_tag) const
|
||||
bool ComponentNode::has_operation(OperationCode opcode, const StringRef name, int name_tag) const
|
||||
{
|
||||
OperationIDKey key(opcode, name, name_tag);
|
||||
return has_operation(key);
|
||||
@@ -159,7 +126,7 @@ bool ComponentNode::has_operation(OperationCode opcode, const char *name, int na
|
||||
|
||||
OperationNode *ComponentNode::add_operation(const DepsEvalOperationCb &op,
|
||||
OperationCode opcode,
|
||||
const char *name,
|
||||
const StringRef name,
|
||||
int name_tag)
|
||||
{
|
||||
OperationNode *op_node = find_operation(opcode, name, name_tag);
|
||||
@@ -168,7 +135,7 @@ OperationNode *ComponentNode::add_operation(const DepsEvalOperationCb &op,
|
||||
op_node = (OperationNode *)factory->create_node(this->owner->id_orig, "", name);
|
||||
|
||||
/* register opnode in this component's operation set */
|
||||
OperationIDKey key(opcode, op_node->name.c_str(), name_tag);
|
||||
OperationIDKey key(opcode, op_node->name, name_tag);
|
||||
operations_map->add(key, op_node);
|
||||
|
||||
/* Set back-link. */
|
||||
|
||||
@@ -8,6 +8,9 @@
|
||||
|
||||
#pragma once
|
||||
|
||||
#include "BLI_span.hh"
|
||||
#include "BLI_string_ref.hh"
|
||||
#include "BLI_struct_equality_utils.hh"
|
||||
#include "intern/eval/deg_eval_copy_on_write.h"
|
||||
#include "intern/node/deg_node.hh"
|
||||
#include "intern/node/deg_node_id.hh"
|
||||
@@ -29,17 +32,23 @@ struct OperationNode;
|
||||
struct ComponentNode : public Node {
|
||||
/* Key used to look up operations within a component */
|
||||
struct OperationIDKey {
|
||||
OperationCode opcode;
|
||||
const char *name;
|
||||
int name_tag;
|
||||
OperationCode opcode = OperationCode::OPERATION;
|
||||
int name_tag = -1;
|
||||
StringRef name = "";
|
||||
|
||||
OperationIDKey();
|
||||
OperationIDKey(OperationCode opcode);
|
||||
OperationIDKey(OperationCode opcode, const char *name, int name_tag);
|
||||
OperationIDKey() = default;
|
||||
OperationIDKey(const OperationCode opcode) : opcode(opcode) {}
|
||||
OperationIDKey(const OperationCode opcode, const StringRef name, const int name_tag)
|
||||
: opcode(opcode), name_tag(name_tag), name(name)
|
||||
{
|
||||
}
|
||||
|
||||
std::string identifier() const;
|
||||
bool operator==(const OperationIDKey &other) const;
|
||||
uint64_t hash() const;
|
||||
BLI_STRUCT_EQUALITY_OPERATORS_3(OperationIDKey, opcode, name_tag, name);
|
||||
uint64_t hash() const
|
||||
{
|
||||
return get_default_hash(opcode, name_tag, name);
|
||||
}
|
||||
};
|
||||
|
||||
/* Typedef for container of operations */
|
||||
@@ -56,19 +65,17 @@ struct ComponentNode : public Node {
|
||||
*/
|
||||
OperationNode *find_operation(OperationIDKey key) const;
|
||||
OperationNode *find_operation(OperationCode opcode,
|
||||
const char *name = "",
|
||||
StringRef name = "",
|
||||
int name_tag = -1) const;
|
||||
|
||||
/* Find an existing operation, will throw an assert() if it does not exist.
|
||||
* See #add_operation for the meaning and examples of #name and #name_tag. */
|
||||
OperationNode *get_operation(OperationIDKey key) const;
|
||||
OperationNode *get_operation(OperationCode opcode,
|
||||
const char *name = "",
|
||||
int name_tag = -1) const;
|
||||
OperationNode *get_operation(OperationCode opcode, StringRef name = "", int name_tag = -1) const;
|
||||
|
||||
/* Check operation exists and return it. */
|
||||
bool has_operation(OperationIDKey key) const;
|
||||
bool has_operation(OperationCode opcode, const char *name = "", int name_tag = -1) const;
|
||||
bool has_operation(OperationCode opcode, StringRef name = "", int name_tag = -1) const;
|
||||
|
||||
/**
|
||||
* Create a new node for representing an operation and add this to graph
|
||||
@@ -88,7 +95,7 @@ struct ComponentNode : public Node {
|
||||
*/
|
||||
OperationNode *add_operation(const DepsEvalOperationCb &op,
|
||||
OperationCode opcode,
|
||||
const char *name = "",
|
||||
const StringRef name = "",
|
||||
int name_tag = -1);
|
||||
|
||||
/* Entry/exit operations management.
|
||||
|
||||
@@ -8,6 +8,8 @@
|
||||
|
||||
#pragma once
|
||||
|
||||
#include "BLI_string_ref.hh"
|
||||
|
||||
#include "intern/node/deg_node.hh"
|
||||
|
||||
struct ID;
|
||||
@@ -19,7 +21,7 @@ struct DepsNodeFactory {
|
||||
|
||||
virtual int id_recalc_tag() const = 0;
|
||||
|
||||
virtual Node *create_node(const ID *id, const char *subdata, const char *name) const = 0;
|
||||
virtual Node *create_node(const ID *id, const char *subdata, StringRef name) const = 0;
|
||||
};
|
||||
|
||||
template<class ModeObjectType> struct DepsNodeFactoryImpl : public DepsNodeFactory {
|
||||
@@ -28,7 +30,7 @@ template<class ModeObjectType> struct DepsNodeFactoryImpl : public DepsNodeFacto
|
||||
|
||||
int id_recalc_tag() const override;
|
||||
|
||||
Node *create_node(const ID *id, const char *subdata, const char *name) const override;
|
||||
Node *create_node(const ID *id, const char *subdata, StringRef name) const override;
|
||||
};
|
||||
|
||||
/* Register typeinfo */
|
||||
|
||||
@@ -32,7 +32,7 @@ template<class ModeObjectType> int DepsNodeFactoryImpl<ModeObjectType>::id_recal
|
||||
template<class ModeObjectType>
|
||||
Node *DepsNodeFactoryImpl<ModeObjectType>::create_node(const ID *id,
|
||||
const char *subdata,
|
||||
const char *name) const
|
||||
const StringRef name) const
|
||||
{
|
||||
Node *node = new ModeObjectType();
|
||||
node->type = type();
|
||||
|
||||
@@ -8,9 +8,6 @@
|
||||
|
||||
#include "intern/node/deg_node_id.hh"
|
||||
|
||||
#include <cstring> /* required for STREQ later on. */
|
||||
|
||||
#include "BLI_ghash.h"
|
||||
#include "BLI_string.h"
|
||||
#include "BLI_utildefines.h"
|
||||
|
||||
@@ -38,20 +35,6 @@ const char *linkedStateAsString(eDepsNode_LinkedState_Type linked_state)
|
||||
return "UNKNOWN";
|
||||
}
|
||||
|
||||
IDNode::ComponentIDKey::ComponentIDKey(NodeType type, const char *name) : type(type), name(name) {}
|
||||
|
||||
bool IDNode::ComponentIDKey::operator==(const ComponentIDKey &other) const
|
||||
{
|
||||
return type == other.type && STREQ(name, other.name);
|
||||
}
|
||||
|
||||
uint64_t IDNode::ComponentIDKey::hash() const
|
||||
{
|
||||
const int type_as_int = int(type);
|
||||
return BLI_ghashutil_combine_hash(BLI_ghashutil_uinthash(type_as_int),
|
||||
BLI_ghashutil_strhash_p(name));
|
||||
}
|
||||
|
||||
void IDNode::init(const ID *id, const char * /*subdata*/)
|
||||
{
|
||||
BLI_assert(id != nullptr);
|
||||
@@ -139,13 +122,13 @@ std::string IDNode::identifier() const
|
||||
(is_visible_on_build ? "true" : "false") + ")";
|
||||
}
|
||||
|
||||
ComponentNode *IDNode::find_component(NodeType type, const char *name) const
|
||||
ComponentNode *IDNode::find_component(NodeType type, const StringRef name) const
|
||||
{
|
||||
ComponentIDKey key(type, name);
|
||||
return components.lookup_default(key, nullptr);
|
||||
}
|
||||
|
||||
ComponentNode *IDNode::add_component(NodeType type, const char *name)
|
||||
ComponentNode *IDNode::add_component(NodeType type, const StringRef name)
|
||||
{
|
||||
ComponentNode *comp_node = find_component(type, name);
|
||||
if (!comp_node) {
|
||||
@@ -154,7 +137,7 @@ ComponentNode *IDNode::add_component(NodeType type, const char *name)
|
||||
comp_node = (ComponentNode *)factory->create_node(this->id_orig, "", name);
|
||||
|
||||
/* Register. */
|
||||
ComponentIDKey key(type, comp_node->name.c_str());
|
||||
ComponentIDKey key(type, comp_node->name);
|
||||
components.add_new(key, comp_node);
|
||||
comp_node->owner = this;
|
||||
}
|
||||
|
||||
@@ -8,11 +8,13 @@
|
||||
|
||||
#pragma once
|
||||
|
||||
#include "BLI_struct_equality_utils.hh"
|
||||
#include "intern/node/deg_node.hh"
|
||||
|
||||
#include "DNA_ID.h"
|
||||
|
||||
#include "BLI_map.hh"
|
||||
#include "BLI_string_ref.hh"
|
||||
#include "BLI_sys_types.h"
|
||||
|
||||
namespace blender::deg {
|
||||
@@ -36,12 +38,16 @@ const char *linkedStateAsString(eDepsNode_LinkedState_Type linked_state);
|
||||
/* ID-Block Reference */
|
||||
struct IDNode : public Node {
|
||||
struct ComponentIDKey {
|
||||
ComponentIDKey(NodeType type, const char *name = "");
|
||||
uint64_t hash() const;
|
||||
bool operator==(const ComponentIDKey &other) const;
|
||||
|
||||
NodeType type;
|
||||
const char *name;
|
||||
StringRef name;
|
||||
|
||||
ComponentIDKey(NodeType type, StringRef name = "") : type(type), name(name) {}
|
||||
BLI_STRUCT_EQUALITY_OPERATORS_2(ComponentIDKey, type, name);
|
||||
uint64_t hash() const
|
||||
{
|
||||
return get_default_hash(type, name);
|
||||
}
|
||||
};
|
||||
|
||||
/** Initialize 'id' node - from pointer data given. */
|
||||
@@ -52,8 +58,8 @@ struct IDNode : public Node {
|
||||
|
||||
std::string identifier() const override;
|
||||
|
||||
ComponentNode *find_component(NodeType type, const char *name = "") const;
|
||||
ComponentNode *add_component(NodeType type, const char *name = "");
|
||||
ComponentNode *find_component(NodeType type, StringRef name = "") const;
|
||||
ComponentNode *add_component(NodeType type, StringRef name = "");
|
||||
|
||||
void tag_update(Depsgraph *graph, eUpdateSource source) override;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user