[asterisk-dev] [Code Review] 2873: Stasis performance improvements

[David Lee]

> On Sept. 24, 2013, 1:40 p.m., rmudgett wrote:
> > /branches/12/main/stasis_message_router.c, lines 217-219
> > <https://reviewboard.asterisk.org/r/2873/diff/3/?file=46134#file46134line217>
> >
> >     Is it possible for route.callback() to be removed by stasis_message_router_remove() by some other thread?  If so you should not change struct stasis_message_route from an ao2 object.

The route variable is local to router_dispatch(), and it's filled in
by find_route() while the lock is held.

So, no, route.callback() can't be changed by any other thread.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2873/#review9780
-----------------------------------------------------------


On Sept. 23, 2013, 11:29 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2873/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2013, 11:29 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch addresses several performance problems that were found in
> the initial performance testing of Asterisk 12.
> 
> The Stasis dispatch object was allocated as an AO2 object, even though
> it has a very confined lifecycle. This was replaced with a straight
> ast_malloc().
> 
> The Stasis message router was spending an inordinate amount of time
> searching hash tables. In this case, most of our routers had 6 or
> fewer routes in them to begin with. This was replaced with an array
> that's searched linearly for the route.
> 
> We more heavily rely on AO2 objects in Asterisk 12, and the memset()
> in ao2_ref() actually became noticeable on the profile. This was
> #ifdef'ed to only run when AO2_DEBUG was enabled.
> 
> After being misled by an erroneous comment in taskprocessor.c during
> profiling, the wrong comment was removed.
> 
> 
> Diffs
> -----
> 
>   /branches/12/include/asterisk/stasis_message_router.h 399650 
>   /branches/12/main/astobj2.c 399650 
>   /branches/12/main/stasis.c 399650 
>   /branches/12/main/stasis_message_router.c 399650 
>   /branches/12/main/taskprocessor.c 399650 
>   /branches/12/res/res_pjsip/include/res_pjsip_private.h 399650 
>   /branches/12/tests/test_stasis.c 399650 
> 
> Diff: https://reviewboard.asterisk.org/r/2873/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> Using the setup below, sipp was run against the 12 branch both with
> and without my changes.
> 
> ==
> [test-server]$ cat extensions.conf 
> 
> [performance]
> exten => service,1,Answer
> exten => service,n,Wait(1)
> exten => service,n,Hangup
> 
> [test-server]$ cat sip.conf 
> 
> [perf](!)
> type=peer
> qualify=no
> disallow=all
> allow=g722
> allow=ulaw
> insecure=invite,port
> context=performance
> 
> [test-client](perf)
> host=test-client
> 
> Simple sipp scenario:
> [test-client]$ sipp -sn uac -l 100 -r 50 tests-server
> 
> 
> Thanks,
> 
> David Lee
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130924/8edc6739/attachment-0001.html>