[asterisk-dev] [Code Review] 2461: Get local channel optimization working using new bridge system.

[rmudgett]

> On April 19, 2013, 8:36 p.m., jrose wrote:
> > /team/group/bridge_construction/channels/chan_local.c, line 465
> > <https://reviewboard.asterisk.org/r/2461/diff/1/?file=36284#file36284line465>
> >
> >     The name 'optimized_out' seems to imply that it's an evaluation rather than an action since it's past tense. Why not just 'optimize_out'?
> >     
> >     Same goes for the ast_bridge_local_optimized_out function this uses.

The function name is a boolean test for where it is used:
if (optimized_out())
   we are done
else
   more work to do

I renamed it to got_optimized_out().


> On April 19, 2013, 8:36 p.m., jrose wrote:
> > /team/group/bridge_construction/channels/chan_local.c, lines 519-520
> > <https://reviewboard.asterisk.org/r/2461/diff/1/?file=36284#file36284line519>
> >
> >     I don't think this comment helps very much. We know it's not the chan and we know it's not the owner. Is this function intended to handle that situation, or is this just being tolerant of the case where the channel isn't something we expected it to be?

That is just to be tolerant of unexpected channel pointers since it would be dangerous if it is not what is expected.


> On April 19, 2013, 8:36 p.m., jrose wrote:
> > /team/group/bridge_construction/main/bridging.c, lines 3182-3186
> > <https://reviewboard.asterisk.org/r/2461/diff/1/?file=36286#file36286line3182>
> >
> >     Since this function doesn't guarantee the local channel is locked when you are done, should this perhaps be called trylock_local_chan? Just a question of consistency since most of the functions just named lock are functions that you can just call and not have to worry about what happens on.

Yes it does.  It assumes that the channel is already locked on entry and leaves with it still locked.

lock_local_chan() and lock_local_peer() are helper functions for ast_bridge_local_optimized_out() to avoid a lot of nasty exit paths.

Renamed to optimize_lock_chan_stack() and optimize_lock_peer_stack() respectively.


- rmudgett


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


On April 19, 2013, 7:57 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2461/
> -----------------------------------------------------------
> 
> (Updated April 19, 2013, 7:57 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21058
>     https://issues.asterisk.org/jira/browse/ASTERISK-21058
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Local channel optimization is needed to reduce system resources and frame delay through Asterisk.  This patch gets local channel optimization working again using the new bridging model.
> 
> The primary method of optimization is to merge bridges together.  The optimizing local channels continuously check for empty queues in the local;1 and local;2 channels.  Once it is found that the queues are empty and the bridges allow merging, either BridgeA or BridgeB is merged into the other bridge.
> 
> A future additional way to optimize out local channels in more circumstances is to swap the Local;2 channel in BridgeB with the only peer channel (A) in BridgeA.  This optimization strategy would be needed if BridgeB prohibits bridge merges.
> 
> A -- BridgeA -- Local;1-Local;2 -- BridgeB -- B
> 
> 
> Diffs
> -----
> 
>   /team/group/bridge_construction/bridges/bridge_builtin_features.c 386151 
>   /team/group/bridge_construction/channels/chan_local.c 386151 
>   /team/group/bridge_construction/include/asterisk/bridging.h 386151 
>   /team/group/bridge_construction/main/bridging.c 386151 
> 
> Diff: https://reviewboard.asterisk.org/r/2461/diff/
> 
> 
> Testing
> -------
> 
> * Checked that local channels can optimize themselves out in the normal situation as shown in the description.
> 
> * Checked that the local channels can optimize themselves out in the situation where there is a chain of 300 local channel pairs.  This seems to work faster than it used to, but that was not quantified.
> 
> * Checked that local channels could not optimize themselves out when the local;2 is running an application instead of being in another bridge.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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