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

[David Lee]

> On Sept. 20, 2013, 12:55 p.m., rmudgett wrote:
> > /branches/12/main/astobj2.c, lines 587-593
> > <https://reviewboard.asterisk.org/r/2873/diff/1/?file=46079#file46079line587>
> >
> >     Delete this as well.

While I agree that the clause can probably be safely deleted, it wasn't pertinent to my changes.


> On Sept. 20, 2013, 12:55 p.m., rmudgett wrote:
> > /branches/12/main/stasis_message_router.c, line 91
> > <https://reviewboard.asterisk.org/r/2873/diff/1/?file=46082#file46082line91>
> >
> >     Don't use single character variable names.
> >     Suggest use idx here.

Sure, but for loop variables, where the use is limited to five lines of code?


> On Sept. 20, 2013, 12:55 p.m., rmudgett wrote:
> > /branches/12/main/taskprocessor.c, lines 678-681
> > <https://reviewboard.asterisk.org/r/2873/diff/1/?file=46083#file46083line678>
> >
> >     Is this a ref leak of tps here?

No; we bump the ref when we add a task to the queue, and decrement it
when a task is pulled out of the queue. No task was pulled out, so no
need to decrement.

Will comment accordingly.


- David


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


On Sept. 20, 2013, 4:05 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2873/
> -----------------------------------------------------------
> 
> (Updated Sept. 20, 2013, 4:05 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.
> 
> It turns out that we don't have many subscribers to Stasis topics in
> the system. We have lots of caching and forwarding, but those don't
> use the threadpool. Since we have such a limited number subscribers,
> it made sense to simply use taskprocessors and allocate a thread per
> subscriber instead.
> 
> 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.
> 
> Finally, since we're more reliant on the taskprocessor, it was
> optimized to reduce the number of times it was locking/unlocking the
> tps mutex.
> 
> 
> Diffs
> -----
> 
>   /branches/12/configs/stasis.conf.sample 399546 
>   /branches/12/include/asterisk/stasis.h 399546 
>   /branches/12/main/astobj2.c 399546 
>   /branches/12/main/stasis.c 399546 
>   /branches/12/main/stasis_config.c 399546 
>   /branches/12/main/stasis_message_router.c 399546 
>   /branches/12/main/taskprocessor.c 399546 
>   /branches/12/res/res_pjsip/include/res_pjsip_private.h 399546 
> 
> Diff: https://reviewboard.asterisk.org/r/2873/diff/
> 
> 
> Testing
> -------
> 
> 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/20130920/41523dce/attachment-0001.htm>