I would rather see TSDebug be optimized and exposing diags instead of creating
a new type of debug.
-Bryan
> On Jun 12, 2019, at 9:44 AM, Walt Karas <wka...@verizonmedia.com.INVALID>
> wrote:
>
> I don't believe so. The issue I'm trying to address is that there is still
> significant execution time overhead for debug output calls when
> proxy.config.diags.debug.enabled is 0. This change would make the
> execution time overhead negligible (if TSFastDbg is used or TSDebug is
> changed to be like the proposed TSFastDbg). Another way of putting it is,
> I want to clean up the existing TSDEBUG alternative to TSDebug.
>
> On Wed, Jun 12, 2019 at 11:31 AM Bryan Call <bc...@apache.org> wrote:
>
>> How would this be controlled on the command line when running
>> trafic_server -T?
>>
>> It looks like it is a departure from how debug tags and controlled now.
>>
>> -Bryan
>>
>>
>>> On Jun 11, 2019, at 1:21 PM, Walt Karas <wka...@verizonmedia.com.INVALID>
>> wrote:
>>>
>>> Any +1's for simply replacing TSDebug with the proposed TSFastDbg ?
>>>
>>> On Mon, Apr 8, 2019 at 6:20 PM Leif Hedstrom <zw...@apache.org> wrote:
>>>
>>>>
>>>>
>>>> On Apr 4, 2019, at 8:05 AM, Walt Karas <wka...@verizonmedia.com
>> .INVALID>
>>>> wrote:
>>>>
>>>> Because senility. probably "on" should be a non-pointer of type const
>>>> volatile char, and InkAPI in the core can just keep a pointer to the
>> whole
>>>> structure for updating.
>>>>
>>>> But I'm leaning more to fixing up the capability that already kinda
>> there.
>>>> Which is optimizimg only for the case when all debug output is disabled.
>>>> Especially since maybe zwoop would be on my side for once :o) .
>>>>
>>>>
>>>>
>>>> My bad, whatever Walt said, I take the opposite stance.
>>>>
>>>> — leif
>>>>
>>>> P.s
>>>> J/K!
>>>>
>>>>
>>>> On Thu, Apr 4, 2019 at 8:51 AM Alan Carroll
>>>> <solidwallofc...@verizonmedia.com.invalid> wrote:
>>>>
>>>> This could enable a significant speed up for debug tags. One point is
>> that
>>>> when the debug tag string is set, at that point the debug objects could
>> be
>>>> updated to the correct state according to the debug tag string, rather
>> than
>>>> checking it every time a debug message is logged.
>>>>
>>>> I have to ask, why is there a pointer to the enable flag, instead of
>> just
>>>> the enable flag? Your logic doesn't even check that flag, it checks
>> whether
>>>> the pointer is null.
>>>>
>>>> On Wed, Apr 3, 2019 at 11:52 AM Leif Hedstrom <zw...@apache.org> wrote:
>>>>
>>>>
>>>>
>>>> On Apr 3, 2019, at 10:47 AM, Walt Karas <wka...@verizonmedia.com
>>>>
>>>> .INVALID>
>>>>
>>>> wrote:
>>>>
>>>>
>>>> It looks like the TS API already has less granular version of this:
>>>>
>>>> extern int diags_on_for_plugins;
>>>>
>>>> #define TSDEBUG \
>>>>
>>>> if (diags_on_for_plugins) \
>>>>
>>>> TSDebug
>>>>
>>>>
>>>> A problem with this is if you write:
>>>>
>>>>
>>>> if (flag)
>>>>
>>>> TSDEBUG(yadayada);
>>>>
>>>> else
>>>>
>>>> do_something();
>>>>
>>>>
>>>> then the else will be associated with the if hidden in the macro. I'd
>>>> prefer to change to something like:
>>>>
>>>>
>>>> extern const volatile char TS_detail_dbg_output_enabled;
>>>>
>>>>
>>>> #define TSFastDbg(_TAG, ...) \
>>>> do { \
>>>> if (TS_detail_dbg_output_enabled) \
>>>> TSDebug(_TAG, __VA_ARGS__); \
>>>>
>>>> } while(0)
>>>>
>>>>
>>>> Yeh, that kinda sounds like a bug, doesn’t it?
>>>>
>>>>
>>>> The negative is that we'd only see the benefit when all debug output
>>>>
>>>> was
>>>>
>>>> disabled. But, in any case, that is the main benefit, to avoid impacts
>>>> during normal operation.
>>>>
>>>>
>>>> Right, we should always optimize for the common cases, obviously. When
>>>> there’s a decision to make, where you can either make it “equally”
>>>> expensive always, or more expensive in a rare case, but cheaper in the
>>>> common case, we should likely chose the latter.
>>>>
>>>>
>>>>
>>>>
>>>> Looking in src/tscore/Diags.cc, it looks like the debug enable flag is
>>>>
>>>> only
>>>>
>>>> set at startup time. So we wouldn't be able to enable TSFastDbg using
>>>> traffic_cntl without adding complexity.
>>>>
>>>> On Tue, Apr 2, 2019 at 5:48 PM Walt Karas <wka...@verizonmedia.com>
>>>>
>>>> wrote:
>>>>
>>>>
>>>> Add this new structure to apidefs.h:
>>>>
>>>> typedef struct TSFastDbgCntl_
>>>> {
>>>> const char * const tag; // nul-terminated string
>>>> const volatile char * const on; // pointer to 1-byte flag
>>>> }
>>>> TSFastDbgCntl;
>>>>
>>>> Add this new API function, that returns a pointer to an instance of
>>>>
>>>> the
>>>>
>>>> above structure:
>>>>
>>>> TSFastDbgCntl * TSCreateFastDbgCntl(const char *tag);
>>>>
>>>> Add this macro, which would be used in place of direct use of
>>>>
>>>> TSDebug().
>>>>
>>>>
>>>> #define TSFastDbg(_FD_CNTL, ...) \
>>>> do { \
>>>> if ((_FD_CNTL)->on) \
>>>> TSDebug((_FD_CNTL)->tag, __VA_ARGS__); \
>>>> } while(0)
>>>>
>>>> The first parameter to TSFastDbg() is a pointer that was returned by
>>>> TSCreateFastDbgCntl(). The remaining parameters are printf parameters
>>>> (format string and values matching format specifiers). The core would
>>>>
>>>> be
>>>>
>>>> responsible for changing the value of the 'on' fields in the
>>>>
>>>> TSFastDbgCntl
>>>>
>>>> instances. For direct calls to TSDebug(), even if the tag is
>>>>
>>>> disabled,
>>>>
>>>> all
>>>>
>>>> the parameters have to be prepared and pushed onto the stack. As was
>>>> discussed in IRC, you have to feel guilty about doing converting a
>>>> string_view to a string and outputting it with c_str(). Because the
>>>>
>>>> code
>>>>
>>>> to do this will be executed even if the debug tag is disabled.
>>>> Furthermore, for each call to TSDebug, it's necessary to do an
>>>>
>>>> associative
>>>>
>>>> lookup on the tag string to determine that output for the tag is
>>>>
>>>> disabled.
>>>>
>>>>
>>>> We would want to put equivalent capabilities in Diags.h in the core
>>>>
>>>> for
>>>>
>>>> debug output. The implementation is non-trivial, but I think it would
>>>>
>>>> take
>>>>
>>>> at most a week including the Au test additions.
>>>>
>>>> I looked at making TSFastDbg() an inline function rather than a macro:
>>>> https://godbolt.org/z/IfVbBk
>>>> The opitimization works well if you compile the code with the inline
>>>> function as C, but the optimization is poor if you compile it as C++.
>>>>
>>>> This
>>>>
>>>> difference exists for both gcc and clang. It's weird.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>>
