get_prop_name #4475
Comments
|
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 |
|
|
|
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. 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. 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_namevs 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_nameand 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 |
It can be None for the same reason when 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. |
akward : / |
|
If it's really worth avoiding and same for |
|
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_nameBut 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 Probably breaking layouts in draft mode is not we must afraid of. |
|
i suspect sockets could have a |
|
if maybe there is some speedup to be gained in the node api ? |
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. |
Problem statement
sverchok/core/sockets.py
Lines 349 to 364 in 89b770b
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.
The text was updated successfully, but these errors were encountered: