[kamailio/kamailio] dispatcher: use avl-tree for ds_set indexing (#578)
1. Get set by id uses loop instead of recursion 2. All other routines that used to iterate over the list are re-factored to trace the tree recursively. You can view, comment on, or merge this pull request online at: https://github.com/kamailio/kamailio/pull/578 -- Commit Summary -- * dispatcher: use avl-tree for ds_set indexing -- File Changes -- M modules/dispatcher/dispatch.c (873) M modules/dispatcher/dispatch.h (13) M modules/dispatcher/dispatcher.c (215) -- Patch Links -- https://github.com/kamailio/kamailio/pull/578.patch https://github.com/kamailio/kamailio/pull/578.diff --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578
@miconda any updates on this PR? --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-217457326
The patch is really big and I didn't have time to analyze it properly. I expect to do it after the Kamailio World, till then it is no much spare time, next week it's also the release of 4.4.1. --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-217612439
Due to lots of changes at the foundation of indexing the destinations, I am thinking of making a new module with your patch in order to be able to compare for a while the results between the two. I all looks ok, then we may just keep one. --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-227064548
Sure. Sounds good. --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-227106116
Any updates on this? I would really like to finish with char* pool id's. --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-233685870
Unfortunately there was no time yet for it in my side. If you like and have time for it, duplicate the dispatcher module to dispatcheravl (or you can think of another name), apply your patch and make a pull request with the new module and then I can accept the pull request. Then we can think if there is some parts that new module can reuse from the old one to avoid as much as possible code duplication. --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-234163971
@miconda there's no problem to copy dispatcher. But who's gonna test the new one? About code duplication: the whole dispatcher logic is common for these 2 modules. The only difference is how we iterate through the "list" of pools and indexed search. The only way for improvement I see at the moment is to unify iteration and that was in my todo's. --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-234224643
Resuming after the holidays season -- so, based on your latest comments we should just merge. I looked a bit more at the patch, the indexing is only for dispatcher sets (groups), still don on an integer id. The addresses in each set are still kept as a list, right? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-246373259
Well the idea was to split these changes. Should I update the PR to include both? (hopefully will have some free time in a couple of weeks) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-246375073
Keeping the addresses in a list makes it optimal for round robin, which is something I like to have. Indexing the set ids in a tree is useful when one has lots of groups, but for addresses might add penalties to some very used algorithms. Also, I don't know if anyone has lots of addresses inside same group to make it worth on indexing with a different structure. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-246385272
Hmmm... Looks like my bad - should have had a better description of an objective. The only 2 things I want to have are: 1. Indexed groups (no change for a list of destinations inside) 2. String id's insead of int's (still no change inside groups) So the PR is for p1 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-246388916
Thanks, clear now. I will try to get all sorted and reviewed, then merge this PR. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#issuecomment-246389765
Merged #578. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/578#event-793734245
participants (2)
-
Alekzander Spiridonov -
Daniel-Constantin Mierla