From 05ddc7daa23ec57a59156db4ebf5f0ba72a668b0 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Mon, 9 Jan 2023 23:29:58 -0500 Subject: [PATCH] Nodes: Avoid small allocations for internal links Since internal links are only runtime data, we have the flexibility to allocating every link individually. Instead we can store links directly in the node runtime vector. This allows avoiding many small allocations when copying and changing node trees. In the future we could use a smaller type like a pair of sockets instead of `bNodeLink` to save memory. Differential Revision: https://developer.blender.org/D16960 --- source/blender/blenkernel/BKE_node.h | 2 +- source/blender/blenkernel/BKE_node_runtime.hh | 4 +-- source/blender/blenkernel/intern/node.cc | 36 +++++++------------ .../blender/blenkernel/intern/node_runtime.cc | 4 +-- .../blenkernel/intern/node_tree_update.cc | 20 +++++------ .../compositor/intern/COM_NodeGraph.cc | 4 +-- .../blender/editors/space_node/node_draw.cc | 6 ++-- source/blender/makesdna/DNA_node_types.h | 2 +- source/blender/makesrna/intern/rna_nodetree.c | 6 ++-- .../blender/nodes/intern/derived_node_tree.cc | 6 ++-- .../intern/geometry_nodes_lazy_function.cc | 10 +++--- source/blender/nodes/intern/node_exec.cc | 14 ++++---- source/blender/nodes/intern/node_socket.cc | 10 +++--- 13 files changed, 56 insertions(+), 68 deletions(-) diff --git a/source/blender/blenkernel/BKE_node.h b/source/blender/blenkernel/BKE_node.h index 47e198c9a12..3cb577fb92e 100644 --- a/source/blender/blenkernel/BKE_node.h +++ b/source/blender/blenkernel/BKE_node.h @@ -690,7 +690,7 @@ void nodeRemoveNode(struct Main *bmain, void nodeDimensionsGet(const struct bNode *node, float *r_width, float *r_height); void nodeTagUpdateID(struct bNode *node); -void nodeInternalLinks(struct bNode *node, struct bNodeLink ***r_links, int *r_len); +void nodeInternalLinks(struct bNode *node, struct bNodeLink **r_links, int *r_len); #ifdef __cplusplus diff --git a/source/blender/blenkernel/BKE_node_runtime.hh b/source/blender/blenkernel/BKE_node_runtime.hh index 6146e8ea7ae..c5322b7c6ba 100644 --- a/source/blender/blenkernel/BKE_node_runtime.hh +++ b/source/blender/blenkernel/BKE_node_runtime.hh @@ -254,7 +254,7 @@ class bNodeRuntime : NonCopyable, NonMovable { float anim_ofsx; /** List of cached internal links (input to output), for muted nodes and operators. */ - Vector internal_links; + Vector internal_links; /** Eagerly maintained cache of the node's index in the tree. */ int index_in_tree = -1; @@ -626,7 +626,7 @@ inline bool bNode::is_group_output() const return this->type == NODE_GROUP_OUTPUT; } -inline blender::Span bNode::internal_links() const +inline blender::Span bNode::internal_links() const { return this->runtime->internal_links; } diff --git a/source/blender/blenkernel/intern/node.cc b/source/blender/blenkernel/intern/node.cc index 0eb5889da65..e42a974eba2 100644 --- a/source/blender/blenkernel/intern/node.cc +++ b/source/blender/blenkernel/intern/node.cc @@ -1959,10 +1959,10 @@ void nodeRemoveSocketEx(bNodeTree *ntree, bNode *node, bNodeSocket *sock, bool d } } - for (bNodeLink *link : node->runtime->internal_links) { - if (link->fromsock == sock || link->tosock == sock) { - node->runtime->internal_links.remove_first_occurrence_and_reorder(link); - MEM_freeN(link); + for (const int64_t i : node->runtime->internal_links.index_range()) { + const bNodeLink &link = node->runtime->internal_links[i]; + if (link.fromsock == sock || link.tosock == sock) { + node->runtime->internal_links.remove_and_reorder(i); BKE_ntree_update_tag_node_internal_link(ntree, node); break; } @@ -1986,9 +1986,6 @@ void nodeRemoveAllSockets(bNodeTree *ntree, bNode *node) } } - for (bNodeLink *link : node->runtime->internal_links) { - MEM_freeN(link); - } node->runtime->internal_links.clear(); LISTBASE_FOREACH_MUTABLE (bNodeSocket *, sock, &node->inputs) { @@ -2312,14 +2309,12 @@ bNode *node_copy_with_mapping(bNodeTree *dst_tree, node_dst->prop = IDP_CopyProperty_ex(node_src.prop, flag); } - node_dst->runtime->internal_links.clear(); - for (const bNodeLink *src_link : node_src.runtime->internal_links) { - bNodeLink *dst_link = (bNodeLink *)MEM_dupallocN(src_link); - dst_link->fromnode = node_dst; - dst_link->tonode = node_dst; - dst_link->fromsock = socket_map.lookup(src_link->fromsock); - dst_link->tosock = socket_map.lookup(src_link->tosock); - node_dst->runtime->internal_links.append(dst_link); + node_dst->runtime->internal_links = node_src.runtime->internal_links; + for (bNodeLink &dst_link : node_dst->runtime->internal_links) { + dst_link.fromnode = node_dst; + dst_link.tonode = node_dst; + dst_link.fromsock = socket_map.lookup(dst_link.fromsock); + dst_link.tosock = socket_map.lookup(dst_link.tosock); } if ((flag & LIB_ID_CREATE_NO_USER_REFCOUNT) == 0) { @@ -2474,8 +2469,8 @@ static void adjust_multi_input_indices_after_removed_link(bNodeTree *ntree, void nodeInternalRelink(bNodeTree *ntree, bNode *node) { /* store link pointers in output sockets, for efficient lookup */ - for (bNodeLink *link : node->runtime->internal_links) { - link->tosock->link = link; + for (bNodeLink &link : node->runtime->internal_links) { + link.tosock->link = &link; } /* redirect downstream links */ @@ -2989,11 +2984,6 @@ void node_free_node(bNodeTree *ntree, bNode *node) MEM_freeN(sock); } - for (bNodeLink *link : node->runtime->internal_links) { - MEM_freeN(link); - } - node->runtime->internal_links.clear(); - if (node->prop) { /* Remember, no ID user refcount management here! */ IDP_FreePropertyContent_ex(node->prop, false); @@ -3644,7 +3634,7 @@ void nodeTagUpdateID(bNode *node) node->runtime->update |= NODE_UPDATE_ID; } -void nodeInternalLinks(bNode *node, bNodeLink ***r_links, int *r_len) +void nodeInternalLinks(bNode *node, bNodeLink **r_links, int *r_len) { *r_links = node->runtime->internal_links.data(); *r_len = node->runtime->internal_links.size(); diff --git a/source/blender/blenkernel/intern/node_runtime.cc b/source/blender/blenkernel/intern/node_runtime.cc index d3b16d4c1a5..dc24e5ddbc4 100644 --- a/source/blender/blenkernel/intern/node_runtime.cc +++ b/source/blender/blenkernel/intern/node_runtime.cc @@ -95,8 +95,8 @@ static void update_internal_link_inputs(const bNodeTree &ntree) for (bNodeSocket *socket : node->runtime->outputs) { socket->runtime->internal_link_input = nullptr; } - for (bNodeLink *link : node->runtime->internal_links) { - link->tosock->runtime->internal_link_input = link->fromsock; + for (bNodeLink &link : node->runtime->internal_links) { + link.tosock->runtime->internal_link_input = link.fromsock; } } } diff --git a/source/blender/blenkernel/intern/node_tree_update.cc b/source/blender/blenkernel/intern/node_tree_update.cc index 93e8f24787a..397556e23fc 100644 --- a/source/blender/blenkernel/intern/node_tree_update.cc +++ b/source/blender/blenkernel/intern/node_tree_update.cc @@ -635,8 +635,8 @@ class NodeTreeMainUpdater { const bNodeSocket *from_socket = item.first; const bNodeSocket *to_socket = item.second; bool found = false; - for (const bNodeLink *internal_link : node->runtime->internal_links) { - if (from_socket == internal_link->fromsock && to_socket == internal_link->tosock) { + for (const bNodeLink &internal_link : node->runtime->internal_links) { + if (from_socket == internal_link.fromsock && to_socket == internal_link.tosock) { found = true; } } @@ -682,19 +682,17 @@ class NodeTreeMainUpdater { bNode &node, Span> links) { - for (bNodeLink *link : node.runtime->internal_links) { - MEM_freeN(link); - } node.runtime->internal_links.clear(); + node.runtime->internal_links.reserve(links.size()); for (const auto &item : links) { bNodeSocket *from_socket = item.first; bNodeSocket *to_socket = item.second; - bNodeLink *link = MEM_cnew(__func__); - link->fromnode = &node; - link->fromsock = from_socket; - link->tonode = &node; - link->tosock = to_socket; - link->flag |= NODE_LINK_VALID; + bNodeLink link{}; + link.fromnode = &node; + link.fromsock = from_socket; + link.tonode = &node; + link.tosock = to_socket; + link.flag |= NODE_LINK_VALID; node.runtime->internal_links.append(link); } BKE_ntree_update_tag_node_internal_link(&ntree, &node); diff --git a/source/blender/compositor/intern/COM_NodeGraph.cc b/source/blender/compositor/intern/COM_NodeGraph.cc index 34a145f6bf5..c4f840cde4e 100644 --- a/source/blender/compositor/intern/COM_NodeGraph.cc +++ b/source/blender/compositor/intern/COM_NodeGraph.cc @@ -181,8 +181,8 @@ void NodeGraph::add_proxies_mute(bNodeTree *b_ntree, bNodeInstanceKey key, bool is_active_group) { - for (const bNodeLink *b_link : b_node->internal_links()) { - SocketProxyNode *proxy = new SocketProxyNode(b_node, b_link->fromsock, b_link->tosock, false); + for (const bNodeLink &b_link : b_node->internal_links()) { + SocketProxyNode *proxy = new SocketProxyNode(b_node, b_link.fromsock, b_link.tosock, false); add_node(proxy, b_ntree, key, is_active_group); } } diff --git a/source/blender/editors/space_node/node_draw.cc b/source/blender/editors/space_node/node_draw.cc index 098c08686b4..ef284c3e3bf 100644 --- a/source/blender/editors/space_node/node_draw.cc +++ b/source/blender/editors/space_node/node_draw.cc @@ -675,9 +675,9 @@ static void node_draw_mute_line(const bContext &C, { GPU_blend(GPU_BLEND_ALPHA); - for (const bNodeLink *link : node.internal_links()) { - if (!nodeLinkIsHidden(link)) { - node_draw_link_bezier(C, v2d, snode, *link, TH_WIRE_INNER, TH_WIRE_INNER, TH_WIRE, false); + for (const bNodeLink &link : node.internal_links()) { + if (!nodeLinkIsHidden(&link)) { + node_draw_link_bezier(C, v2d, snode, link, TH_WIRE_INNER, TH_WIRE_INNER, TH_WIRE, false); } } diff --git a/source/blender/makesdna/DNA_node_types.h b/source/blender/makesdna/DNA_node_types.h index 7053836f7ff..2ddf166566a 100644 --- a/source/blender/makesdna/DNA_node_types.h +++ b/source/blender/makesdna/DNA_node_types.h @@ -375,7 +375,7 @@ typedef struct bNode { bool is_group_output() const; const blender::nodes::NodeDeclaration *declaration() const; /** A span containing all internal links when the node is muted. */ - blender::Span internal_links() const; + blender::Span internal_links() const; /* The following methods are only available when #bNodeTree.ensure_topology_cache has been * called. */ diff --git a/source/blender/makesrna/intern/rna_nodetree.c b/source/blender/makesrna/intern/rna_nodetree.c index 300bca9e369..f895a00e23a 100644 --- a/source/blender/makesrna/intern/rna_nodetree.c +++ b/source/blender/makesrna/intern/rna_nodetree.c @@ -2377,10 +2377,10 @@ static void rna_Node_parent_set(PointerRNA *ptr, static void rna_Node_internal_links_begin(CollectionPropertyIterator *iter, PointerRNA *ptr) { bNode *node = ptr->data; - bNodeLink **begin; + bNodeLink *begin; int len; nodeInternalLinks(node, &begin, &len); - rna_iterator_array_begin(iter, begin, sizeof(bNodeLink *), len, false, NULL); + rna_iterator_array_begin(iter, begin, sizeof(bNodeLink), len, false, NULL); } static bool rna_Node_parent_poll(PointerRNA *ptr, PointerRNA value) @@ -12203,7 +12203,7 @@ static void rna_def_node(BlenderRNA *brna) "rna_Node_internal_links_begin", "rna_iterator_array_next", "rna_iterator_array_end", - "rna_iterator_array_dereference_get", + "rna_iterator_array_get", NULL, NULL, NULL, diff --git a/source/blender/nodes/intern/derived_node_tree.cc b/source/blender/nodes/intern/derived_node_tree.cc index 9b8a9f2401a..ff978efc14a 100644 --- a/source/blender/nodes/intern/derived_node_tree.cc +++ b/source/blender/nodes/intern/derived_node_tree.cc @@ -236,8 +236,8 @@ void DOutputSocket::foreach_target_socket(ForeachTargetSocketFn target_fn, path_info.sockets.pop_last(); } else if (linked_node->is_muted()) { - for (const bNodeLink *internal_link : linked_node->internal_links()) { - if (internal_link->fromsock != linked_socket.bsocket()) { + for (const bNodeLink &internal_link : linked_node->internal_links()) { + if (internal_link.fromsock != linked_socket.bsocket()) { continue; } /* The internal link only forwards the first incoming link. */ @@ -247,7 +247,7 @@ void DOutputSocket::foreach_target_socket(ForeachTargetSocketFn target_fn, } } const DInputSocket mute_input = linked_socket; - const DOutputSocket mute_output{context_, internal_link->tosock}; + const DOutputSocket mute_output{context_, internal_link.tosock}; path_info.sockets.append(mute_input); path_info.sockets.append(mute_output); mute_output.foreach_target_socket(target_fn, path_info); diff --git a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc index bb4337a75eb..4aec8d405ea 100644 --- a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc +++ b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc @@ -400,9 +400,9 @@ class LazyFunctionForMutedNode : public LazyFunction { input_by_output_index_.reinitialize(outputs_.size()); input_by_output_index_.fill(-1); - for (const bNodeLink *internal_link : node.internal_links()) { - const int input_i = r_used_inputs.first_index_of_try(internal_link->fromsock); - const int output_i = r_used_outputs.first_index_of_try(internal_link->tosock); + for (const bNodeLink &internal_link : node.internal_links()) { + const int input_i = r_used_inputs.first_index_of_try(internal_link.fromsock); + const int output_i = r_used_outputs.first_index_of_try(internal_link.tosock); if (ELEM(-1, input_i, output_i)) { continue; } @@ -1945,8 +1945,8 @@ struct GeometryNodesLazyFunctionGraphBuilder { { /* Find all outputs that use a specific input. */ MultiValueMap outputs_by_input; - for (const bNodeLink *blink : bnode.internal_links()) { - outputs_by_input.add(blink->fromsock, blink->tosock); + for (const bNodeLink &blink : bnode.internal_links()) { + outputs_by_input.add(blink.fromsock, blink.tosock); } for (const auto item : outputs_by_input.items()) { const bNodeSocket &input_bsocket = *item.key; diff --git a/source/blender/nodes/intern/node_exec.cc b/source/blender/nodes/intern/node_exec.cc index 3d8eafec75a..da8e2148ab0 100644 --- a/source/blender/nodes/intern/node_exec.cc +++ b/source/blender/nodes/intern/node_exec.cc @@ -71,18 +71,18 @@ static void node_init_input_index(bNodeSocket *sock, int *index) static void node_init_output_index_muted(bNodeSocket *sock, int *index, - const blender::Span internal_links) + const blender::MutableSpan internal_links) { - bNodeLink *link; + const bNodeLink *link; /* copy the stack index from internally connected input to skip the node */ - for (bNodeLink *iter_link : internal_links) { - if (iter_link->tosock == sock) { - sock->stack_index = iter_link->fromsock->stack_index; + for (bNodeLink &iter_link : internal_links) { + if (iter_link.tosock == sock) { + sock->stack_index = iter_link.fromsock->stack_index; /* set the link pointer to indicate that this socket * should not overwrite the stack value! */ - sock->link = iter_link; - link = iter_link; + sock->link = &iter_link; + link = &iter_link; break; } } diff --git a/source/blender/nodes/intern/node_socket.cc b/source/blender/nodes/intern/node_socket.cc index 11fc900ea94..9110813b4d7 100644 --- a/source/blender/nodes/intern/node_socket.cc +++ b/source/blender/nodes/intern/node_socket.cc @@ -220,12 +220,12 @@ static void refresh_socket_list(bNodeTree &ntree, link->tosock = new_socket; } } - for (bNodeLink *internal_link : node.runtime->internal_links) { - if (internal_link->fromsock == old_socket_with_same_identifier) { - internal_link->fromsock = new_socket; + for (bNodeLink &internal_link : node.runtime->internal_links) { + if (internal_link.fromsock == old_socket_with_same_identifier) { + internal_link.fromsock = new_socket; } - else if (internal_link->tosock == old_socket_with_same_identifier) { - internal_link->tosock = new_socket; + else if (internal_link.tosock == old_socket_with_same_identifier) { + internal_link.tosock = new_socket; } } }