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

[Rob Gagnon]

> On Aug. 23, 2011, 12:36 p.m., rmudgett wrote:
> > /trunk/channels/chan_sip.c, lines 25672-25674
> > <https://reviewboard.asterisk.org/r/1377/diff/1/?file=18571#file18571line25672>
> >
> >     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.

Although initialization is not needed, I always do this as a best practice thing.

As for the name, "holdtimeout" is the name used for the value everywhere else in chan_sip, including the sip.conf variable name.  "hold_timeout" only appears in the rtp instance code.


- Rob


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


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/d186f8bc/attachment.htm>