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:
Hans Goudey
2025-04-01 17:30:39 +02:00
committed by Hans Goudey
parent 3727c78bcf
commit 83325d1fd3
6 changed files with 47 additions and 82 deletions

View File

@@ -9,10 +9,6 @@
#include "intern/node/deg_node_component.hh" #include "intern/node/deg_node_component.hh"
#include <cstdio> #include <cstdio>
#include <cstring> /* required for STREQ later on. */
#include "BLI_ghash.h"
#include "BLI_utildefines.h"
#include "DNA_object_types.h" #include "DNA_object_types.h"
@@ -31,41 +27,12 @@ namespace blender::deg {
/** \name Standard Component Methods /** \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 std::string ComponentNode::OperationIDKey::identifier() const
{ {
const std::string codebuf = std::to_string(int(opcode)); const std::string codebuf = std::to_string(int(opcode));
return "OperationIDKey(" + codebuf + ", " + name + ")"; 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() ComponentNode::ComponentNode()
: entry_operation(nullptr), : entry_operation(nullptr),
exit_operation(nullptr), exit_operation(nullptr),
@@ -106,7 +73,7 @@ OperationNode *ComponentNode::find_operation(OperationIDKey key) const
else { else {
for (OperationNode *op_node : operations) { for (OperationNode *op_node : operations) {
if (op_node->opcode == key.opcode && op_node->name_tag == key.name_tag && 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; node = op_node;
break; break;
@@ -117,7 +84,7 @@ OperationNode *ComponentNode::find_operation(OperationIDKey key) const
} }
OperationNode *ComponentNode::find_operation(OperationCode opcode, OperationNode *ComponentNode::find_operation(OperationCode opcode,
const char *name, const StringRef name,
int name_tag) const int name_tag) const
{ {
OperationIDKey key(opcode, name, name_tag); OperationIDKey key(opcode, name, name_tag);
@@ -139,7 +106,7 @@ OperationNode *ComponentNode::get_operation(OperationIDKey key) const
} }
OperationNode *ComponentNode::get_operation(OperationCode opcode, OperationNode *ComponentNode::get_operation(OperationCode opcode,
const char *name, const StringRef name,
int name_tag) const int name_tag) const
{ {
OperationIDKey key(opcode, name, name_tag); OperationIDKey key(opcode, name, name_tag);
@@ -151,7 +118,7 @@ bool ComponentNode::has_operation(OperationIDKey key) const
return find_operation(key) != nullptr; 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); OperationIDKey key(opcode, name, name_tag);
return has_operation(key); 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, OperationNode *ComponentNode::add_operation(const DepsEvalOperationCb &op,
OperationCode opcode, OperationCode opcode,
const char *name, const StringRef name,
int name_tag) int name_tag)
{ {
OperationNode *op_node = find_operation(opcode, name, 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); op_node = (OperationNode *)factory->create_node(this->owner->id_orig, "", name);
/* register opnode in this component's operation set */ /* 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); operations_map->add(key, op_node);
/* Set back-link. */ /* Set back-link. */

View File

@@ -8,6 +8,9 @@
#pragma once #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/eval/deg_eval_copy_on_write.h"
#include "intern/node/deg_node.hh" #include "intern/node/deg_node.hh"
#include "intern/node/deg_node_id.hh" #include "intern/node/deg_node_id.hh"
@@ -29,17 +32,23 @@ struct OperationNode;
struct ComponentNode : public Node { struct ComponentNode : public Node {
/* Key used to look up operations within a component */ /* Key used to look up operations within a component */
struct OperationIDKey { struct OperationIDKey {
OperationCode opcode; OperationCode opcode = OperationCode::OPERATION;
const char *name; int name_tag = -1;
int name_tag; StringRef name = "";
OperationIDKey(); OperationIDKey() = default;
OperationIDKey(OperationCode opcode); OperationIDKey(const OperationCode opcode) : opcode(opcode) {}
OperationIDKey(OperationCode opcode, const char *name, int name_tag); OperationIDKey(const OperationCode opcode, const StringRef name, const int name_tag)
: opcode(opcode), name_tag(name_tag), name(name)
{
}
std::string identifier() const; std::string identifier() const;
bool operator==(const OperationIDKey &other) const; BLI_STRUCT_EQUALITY_OPERATORS_3(OperationIDKey, opcode, name_tag, name);
uint64_t hash() const; uint64_t hash() const
{
return get_default_hash(opcode, name_tag, name);
}
}; };
/* Typedef for container of operations */ /* Typedef for container of operations */
@@ -56,19 +65,17 @@ struct ComponentNode : public Node {
*/ */
OperationNode *find_operation(OperationIDKey key) const; OperationNode *find_operation(OperationIDKey key) const;
OperationNode *find_operation(OperationCode opcode, OperationNode *find_operation(OperationCode opcode,
const char *name = "", StringRef name = "",
int name_tag = -1) const; int name_tag = -1) const;
/* Find an existing operation, will throw an assert() if it does not exist. /* 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. */ * See #add_operation for the meaning and examples of #name and #name_tag. */
OperationNode *get_operation(OperationIDKey key) const; OperationNode *get_operation(OperationIDKey key) const;
OperationNode *get_operation(OperationCode opcode, OperationNode *get_operation(OperationCode opcode, StringRef name = "", int name_tag = -1) const;
const char *name = "",
int name_tag = -1) const;
/* Check operation exists and return it. */ /* Check operation exists and return it. */
bool has_operation(OperationIDKey key) const; 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 * 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, OperationNode *add_operation(const DepsEvalOperationCb &op,
OperationCode opcode, OperationCode opcode,
const char *name = "", const StringRef name = "",
int name_tag = -1); int name_tag = -1);
/* Entry/exit operations management. /* Entry/exit operations management.

View File

@@ -8,6 +8,8 @@
#pragma once #pragma once
#include "BLI_string_ref.hh"
#include "intern/node/deg_node.hh" #include "intern/node/deg_node.hh"
struct ID; struct ID;
@@ -19,7 +21,7 @@ struct DepsNodeFactory {
virtual int id_recalc_tag() const = 0; 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 { template<class ModeObjectType> struct DepsNodeFactoryImpl : public DepsNodeFactory {
@@ -28,7 +30,7 @@ template<class ModeObjectType> struct DepsNodeFactoryImpl : public DepsNodeFacto
int id_recalc_tag() const override; 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 */ /* Register typeinfo */

View File

@@ -32,7 +32,7 @@ template<class ModeObjectType> int DepsNodeFactoryImpl<ModeObjectType>::id_recal
template<class ModeObjectType> template<class ModeObjectType>
Node *DepsNodeFactoryImpl<ModeObjectType>::create_node(const ID *id, Node *DepsNodeFactoryImpl<ModeObjectType>::create_node(const ID *id,
const char *subdata, const char *subdata,
const char *name) const const StringRef name) const
{ {
Node *node = new ModeObjectType(); Node *node = new ModeObjectType();
node->type = type(); node->type = type();

View File

@@ -8,9 +8,6 @@
#include "intern/node/deg_node_id.hh" #include "intern/node/deg_node_id.hh"
#include <cstring> /* required for STREQ later on. */
#include "BLI_ghash.h"
#include "BLI_string.h" #include "BLI_string.h"
#include "BLI_utildefines.h" #include "BLI_utildefines.h"
@@ -38,20 +35,6 @@ const char *linkedStateAsString(eDepsNode_LinkedState_Type linked_state)
return "UNKNOWN"; 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*/) void IDNode::init(const ID *id, const char * /*subdata*/)
{ {
BLI_assert(id != nullptr); BLI_assert(id != nullptr);
@@ -139,13 +122,13 @@ std::string IDNode::identifier() const
(is_visible_on_build ? "true" : "false") + ")"; (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); ComponentIDKey key(type, name);
return components.lookup_default(key, nullptr); 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); ComponentNode *comp_node = find_component(type, name);
if (!comp_node) { 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); comp_node = (ComponentNode *)factory->create_node(this->id_orig, "", name);
/* Register. */ /* Register. */
ComponentIDKey key(type, comp_node->name.c_str()); ComponentIDKey key(type, comp_node->name);
components.add_new(key, comp_node); components.add_new(key, comp_node);
comp_node->owner = this; comp_node->owner = this;
} }

View File

@@ -8,11 +8,13 @@
#pragma once #pragma once
#include "BLI_struct_equality_utils.hh"
#include "intern/node/deg_node.hh" #include "intern/node/deg_node.hh"
#include "DNA_ID.h" #include "DNA_ID.h"
#include "BLI_map.hh" #include "BLI_map.hh"
#include "BLI_string_ref.hh"
#include "BLI_sys_types.h" #include "BLI_sys_types.h"
namespace blender::deg { namespace blender::deg {
@@ -36,12 +38,16 @@ const char *linkedStateAsString(eDepsNode_LinkedState_Type linked_state);
/* ID-Block Reference */ /* ID-Block Reference */
struct IDNode : public Node { struct IDNode : public Node {
struct ComponentIDKey { struct ComponentIDKey {
ComponentIDKey(NodeType type, const char *name = "");
uint64_t hash() const;
bool operator==(const ComponentIDKey &other) const;
NodeType type; 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. */ /** Initialize 'id' node - from pointer data given. */
@@ -52,8 +58,8 @@ struct IDNode : public Node {
std::string identifier() const override; std::string identifier() const override;
ComponentNode *find_component(NodeType type, const char *name = "") const; ComponentNode *find_component(NodeType type, StringRef name = "") const;
ComponentNode *add_component(NodeType type, const char *name = ""); ComponentNode *add_component(NodeType type, StringRef name = "");
void tag_update(Depsgraph *graph, eUpdateSource source) override; void tag_update(Depsgraph *graph, eUpdateSource source) override;