'Re: [asterisk-dev] [Code Review] Hints and devices from hints moved'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    Re: [asterisk-dev] [Code Review] Hints and devices from hints moved
From:       "Russell Bryant" <reviewboard () asterisk ! org>
Date:       2010-11-29 18:40:07
Message-ID: 20101129184007.31433.1165 () hotblack ! digium ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]

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

Ship it!

These last comments are pretty minor.  After these changes, feel free to me=
rge.  Excellent work!

branches/1.8/main/pbx.c
<https://reviewboard.asterisk.org/r/1003/#comment6199>

    It would be good to convert this comment to doxygen format.  Also, plea=
se convert your other comments to match the style of others with a '*' on e=
ach line:
    =

    /*!
     * \brief
     *
     * ...
     */

branches/1.8/main/pbx.c
<https://reviewboard.asterisk.org/r/1003/#comment6200>

    I think for the sake of simplifying the code, I would remove the entire=
 if block for the case that only a single device is specified.  Your code i=
n the else block will handle that case, too.

branches/1.8/main/pbx.c
<https://reviewboard.asterisk.org/r/1003/#comment6203>

    No need to +1 here since you include it in the structure definition

branches/1.8/main/pbx.c
<https://reviewboard.asterisk.org/r/1003/#comment6202>

    No need to add +1 here since you include a byte for the trailing null c=
haracter in the structure definition.

branches/1.8/main/pbx.c
<https://reviewboard.asterisk.org/r/1003/#comment6201>

    ast_malloc() will generate an ERROR for you on failure.

- Russell

On 2010-11-17 04:35:40, schmidts wrote:
> =

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

> (Updated 2010-11-17 04:35:40)
> =

> =

> Review request for Asterisk Developers.
> =

> =

> Summary
> -------
> =

> handle_statechange in pbx.c has a very great impact on call handling perf=
ormance. i have noticed this several times and i have written a solution wh=
ich 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_conta=
iner.
> =

> after thinking about the best solution IMO iterating through the whole hi=
nt list/container to maybe find a hint with a matching device is very expen=
sive 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 2=
500 different users on this, several times and it works just fine. If a sub=
scribe 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
> -----
> =

>   branches/1.8/main/pbx.c 295358 =

> =

> Diff: https://reviewboard.asterisk.org/r/1003/diff
> =

> =

> Testing
> -------
> =

> dialplan reload drops all devices out of the device container and recreat=
e 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
> =

>

