<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/2879/">https://reviewboard.asterisk.org/r/2879/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 26th, 2013, 3:59 p.m. UTC, <b>David Lee</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/2879/diff/1/?file=46307#file46307line197" style="color: black; font-weight: bold; text-decoration: underline;">/team/group/performance/main/format_cap.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">struct ast_format_cap {</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">187</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">update_format_cap_string_cache</span><span class="p">(</span><span class="n">cap</span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">135</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ast_format_list_destroy</span><span class="p">(</span><span class="n">f_list</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">188</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ast_format_list_destroy</span><span class="p">(</span><span class="n">f_list</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Maybe this would be a micro-optimization, and not make a difference, but...
Instead of generating the string every time the format_cap changes, you can add a 'cache_valid' flag that gets unset.
ast_getformatname_multiple() can test the flag, and update the cache if needed.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Sounds like a reasonable idea to me.</pre>
<br />
<p>- Mark</p>
<br />
<p>On September 24th, 2013, 10:42 p.m. UTC, Mark Michelson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/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, David Lee and Joshua Colp.</div>
<div>By Mark Michelson.</div>
<p style="color: grey;"><i>Updated Sept. 24, 2013, 10:42 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<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;">While profiling Asterisk 12, it became apparent that ao2 container traversals account for a high percentage of CPU time. Drilling down a bit reveals that the heaviest user is the function ast_getformatname_multiple() in main/format_cap.c. Every time a channel snapshot is created, the native formats on the channel are traversed in order to create the string. Applying some smart caching could save some unnecessary traversals since native formats on a channel do not change often.
Due to the nature of ast_format_cap(), the best way to improve the situation was to modify main/format_cap.c to optionally cache the string representation of formats any time the format_cap changes. Rather than creating more permutations of allocations for ast_format_cap(), I consolidated into a single allocation function that takes a set of flags to determine behavior. If AST_FORMAT_CAP_FLAG_CACHE_STRINGS is passed in when allocating the ast_format_cap, then any updates will result in caching the string representation of the formats. This way, in the many channel snapshot creations that occur, the strings are simply copied from the ast_format_cap into the snapshot rather than having to re-build the strings each time.
As a secondary change, I also decreased the number of buckets in the format container from 283 to 11 since it is quite rare for a channel to have many formats on it at all.
Note to reviewers: the main bulk of what to look at here is in main/format_cap.c. Since I changed ast_format_cap_alloc(), I had to make changes all over the place. I used a sed script to make the changes, so there should be little risk of having made mistakes. The only allocation that has actually been changed is the allocation of channel native formats in main/channel.c</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;">Ran a SIPp test that sent 25 cps at Asterisk 12 (using chan_sip) with a maximum of 100 calls up at any given time. Dialplan for these calls simply answered, waited a second, and hung up. With these changes, no problems were encountered. top showed CPU percentage decrease by 1 to 2 percent on a test like this. When the cps and max calls were increased, the CPU savings increased linearly (i.e. 50 cps saved 2-4 % CPU).</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>/team/group/performance/addons/chan_mobile.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/addons/chan_ooh323.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/apps/app_confbridge.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/apps/app_meetme.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/apps/app_originate.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/bridges/bridge_holding.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/bridges/bridge_native_rtp.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/bridges/bridge_simple.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/bridges/bridge_softmix.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_alsa.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_bridge_media.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_console.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_dahdi.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_gtalk.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_h323.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_iax2.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_jingle.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_mgcp.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_misdn.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_motif.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_multicast_rtp.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_nbs.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_oss.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_phone.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_pjsip.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_sip.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_skinny.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/chan_unistim.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/channels/dahdi/bridge_native_dahdi.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/include/asterisk/format_cap.h <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/bridge_basic.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/ccss.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/channel.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/core_local.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/dial.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/file.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/format_cap.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/manager.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/media_index.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/main/rtp_engine.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/pbx/pbx_spool.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/res/ari/resource_bridges.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/res/parking/parking_applications.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/res/res_agi.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/res/res_clioriginate.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/res/res_pjsip/pjsip_configuration.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/res/res_pjsip_sdp_rtp.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/res/res_pjsip_session.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/res/res_stasis.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/tests/test_config.c <span style="color: grey">(399697)</span></li>
<li>/team/group/performance/tests/test_format_api.c <span style="color: grey">(399697)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2879/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>