[asterisk-dev] [Code Review] Optimize chan_sip.c check_rtp_timeout() function

rmudgett reviewboard at asterisk.org
Tue Aug 23 12:36:37 CDT 2011


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

Ship it!


Looks fine to me other than the comment below.  It sure simplifies the long lines in the function.


/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1377/#comment8057>

    Initialization not necessary since they are initialized later.
    
    Also you might wish to rename holdtimeout to hold_timeout to be consistent with the function name.


- rmudgett


On Aug. 23, 2011, 11:15 a.m., Rob Gagnon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1377/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2011, 11:15 a.m.)
> 
> 
> Review request for Asterisk Developers and Rob Gagnon.
> 
> 
> Summary
> -------
> 
> Copying from original post on Jira:
> 
> While reviewing how "rtptimeout" and "rtpholdtimeout" operate in code, I found that the check_rtp_timeout() function could be optimized a little to speed up asterisk performance.
> 
> Currently, three integers are fetched from the rtp instance multiple times apiece (causing of course, multiple stack operations)
> 
> In the worst-case scenario:
>  - ast_rtp_instance_get_timeout() is called 4 times.
>  - ast_rtp_instance_get_hold_timeout() is called 4 times.
>  - ast_rtp_instance_get_keepalive() is called 3 times.
>  
> With this patch, each function will be called only once, thus removing up to 9 stack push/pops during runtime
> Description posted on jira would be the same
> 
> 
> This addresses bug ASTERISK-18319.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18319
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 333000 
> 
> Diff: https://reviewboard.asterisk.org/r/1377/diff
> 
> 
> Testing
> -------
> 
> Compiled code with patch, ran on test system without any problems.  Turned logging way up, and verifiied that calls that were silent/disconnected by accident were still being hung-up on.
> 
> 
> Thanks,
> 
> Rob
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110823/767369cf/attachment.htm>


More information about the asterisk-dev mailing list