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.
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
