[asterisk-dev] Astobj2 debugging change proposal

[Matthew Jordan]

On Mon, Jun 9, 2014 at 5:45 PM, Richard Mudgett <rmudgett at digium.com> wrote:

>
>
>
> On Mon, Jun 9, 2014 at 12:31 PM, George Joseph <
> george.joseph at fairview5.com> wrote:
>
>> On Mon, Jun 9, 2014 at 11:22 AM, Corey Farrell <git at cfware.com> wrote:
>>
>>> One thing I dislike about AST_DEVMODE is that changing it requires
>>> rerunning ./configure.  I feel ./configure should be restricted as much as
>>> possible to checking dependencies.  Any build configuration that is not
>>> directly enabled/disabled by an external dependency should be in menuselect.
>>>
>>> If we are going to make it so TEST_FRAMEWORK doesn't enable AO2_DEBUG, I
>>> think we should allow Bamboo to run without AO2_DEBUG once.  This way if
>>> any tests actually require it we can update dependencies.
>>>
>>
>> I ran the testsuite last night without AO2_DEBUG and didn't notice
>> anything different but I'm going to run it again now.
>>
>>
>>> I'm not sure how I feel about AO2_DEBUG being combined with REF_DEBUG.
>>> AO2_DEBUG doesn't appear to add much overhead, but REF_DEBUG adds
>>> significant overhead through output to the refs log.  I'm not sure someone
>>> who specifically wants AO2_DEBUG would want REF_DEBUG.  Especially since
>>> REF_DEBUG can change timing and that could change results.
>>>
>>> Agreed.  REF_DEBUG seems more aimed at debugging the use of astobj2
>> whereas AO2_DEBUG is more aimed at debugging astobj2 itself.  2 different
>> purposes.
>>
>
> Yes.   REF_DEBUG and AO2_DEBUG have different purposes so they should be
> separate.  AO2_DEBUG
> can add its own potentially significant overhead checking ao2 containers
> for consistency.  The main reason
> it currently does not add too much overhead is that the hash container
> consistency checking is disabled if
> there isn't a sort function.  It was disabled because of abuses of the
> hash function callback by chan_iax2.
> There are some other users abusing it as well.  (Apparently unit tests and
> frame formats?)
>
>

Works for me.


>
>>> On Mon, Jun 9, 2014 at 12:46 PM, Matthew Jordan <mjordan at digium.com>
>>> wrote:
>>>
>>>> On Sun, Jun 8, 2014 at 10:03 AM, George Joseph
>>>> <george.joseph at fairview5.com> wrote:
>>>> > Right now, the non-ref debugging code in astobj2 is triggered by a
>>>> mix of
>>>> > AST_DEVMODE and AO2_DEBUG and both get set if you want to run the test
>>>> > framework.  I've noticed though that the inclusion of the debugging
>>>> code can
>>>> > actually hide problems as well as highlight them, especially related
>>>> to
>>>> > performance and locking.   Case in point:  Try running 'test execute
>>>> > category /main/astobj2 name thrash' with REF_DEBUG turned on and off.
>>>>  While
>>>> > the test passes both ways, the timing is significantly different (and
>>>> not
>>>> > what you'd expect).  Luckily, you can turn REF_DEBUG on and off from
>>>> > menuselect.
>>>> >
>>>> > To provide a little more flexibility and visibility, I'd like to
>>>> propose the
>>>> > following...
>>>> >
>>>> > 1.  Change astobj2 so the non-ref debugging code is dependent on
>>>> AO2_DEBUG
>>>> > solely instead of the mix of AST_DEVMODE and AO2_DEBUG.  (REF_DEBUG
>>>> would
>>>> > remain as is)
>>>>
>>>> Having two different 'dev' or 'debug' compilation flags is confusing.
>>>> I'd be happy with having them collapsed down to one.
>>>>
>>>
> They should not be collapsed into one because they have different purposes.
>
>

Agreed; I had forgotten about the container integrity checking.


>
>>>> > 2.  Remove the code that automatically sets AO2_DEBUG if
>>>> TEST_FRAMEWORK is
>>>> > defined.
>>>>
>>>> Richard may want to chime in on this, since I'm pretty sure that was a
>>>> change he initiated.
>>>>
>>>> Personally, I think it is okay if they are decoupled. We can always
>>>> enable AO2_DEBUG from menuselect in Bamboo build plans, and I'm not
>>>> aware of any Asterisk Test Suite functionality that explicitly relies
>>>> on AO2_DEBUG being defined.
>>>>
>>>
> I linked TEST_FRAMEWORK and AO2_DEBUG so the testsuite would run with the
> container integrity checking enabled.  If it is changed to be settable in
> menuselect
> that is fine by me.
>

Make it so!

-- 
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140609/b1d31ba9/attachment.html>