Refactor rizin graph in rz_util
Your checklist for this pull request
- [ ] I've read the guidelines for contributing to this repository
- [ ] I made sure to follow the project's coding style
- [ ] I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
- [ ] I've added tests that prove my fix is effective or that my feature works (if possible)
- [ ] I've updated the rizin book with the relevant information (if needed)
Detailed description
working on implement functions Add more graph representation for rizin, see https://github.com/rizinorg/rizin/issues/4231
- support identifier for nodes
- matrix based graph
Test plan
- unit test
- replace old graph api and test
Closing issues
...
Looks already way way better then before! Thanks a lot.
Some questions you might have already considered.
1. Should the graph's API mostly work on Node pointers or the node ids? Having both would be nice. If the node content doesn't matter to the user. But working on pointers is fine for me as well for now. 2. Runtime complexity for getting neighbors. Please let's not clone the nodes every time the neighbor list is requested. This doesn't scale well. Instead a read only `RzIterator` over the lists/matrix is a better fit I think. 3. Could the elements in the List based graph implementation contain edges? It would maybe reduce internal complexity getting the neighbors. Especially for directed graphs.
- yes, this is also something I have been contemplating. don't know if it would be confused to provide both way to access. so I defined them mostly work on Node pointer, and they could turn
idinto pointer through hash_find - exactly, it did bring crazy workload for runtime. I'm looking for a better way to return neighbors, do u know which file provide a best practice sample for returning
RzIterator? - no they don't contain explicit edges, initially I implemented it as edge and nodes, but later remove edges to abstract the RzGraph and implement PoC. You r right It's good to have edge in impl for list-based graph, and to reduce the complexity, thanks for point out that
do u know which file provide a best practice sample for returning RzIterator
You can checkout ht_inc.c. The functions Ht_(as_iter_keys), Ht_(as_iter_mut) and the corresponding (iter_next etc.) return an iterator over a hash map. Your usecase is even simpler I think.
no they don't contain explicit edges, initially I implemented it as edge and nodes, but later remove edges to abstract the RzGraph and implement PoC. You r right It's good to have edge in impl for list-based graph, and to reduce the complexity, thanks for point out that
You make a good point there. Considering code complexity (meaning: many lines of code) vs. low runtime complexity.
If you don't want to add an edge list for now (because too much work/ too many things to consider), it is fine. We can still extend it.
cc @wargio
agreed with @Rot127
I think it is ok to have first an implementation that works, then we can improve the execution time.
@Heersin have you had any updates since?