[asterisk-dev] [Code Review] 3175: timing: Improve performance for most timing implementations

[Mark Michelson]

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


Since this patch is optimazation-related, I have added some small suggestions that may or may not help (the optimizer may already be doing something equivalent). They're not opened as issues, so feel free to ignore if you want.


/trunk/res/res_timing_kqueue.c
<https://reviewboard.asterisk.org/r/3175/#comment20263>

    This could be made static, and if kevent() allows it, could be made const, too.



/trunk/res/res_timing_kqueue.c
<https://reviewboard.asterisk.org/r/3175/#comment20262>

    You missed a conversion from our_timer to timer on this line.



/trunk/res/res_timing_kqueue.c
<https://reviewboard.asterisk.org/r/3175/#comment20261>

    This can be static. If kevent() allows it, this could also be const.



/trunk/res/res_timing_timerfd.c
<https://reviewboard.asterisk.org/r/3175/#comment20260>

    You could make this static const.


- Mark Michelson


On Feb. 4, 2014, 3:16 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3175/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2014, 3:16 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The timing API is currently optimized for the case where timing implementations provide a file descriptor and any API invocations directly act on the file descriptor. In practice this turns out to not be the most common case. Timing implementations commonly store a structure with additional information and then have to use a container lookup on any API invocation to get to it.
> 
> The attached change removes this container lookup requirement by allowing implementations to store a pointer to their data directly. This removes the container lookup and the world is happier.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_timing_timerfd.c 407196 
>   /trunk/res/res_timing_pthread.c 407196 
>   /trunk/res/res_timing_kqueue.c 407196 
>   /trunk/res/res_timing_dahdi.c 407196 
>   /trunk/main/timing.c 407196 
>   /trunk/include/asterisk/timing.h 407196 
> 
> Diff: https://reviewboard.asterisk.org/r/3175/diff/
> 
> 
> Testing
> -------
> 
> Ran the timing test using the various timing implementations on Linux and confirmed they still work. I also placed some calls to a Playback and confirmed it works.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140205/f7384bb7/attachment.html>