sverchok
sverchok copied to clipboard
nortikin
get_prop_name
Problem statement
https://github.com/nortikin/sverchok/blob/89b770b3b27805d85ba2f83f569a0e0897692ccc/core/sockets.py#L349-L364
In a test tree execution of this method takes 1% of whole time. It seems that it's probably too expensive for just getting name of a default value. I'm not sure which solution can be proposed here.
can it be cached? the value isn't likely to change often on any given socket.
1% improvement might also be a quest with diminishing returns
cant this be written a bit simpler?
if node and hasattr(node, 'does_support_draft_mode') and node.does_support_draft_mode() and hasattr(node.id_data, 'sv_draft') and node.id_data.sv_draft:
vs
if node and getattr(node, 'does_support_draft_mode', False) and getattr(node.id_data, 'sv_draft', False):
?
I don't think this can be catched because it should return different values dependently on sv_draft mode of a tree. It seems solution can be to replace self.prop_name on change of the sv_draft mode with name of a sv draft property.
in certain "high volume" situations we could use a cache, as socket instances will never be shared by multiple nodes.
this could be worse :)
_highspeed_NodeSocket_Origins = {}
def get_prop_name(self):
"""
Intended to return name of property related with socket owned by its node
Name can be replaced by twin property name in draft mode of a tree
If does not have 'missing_dependency' attribute it can return empty list, reasons unknown
"""
node = _highspeed_NodeSocket_Origins.get(self.socket_id)
if not node:
node = self.node
_highspeed_NodeSocket_Origins[self.socket_id] = node
if hasattr(node, 'missing_dependency'):
return []
if node and getattr(node, 'does_support_draft_mode', False) and getattr(node.id_data, 'sv_draft', False):
prop_name_draft = node.draft_properties_mapping.get(self.prop_name, None)
if prop_name_draft:
return prop_name_draft
return self.prop_name
Hm, it might give some speed up but looks quite complicated. Probably such thing could be implemented as separate property
Class Socket:
cache = {} # should be reseted on undo events etc.
@property
def node_(self):
if self.socket_id not in Socket.cache:
Socket.cache[self.socket_id] = self.node
return Socket.cache[self.socket_id]
yes! suitable.
also is there a genuine need to check node here?
if node and getattr(node, 'does_support_draft_mode', False) and getattr(node.id_data, 'sv_draft', False):
prop_name_draft = node.draft_properties_mapping.get(self.prop_name, None)
if prop_name_draft:
return prop_name_draft
return self.prop_name
vs
node = self.node_
if getattr(node, 'does_support_draft_mode', False) and getattr(node.id_data, 'sv_draft', False):
prop_name_draft = node.draft_properties_mapping.get(self.prop_name, None)
if prop_name_draft:
return prop_name_draft
return self.prop_name
and more and more i get the sense that there is a less verbose way to write this
prop_name_draft = node.draft_properties_mapping.get(self.prop_name, None)
if prop_name_draft:
return prop_name_draft
maybe i'm missing some logic here but..
def get_prop_name(self):
"""
Intended to return name of property related with socket owned by its node
Name can be replaced by twin property name in draft mode of a tree
Currently the presence of the 'missing_dependency' attribute indicates a degenerate node.
"""
node = self.node_
if not node or hasattr(node, 'missing_dependency'):
return self.prop_name # '' should be as good as []
if getattr(node, 'does_support_draft_mode', False) and getattr(node.id_data, 'sv_draft', False):
return node.draft_properties_mapping.get(self.prop_name, self.prop_name)
return self.prop_name
unless we are testing the result of isinstance(self.get_prop_name(), list) somewhere..
also is there a genuine need to check node here?
It can be None for the same reason when socket.is_linked is False when it should beTrue.
Another solution would be to path node as parameter.
def get_prop_name(self, node): ...
# in a node process method
self.inputs[0].sv_get(self)
It's ugly but super efficient and simple. Potentially it requires a lot of little fixes in code of nodes.
Returning empty list by the method is quite ugly. The name of the method suggests that it should return a string.
If it's really worth avoiding self.node maybe gradually convert nodes? and make node an optional kw of sv_get ?
def sv_get(self, default=sentinel, deepcopy=True, implicit_conversions=None, node=None)
...
and same for def get_prop_name(self, node=None): ?
I think it worths because it has O(len(tree.nodes)) to call, so in bigger trees (more then 100 nodes) the overhead can be even more significant.
Probably the node parameter should be placed before other ones so it will be easier to convert it into mandatory parameter later.
def sv_get(self, node=None, *, default=sential, deepcopy=True, implicit_conversion=None):
...
The asterisk should halpe to remove None default value of the node parameter but it assumes that now all parameters are passed via keywords.
ok. i think the vast majority of nodes can be updated with a relatively simple search/replace, the remaining 20/30 nodes we can do by hand.
I think it's possible simplify the draft mode and instead of calling the properties_mapping method to use a naming convention:
if socket.id_data.sv_draft:
return f"{socket.prop_name}_draft"
else:
return socket.prop_name
But this seems can break layouts if some nodes do not use this convention now and their draft properties should be renamed to fit. And this does not take into account that not all nodes have draft properties 🤔
Probably this can be done in the sv_get method instead
if socket.id_data.sv_draft:
try:
return getattr(node, f"{socket.prop_name}_draft")
except AtrributeError:
return getattr(node, socket.prop_name)
else:
return getattr(node, socket.prop_name)
Looks a bit overcomplicated but might be more efficient.
I have looked at the code and it seems all nodes follow the convention except the Number node which instead of _draft suffix has draft_ one. 🔫
Probably breaking layouts in draft mode is not we must afraid of.
i suspect sockets could have a socket_dict ? inside which we could store a link when made and remove the link when disconnected? if so it would also be able to store the node associated with the socket instance, and might be faster than self.node and socket.is_linked ?
if self.node is expensive, why is that? it is Blender's api.. https://docs.blender.org/api/current/bpy.types.NodeSocket.html#bpy.types.NodeSocket.node
maybe there is some speedup to be gained in the node api ?
if self.node is expensive, why is that?
According to information from Jacques Lucke
I don't think having dictionary will be simplest solution because we can't keep straight references to nodes without risk of a crash. So there is need in protection from undo events, removing nodes etc.