Avoid state_can_deliver call when SubOptions are not available causing Pub-sub performance improvement of 40 times

[janani4u]

For our use-case of a large pub-sub node containing about 1 lakh subscribers, a publish item call was taking more than 2 minutes(measured using Tsung).
In our use-case, the pub-sub subscribers are added by the node-owner himself through manage subscriptions and users are not allowed to manage their own subscription.

Hence SubOptions were never used.
Using eprof and other profiling tools, we were able to narrow the problem area to the method lists_member/2 that gets called by subscribed_nodes_by_jid method.
Through further analysis on the ejabberd code, we were able to see a solution where for our use-case of pubsub-owner managed subscriptions, call to state_can_deliver method seemed unnecessary.

This would help avoid the subsequent loop processing and call to the fateful lists_member method, thereby giving a performance boost in the order of 40 times.
For the same use-case, the average time taken went down from 2 minutes to about 3 seconds(Testing done using Tsung).

Please read more on this fix in this post below:-
https://techenthusiast4u.wordpress.com/2016/10/04/ejabberd_pubsub_performance_improvement/

[coveralls]

Coverage decreased (-0.002%) to 31.822% when pulling e8607bb on janani4u:master into 15ebb79 on processone:master.

[janani4u]

I see that the code-coverage decreased by 0.002% and hence this check was unsuccessful.
Is this code-coverage based out of elixir test-cases?
The change is in the mod_pubsub code where elixir test-case for many use-cases do not exist.

Would this code still be reviewed?

I believe this change can be a significant performance booster and the community would benefit greatly.

[janani4u]

@cromain @badlop Hi, I would like to know which ejabberd release can this fix be a part of?

[eswarantg]

@cromain @badlop We are looking forward to having this fix for our work. Do let know progress.

[cromain]

i need to review possible side effect on all possible cases

[cromain]

I applied a simple similar change. call to state_can_deliver does not cost when SubOpts=[] so I kept same structure of the loop to make it simpler to optimize later.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.