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

[jrose]

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

Ship it!



/team/group/bridge_construction/channels/chan_local.c
<https://reviewboard.asterisk.org/r/2461/#comment16029>

    Should you maybe go ahead and push these all down by a bit now that LOCAL_ALREADY_MASQED is gone?



/team/group/bridge_construction/channels/chan_local.c
<https://reviewboard.asterisk.org/r/2461/#comment16034>

    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.



/team/group/bridge_construction/channels/chan_local.c
<https://reviewboard.asterisk.org/r/2461/#comment16030>

    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?



/team/group/bridge_construction/channels/chan_local.c
<https://reviewboard.asterisk.org/r/2461/#comment16031>

    toss the isoutbound variable, change this to:
    
    res = local_queue_frame(p, IS_OUTBOUND(ast, p), f, ast, 1)
    
    Reduce a little SLOC. Unless you think this is easier to read or something. Eh.



/team/group/bridge_construction/main/bridging.c
<https://reviewboard.asterisk.org/r/2461/#comment16032>

    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.



/team/group/bridge_construction/main/bridging.c
<https://reviewboard.asterisk.org/r/2461/#comment16033>

    Same comment, different function.


All pretty minor stuff and I didn't see any show-stoppers. I think you are good to go.

- jrose


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/199ab904/attachment-0001.htm>