Skip to content
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

dispatcher: use avl-tree for ds_set indexing #578

Merged
merged 1 commit into from Sep 19, 2016

Conversation

sipidronov
Copy link
Contributor

  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.

@sipidronov
Copy link
Contributor Author

@miconda any updates on this PR?

@miconda
Copy link
Member

miconda commented May 7, 2016

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.

@miconda
Copy link
Member

miconda commented Jun 20, 2016

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.

@sipidronov
Copy link
Contributor Author

Sure. Sounds good.

@sipidronov
Copy link
Contributor Author

Any updates on this? I would really like to finish with char* pool id's.

@miconda
Copy link
Member

miconda commented Jul 21, 2016

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.

@sipidronov
Copy link
Contributor Author

@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.

@miconda
Copy link
Member

miconda commented Sep 12, 2016

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?

@sipidronov
Copy link
Contributor Author

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)

@miconda
Copy link
Member

miconda commented Sep 12, 2016

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.

@sipidronov
Copy link
Contributor Author

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

@miconda
Copy link
Member

miconda commented Sep 12, 2016

Thanks, clear now. I will try to get all sorted and reviewed, then merge this PR.

@miconda miconda merged commit e404e1a into kamailio:master Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants