Vertex access speedups #197
Conversation
Update documentation to point out that this option exists.
This prevent any(is.na(vs)) from getting evaluated if na.ok = TRUE
This is intended to be useful when the vertex indices are guaranteed to be in the set of possible vertices. This is true particularly after C-code when domain checks are clearly unnecessary. Also allows vertices to be pre-computed. This speeds up as_adj_list and ego
There was a problem hiding this comment.
This looks potentially quite useful, thanks for contributing!
I was thinking about one thing however. You are saying that the key time saver is the computation of V(graph) only once. I therefore wonder whether it would not be better to simply keep the existing create_vs function, but add the optional verts argument you now added to unsafe_create_vs? I am not sure of the performance impact of the ifelse, but I can imagine that is relatively minor. That is, we could hence have no need of the unsafe_create_vs and only have the "safe" create_vs function, but which can potentially be sped up if we pass in the vertices. There are a few other such use cases in the code base that might benefit from a similar speed up. What do you think about this?
|
This PR seems okay to be, but there are multiple occurrences of |
* Speed up if igraph_opt(return.vs.es) is false Update documentation to point out that this option exists. * Change condition in simple_vs_index to allow short-circuit This prevent any(is.na(vs)) from getting evaluated if na.ok = TRUE * Add unsafe_create_vs This is intended to be useful when the vertex indices are guaranteed to be in the set of possible vertices. This is true particularly after C-code when domain checks are clearly unnecessary. Also allows vertices to be pre-computed. This speeds up as_adj_list and ego * undo change to simple_vs_index Co-authored-by: Vincent Traag <vincent@traag.net> Co-authored-by: Tamás Nepusz <ntamas@gmail.com> [Triggered by ba04339]
A speed up for
egoandas_adj_list, this goes without adjustingigraph_opt("return.vs.es")so it will benefit users automatically. Time reduction foras_adj_liston a 40000 node graph dropped from 500 -> 9 seconds. The key time saver is only computingV(graph)once. This is done with the newunsafe_create_vsfunction documented in code and the commit message. Still ~100 fold slower thanigraph_options(return.vs.es = FALSE), but you get the benefit of reclassing the integer vectorsigraph.vsand associating the graph pointer.