-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Slow call get_hash
in NodeEncoder
#134
Comments
It should not be too hard to make the hashing function used there configurable. There is no specific reason I picked the The Can you maybe elaborate a little bit on the circumstance in which you find the slowdown from the NodeEncoder? Maybe this is a pointer to something else that needs improvement. |
I agree on why you picked the If it should only be called by I will look why they are called and where :) I keep you updated! |
So I profiled my code and saw that it's being called in: Lines 60 to 66 in 125e5bd
This function is called every time a value is updated. I don't think this is expected behavior isn't it? |
Oh, yes it is. This piece of logic makes sure that a plug/node stays clean if you set the same input again. Thay way, when you re-run the graph and only parts of the inputs change, you can simply set all of the values, but only the altered part of the graph re-runs. Would a way to override the hash function used here help you out? Alternatively, I think you might be able to get a dirty quick fix by monkey-patching the utilities.get_hash function before importing anything else from fowpipe
I haven't tried it though, it might not work at all. |
Hmm, again, you're right :p I don't use it that frequently (updating an existing graph), so that's why I didn't thought about it I think.. Maybe it's due to my other issue #135, were I frequently re-create Graphs because they are inside Nodes. |
It would be easy to make that hash function a constant in the
and
This way, users like you, that are slowed down by this evaluation (or work with objects where the current hashing function doesn't work), can override the hash function conveniently, by just setting
Do you think that will help you? It feels pythonic to me, since it's easy to explain and explicit. |
Is your feature request related to a problem? Please describe.
In our project some graphs are time critical, when profiling the code the
NodeEncoder
arose as a bottleneck.Describe the solution you'd like
Can we think of something else to serialize nodes without using the
sha256
of thehashlib
?Describe alternatives you've considered
Maybe we can use the
__repr__
, this method describe the object in a single line.Do you guys have alternatives?
The text was updated successfully, but these errors were encountered: