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

[Kevin Fleming]

> On Aug. 23, 2011, 2:13 p.m., rmudgett wrote:
> > /trunk/channels/chan_sip.c, lines 25699-25701
> > <https://reviewboard.asterisk.org/r/1377/diff/1-2/?file=18571#file18571line25699>
> >
> >     The coding guidelines to not allow inline variable declarations.  Please separate the declaration from the initialization as you had it in your previous diff.
> 
> Kevin Fleming wrote:
>     The coding guidelines forbid 'mixed code and declarations', but this sort of usage is fine. The guideline was never intended to forbid this, but rather to keep all variable declarations together at the beginning of the enclosing block. Initialization of a variable with a computed value, even one which includes function calls, is acceptable.
> 
> Rob Gagnon wrote:
>     Good to know, Kevin. Thanks. I purposely did not put these at the top as to avoid the function calls if the conditions prior to their use caused the function to exit.

Ahh... my mistake then. I replied too quickly. If these variable declarations are not at the top of the block, then indeed they do not conform to the coding guidelines. At some point we should probably consider revising the guidelines, but at this point, the variable declarations should be at the top of the block, and they can be initialized later. As Richard posted before, it's perfectly acceptable to leave the variable uninitialized until they are used.


- Kevin


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


On Aug. 23, 2011, 1:56 p.m., Rob Gagnon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1377/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2011, 1:56 p.m.)
> 
> 
> Review request for Asterisk Developers, Paul Belanger, rmudgett, 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/70c79457/attachment-0001.htm>