<p>Corey Farrell <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/8366">View Change</a></p><p>Patch set 4:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">I understand that but knowing the repercussions of it is a good<br>thing.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">I created a unit test with the above loop to generate the results<br>below.<br>I expected the patch to slow execution some because of the extra<br>function<br>call layer.  However, I frequently saw reported execution times<br>with and<br>without the patch being consistently about a second higher.  It<br>made it<br>very difficult to get believable results to compare with and<br>without the<br>patch when I compiled it one way and it returned higher times than<br>the<br>last time.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">I ran the test as well but with DONT_OPTIMIZE and MALLOC_DEBUG disabled.  My results were pretty consistent between iterations in each configuration so I'm only posting the total runtimes:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">   Without patch: 8874ms with the test in main/asterisk.c, 9028ms with the test in a module.<br> With this patch: 8918ms with the test in main/asterisk.c, 9290ms with the test in a module.<br>With my followup: 9345ms with the test in main/asterisk.c, 9409ms with the test in a module.</pre><p style="white-space: pre-wrap; word-wrap: break-word;">This means 100 million pairs of calls to ast_malloc/ast_free will take 50-250ms longer with Richard's patch, 400-500ms longer when my followup is included.  I'm unsure why my patch causes the runtime to increase, it actually eliminates an extra function call layer.  This leads me to wonder if optimization is bypassing the __ast_repl_malloc function in Richard's patch and causing it to be inlined.</p><ul style="list-style: none; padding-left: 20px;"></ul><p>To view, visit <a href="https://gerrit.asterisk.org/8366">change 8366</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/8366"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Ic07ad80b2c2df894db984cf27b16a69383ce0e10 </div>
<div style="display:none"> Gerrit-Change-Number: 8366 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 06 Mar 2018 04:26:54 +0000 </div>
<div style="display:none"> Gerrit-HasComments: No </div>