Optimize the linear edge list generation using caching #2321
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DolphinDream
force-pushed
the
edgeGenerationSpeedUp
branch
from
15108ce
to
3ac7eff
Compare
|
Below is the profiling for Line MK4 node (in development) with and without the edge generation optimization. The first statistics are for the line node using the optimized edge generation (using the edge cache), the other is for the node using the typical list comprehension. The percentages of the edge generation time (get_edge_list) from the total time spent in making the line (make_line) are significantly different: |
zeffii
reviewed
Nov 28, 2018
data_structure.py
Outdated
|
|
||
|
|
||
| def update_edge_cache(n): | ||
| ''' |
There was a problem hiding this comment.
these look like backticks?
There was a problem hiding this comment.
I checked. they are not. I can replace them with normal quotes if that’s more desirable.
There was a problem hiding this comment.
no matter, docstring syntax is unfortunately mixed in data_structure.py. so asking you to use double quotes would be almost arbitrary.
It seems neither of these two are any faster than even the list comprehension.. here are some profiling results: The percentage of time to generate edge / time to make the line: which is close to the list comprehension results. Honestly, I suspect it’s gonna be hard to beat the edge cache implementation. I’m not bragging, but just thinking that any conversion to other types would necessarily incur some overhead which is going to be more than just slicing a cached list (yet, I can’t claim I know enough python to say this for sure. Just my own hunch). Sometimes sacrificing some memory to gain speed may be the only way. Besides, memory is not even a big concern since even if we cache a 1 million edge list (which would likely be way and beyond what any node tree would ever need to create), such a list hardly imposes any memory burden (about 8MB). Though, a growing edge cache per node tree necessity I think it would be better than pre caching some arbitrarily large edge list. |
|
by all means cache the living snot out of the things that can be cached :) |
|
If there are no objections this is ready to land. |
|
do it |
DolphinDream
force-pushed
the
edgeGenerationSpeedUp
branch
from
0c32ed9
to
12ff390
Compare
Profiling the edge generation indicated that it is a slow operation and since this edge list is generated over and over in various nodes it makes sense to cache it. Profiling of various nodes (generating edges) indicated a speedup of almost two orders of magnitude in generating the edges compared to the list comprehension counterpart (e.g. edges = [[i, i+1] for i in range(N)]. With the proposed optimization, the list of edges are stored in an edge cache (e.g. [[0,1], [1,2]... [n-1,n]] .. and the cache is extended as longer edge lists are requested/generated. Any call to get_edge_list will return a subset of this edge cache thus not having to re-generate the same list over and over. Various nodes like the line, spiral, torus knot, ellipse etc, which that generate lines with linear list of edges can benefit substantially from this speedup. To get the linear edge list use: edges = get_edge_list(n) returning: [[0, 1], [1, 2], ... , [n-1, n]] To get the loop edge list use: edges = get_edge_loop(n) returning: [[0, 1], [1, 2], ... , [n-2, n-1], [n-1, 0]]
DolphinDream
force-pushed
the
edgeGenerationSpeedUp
branch
from
12ff390
to
283a662
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Profiling the edge generation in some nodes indicated that it is a slow operation and since this edge list is generated over and over in various nodes it makes sense to cache it. By using caching a speedup of more than an order of magnitude in generating the edges was measured compared to generating the edges using list comprehension (e.g. edges = [[i, i+1] for i in range(N)].
With the proposed optimization, the list of edges are stored in an edge cache (e.g. [[0,1], [1,2]... [n-1,n]] .. and the cache is extended as longer edge lists are requested/generated. Any call to get_edge_list will return a subset of this edge cache thus not having to re-generate the same list over and over.
Various nodes like the line, spiral, torus knot, ellipse etc, which generate lines with linear list of edges can benefit substantially from this speedup.
example code:
Additionally, if multiple calls are going to be made to get_edge_list with different numbers (of which max value is known), one could further optimize the edge generation calls by pre-caching the largest edge list by first calling update_edge_list(maxN), this way the subsequent calls to get_edge_list with different n values (less than maxN) will not have to extend the cache since the cache is large enough to generate edges for all n values.