[asterisk-dev] [Code Review] Hints and devices from hints moved to ao2_container

[Tilghman Lesher]

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



trunk/include/asterisk/pbx.h
<https://reviewboard.asterisk.org/r/1003/#comment6107>

    It is not apparent that any of these need to exposed as public interfaces.  You should be fine with declaring each of these as static in pbx.c and removing them from pbx.h.
    
    If I've potentially missed one that does need to be a public API, then it needs to have the "ast_pbx_" prefix; otherwise, it will not be exported.



trunk/include/asterisk/pbx.h
<https://reviewboard.asterisk.org/r/1003/#comment6111>

    This appears to be only useful as a shutdown method and thus could be removed to _private.h.  Additionally, please change the prefix to "ast_pbx_" to indicate that the code belongs to the pbx compendium of code.



trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/1003/#comment6108>

    Ugly red marks



trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/1003/#comment6109>

    Space after if, space after comma



trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/1003/#comment6110>

    Please use the ast_str_buffer() abstraction, instead of directly accessing the members of the ast_str struct.


- Tilghman


On 2010-11-05 10:33:32, schmidts wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1003/
> -----------------------------------------------------------
> 
> (Updated 2010-11-05 10:33:32)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> handle_statechange in pbx.c has a very great impact on call handling performance. i have noticed this several times and i have written a solution which makes handle_statechange handling 61 times faster.
> 
> i have used the patch (rev. 293802 for 1.4) from russellb and jeffpeeler to also fix a deadlock problem and their part to move hints to an ao2_container.
> 
> after thinking about the best solution IMO iterating through the whole hint list/container to maybe find a hint with a matching device is very expensive way. So i have used a own container for devices which just have a link back to their hint. 
> This means there is a hashed cmp for devices which gives back a multiple object ao2_iterator for all hints, this device, which have a state change, is used in. 
> 
> these devices are not longer limited to any length so its possible to use a hint for 100 peers at once.
> 
> Tilghman Lesher noticed on the dev-list what would happen when a dynamic hint is used. I have tried this with one dynamic hint and subscribe up to 2500 different users on this, several times and it works just fine. If a subscribe for a dynamic hint comes in, a static extension with static appdata is generated which acts like a normal hint so there is no problem on this. 
> 
> 
> Diffs
> -----
> 
>   trunk/include/asterisk.h 294045 
>   trunk/include/asterisk/pbx.h 294045 
>   trunk/main/asterisk.c 294045 
>   trunk/main/pbx.c 294045 
> 
> Diff: https://reviewboard.asterisk.org/r/1003/diff
> 
> 
> Testing
> -------
> 
> dialplan reload drops all devices out of the device container and recreate them by their callback list.
> core show hint / hints show all hints also dynamic and their watchers
> 
> 
> loadtests shows the following result:
> 
> orig version: 
> handle_statechange COUNTER: 10009 overalltime: 7041 ms 
> MAX: 365 CPS
> 
> patched version:
> handle_statechange COUNTER: 10009 overalltime: 115 ms
> MAX: 580 CPS
> 
> same setup, same config tested with sipp.
> 
> 
> Thanks,
> 
> schmidts
> 
>