<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 />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I just thought I would point out the a few new problematic areas I found while merging up.</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/3/?file=12866#file12866line6030" style="color: black; font-weight: bold; text-decoration: underline;">trunk/main/pbx.c</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">static char *handle_show_application(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">5778</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">strncasecmp</span><span class="p">(</span><span class="n">word</span><span class="p">,</span> <span class="n">ast_get_extension_name</span><span class="p">(</span><span class="n">hint</span><span class="o">-></span><span class="n">exten</span><span class="p">),</span> <span class="n">wordlen</span><span class="p">)</span> <span class="o">&&</span> <span class="o">++</span><span class="n">which</span> <span class="o">></span> <span class="n">state</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">5983</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">strncasecmp</span><span class="p">(</span><span class="n">word</span><span class="p">,</span> <span class="n">ast_get_extension_name</span><span class="p">(</span><span class="n">hint</span><span class="o">-></span><span class="n">exten</span><span class="p">),</span> <span class="n">wordlen</span><span class="p">)</span> <span class="o">&&</span> <span class="o">++</span><span class="n">which</span> <span class="o">></span> <span class="n">state</span><span class="p">)</span> <span class="p">{</span></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 went out of order. Refer to comment on line 7053.</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/3/?file=12866#file12866line6074" style="color: black; font-weight: bold; text-decoration: underline;">trunk/main/pbx.c</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">static char *handle_show_hint(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">5818</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">strncasecmp</span><span class="p">(</span><span class="n">ast_get_extension_name</span><span class="p">(</span><span class="n">hint</span><span class="o">-></span><span class="n">exten</span><span class="p">),</span> <span class="n">a</span><span class="o">-></span><span class="n">argv</span><span class="p">[</span><span class="mi">3</span><span class="p">],</span> <span class="n">extenlen</span><span class="p">))</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">6025</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">strncasecmp</span><span class="p">(</span><span class="n">ast_get_extension_name</span><span class="p">(</span><span class="n">hint</span><span class="o">-></span><span class="n">exten</span><span class="p">),</span> <span class="n">a</span><span class="o">-></span><span class="n">argv</span><span class="p">[</span><span class="mi">3</span><span class="p">],</span> <span class="n">extenlen</span><span class="p">))</span> <span class="p">{</span></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;">Same here.</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/3/?file=12866#file12866line7313" style="color: black; font-weight: bold; text-decoration: underline;">trunk/main/pbx.c</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">void ast_merge_contexts_and_delete(struct ast_context **extcontexts, struct ast_hashtab *exttable, const char *registrar)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7053</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">AST_LIST_APPEND_LIST</span><span class="p">(</span><span class="o">&</span><span class="n">this</span><span class="o">-></span><span class="n">callbacks</span><span class="p">,</span> <span class="o">&</span><span class="n">hint</span><span class="o">-></span><span class="n">callbacks</span><span class="p">,</span> <span class="n">entry</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7263</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">AST_LIST_APPEND_LIST</span><span class="p">(</span><span class="o">&</span><span class="n">this</span><span class="o">-></span><span class="n">callbacks</span><span class="p">,</span> <span class="o">&</span><span class="n">hint</span><span class="o">-></span><span class="n">callbacks</span><span class="p">,</span> <span class="n">entry</span><span class="p">);</span></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;">Essentially any time the hints container is iterated over, the hints extension may be destroyed before the hint is locked. So if there are any unsafe operations that are going to happen on the extension, we need to check if it is NULL first.</pre>
</div>
<br />
<p>- Jeff</p>
<br />
<p>On November 9th, 2010, 2:34 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-09 02:34:19</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>trunk/include/asterisk.h <span style="color: grey">(294045)</span></li>
<li>trunk/main/asterisk.c <span style="color: grey">(294045)</span></li>
<li>trunk/main/pbx.c <span style="color: grey">(294045)</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>