[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://reviewboard.asterisk.org/r/1003/">https://reviewboard.asterisk.org/r/1003/</a>
  </td>
    </tr>
   </table>
   <br />

 <p>Ship it!</p>

 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">These last comments are \
pretty minor.  After these changes, feel free to merge.  Excellent work!</pre>  <br \
/>

<div>

<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;">  <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;">  <a \
href="https://reviewboard.asterisk.org/r/1003/diff/6/?file=12924#file12924line955" \
style="color: black; font-weight: bold; text-decoration: \
underline;">branches/1.8/main/pbx.c</a>  <span style="font-weight: normal;">

     (Diff revision 6)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"></pre></td>  <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: \
0; ">static struct ao2_container *hintdevices;</pre></td>

  </tr>
 </tbody>

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">955</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">/* \
WARNING:</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">956</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">   When \
holding this container&#39;s lock, do _not_ do anything that will cause \
conlock</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">957</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">   to be \
taken, unless you _already_ hold it. The ast_merge_contexts_and_delete</pre></td>  \
</tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">958</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">   function \
will take the locks in conlock/hints order, so any other</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">959</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">   paths that \
require both locks must also take them in that order.</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">960</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">*/</pre></td> \
</tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It would be \
good to convert this comment to doxygen format.  Also, please convert your other \
comments to match the style of others with a &#39;*&#39; on each line:

/*!
 * \brief
 *
 * ...
 */</pre>
</div>
<br />

<div>

<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;">  <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;">  <a \
href="https://reviewboard.asterisk.org/r/1003/diff/6/?file=12924#file12924line1025" \
style="color: black; font-weight: bold; text-decoration: \
underline;">branches/1.8/main/pbx.c</a>  <span style="font-weight: normal;">

     (Diff revision 6)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"></pre></td>  <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: \
0; ">static struct ao2_container *hintdevices;</pre></td>

  </tr>
 </tbody>

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1025</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">	if \
(!strchr(devicelist, &#39;&amp;&#39;)) {</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1026</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		devicelength = strlen(devicelist) + 1;</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1027</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">		if \
(!(device = ao2_t_alloc(sizeof(*device) + devicelength, NULL, &quot;allocating a \
hintdevice structure&quot;))) {</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1028</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">			return \
-1;</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1029</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		}</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1030</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		strcpy(device-&gt;hintdevice, devicelist);</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1031</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		device-&gt;hint = hint;</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1032</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		ao2_t_link(hintdevices, device, &quot;Linking device into hintdevice \
container.&quot;);</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1033</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		ao2_t_ref(device, -1, &quot;hintdevice is linked so we can \
unref&quot;);</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1034</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  \
</tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1035</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">	} else \
{</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1036</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">		if (!(str = \
ast_str_thread_get(&amp;hintdevice_data, 16))) {</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1037</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">			return \
-1;</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1038</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		}</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1039</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		ast_str_set(&amp;str, 0, &quot;%s&quot;, devicelist);</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1040</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">		parse = \
ast_str_buffer(str);</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1041</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  \
</tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1042</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">              \
while ((cur = strsep(&amp;parse, &quot;&amp;&quot;))) {</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1043</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">			devicelength = strlen(cur) + 1;</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1044</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">			if \
(!(device = ao2_t_alloc(sizeof(*device) + devicelength, NULL, &quot;allocating a \
hintdevice structure&quot;))) {</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1045</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">				return \
-1;</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1046</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">			}</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1047</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">			strcpy(device-&gt;hintdevice, cur);</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1048</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">			device-&gt;hint = hint;</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1049</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">			ao2_t_link(hintdevices, device, &quot;Linking device into hintdevice \
container.&quot;);</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1050</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">			ao2_t_ref(device, -1, &quot;hintdevice is linked so we can \
unref&quot;);</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1051</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		}</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1052</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">	}</pre></td> \
</tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think for \
the sake of simplifying the code, I would remove the entire if block for the case \
that only a single device is specified.  Your code in the else block will handle that \
case, too.</pre> </div>
<br />

<div>

<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;">  <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;">  <a \
href="https://reviewboard.asterisk.org/r/1003/diff/6/?file=12924#file12924line1043" \
style="color: black; font-weight: bold; text-decoration: \
underline;">branches/1.8/main/pbx.c</a>  <span style="font-weight: normal;">

     (Diff revision 6)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"></pre></td>  <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: \
0; ">static struct ao2_container *hintdevices;</pre></td>

  </tr>
 </tbody>

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1043</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">			devicelength = strlen(cur) + 1;</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">1044</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">			if \
(!(device = ao2_t_alloc(sizeof(*device) + devicelength, NULL, &quot;allocating a \
hintdevice structure&quot;))) {</pre></td>  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">No need to \
+1 here since you include it in the structure definition</pre> </div>
<br />

<div>

<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;">  <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;">  <a \
href="https://reviewboard.asterisk.org/r/1003/diff/6/?file=12924#file12924line4368" \
style="color: black; font-weight: bold; text-decoration: \
underline;">branches/1.8/main/pbx.c</a>  <span style="font-weight: normal;">

     (Diff revision 6)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int \
ast_extension_state(struct ast_channel *c, const char *context, const char \
*exten)</pre></td>

  </tr>
 </tbody>

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">4354</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">	if \
(!(cmpdevice = ast_malloc(sizeof(*cmpdevice) + strlen(sc-&gt;dev) + 1))) {</pre></td> \
</tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">No need to \
add +1 here since you include a byte for the trailing null character in the structure \
definition.</pre> </div>
<br />

<div>

<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;">  <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;">  <a \
href="https://reviewboard.asterisk.org/r/1003/diff/6/?file=12924#file12924line4369" \
style="color: black; font-weight: bold; text-decoration: \
underline;">branches/1.8/main/pbx.c</a>  <span style="font-weight: normal;">

     (Diff revision 6)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int \
ast_extension_state(struct ast_channel *c, const char *context, const char \
*exten)</pre></td>

  </tr>
 </tbody>

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">4355</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">		ast_log(LOG_ERROR,&quot;Cannot allocate device.\n&quot;);</pre></td>  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">ast_malloc() will generate an ERROR for you on failure.</pre> </div>
<br />

<p>- Russell</p>

<br />
<p>On November 17th, 2010, 4:35 a.m., schmidts wrote:</p>

<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://reviewboard.asterisk.orgrb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for Asterisk Developers.</div>
<div>By schmidts.</div>

<p style="color: grey;"><i>Updated 2010-11-17 04:35:40</i></p>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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. </pre>  </td>
 </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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.</pre>
  </td>
 </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>branches/1.8/main/pbx.c <span style="color: grey">(295358)</span></li>

</ul>

<p><a href="https://reviewboard.asterisk.org/r/1003/diff/" style="margin-left: \
3em;">View Diff</a></p>

  </td>
 </tr>
</table>

  </div>
 </body>
</html>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[prev in list] [next in list] [prev in thread] [next in thread] 