We help IT Professionals succeed at work.

log4c practice

CEHJ
CEHJ asked
on
The following appears in category.h

/**
 * Return true if the category will log messages with priority @c
 * LOG4C_PRIORITY_DEBUG.
 *
 * @param a_category the log4c_category_t object
 * @returns Whether the category will log.
 **/ 
#if !defined(_WIN32) && !defined(__HP_cc)
static inline int log4c_category_is_debug_enabled(const log4c_category_t* a_category) 
{	
    return log4c_category_is_priority_enabled(a_category, LOG4C_PRIORITY_DEBUG); 
}
#else
#define log4c_category_is_debug_enabled(a) \
  (log4c_category_is_priority_enabled(a, LOG4C_PRIORITY_DEBUG))
#endif

Open in new window


and this seems to make sense, but what of the frequently used

if (log4c_category_is_debug_enabled(a_cat)) {
// Log something at debug level
}

Open in new window


? Does anyone here do something like log_debug(...) such that the if gets inlined too? Surely that would be more convenient than evaluating (explicitly) the if each time
Comment
Watch Question

I guess the idea is to evaluate the category the minimum number of times for efficiency. It makes sense for the if to be evaluated explicitly by the caller and for the boolean to be retained for the relevant scope.
Top Expert 2009

Commented:
>> such that the if gets inlined too?

What do you mean by that ? The if is already "in line", isn't it ?

If you want conditional logging, then you can't really get around the if ... It's in the nature of the beast.
Top Expert 2016

Author

Commented:
Sorry, i guess i'm not explaining myself very clearly.

The objective is to avoid repeatedly writing

[code]
if (log4c_category_is_debug_enabled(acat)) {
   //Then some
}[/code]

A wrapper function would obviously do it, but i was wondering if anyone here already has such a strategy when using this library..?
Top Expert 2009

Commented:
You mean you want to call the log4c_category_debug function ?
Top Expert 2016

Author

Commented:
My goal is to make logging debug calls with the least code clutter
You can roll your own [inline] function for this, if you make the category accessible to it. Then when you call the function, the category can be evaluated once only - e.g. you test log4c_category_is_debug_enabled(acat) and set a static variable with the result. Subsequent passes avoid evaluating the category. This of course assumes that you are only dealing with one category. You need to organise it in a way that is appropriate for your application.
Top Expert 2016

Author

Commented:
Yes, that sounds good. I wonder if you'd be good enough to post some code? - i'm a C newb really ;-)
Top Expert 2009

Commented:
>> My goal is to make logging debug calls with the least code clutter

Right, so what I said in http:#32991810 applies.
Top Expert 2009

Commented:
Top Expert 2016

Author

Commented:
>>http://log4c.sourceforge.net/category_8h.html#05c59bda7dd7782b375a328207332f06

Is that an example of what was mentioned at http:#32991841 ?

There are two motivations here:

a. less code clutter by not issuing an if statement
b. avoiding the expense of pushing the arguments to log4c_category_debug on the stack (don't forget, some log messages can be long) if the category isn't debug-enabled anyway (a wasted call)
> Is that an example of what was mentioned at http:#32991841  ?

The only reason to roll your own, is to retain a copy of the pointer to the category, so you can lose a parameter and have (say)...

  my_log4c_category_debug("message");

You would need an initialisation function to set up the category pointer that you retain.

Look at http://log4c.sourceforge.net/category_8h-source.html and you will see that they are all inlined, which means you won't be pushing, so b is dealt with for you by the library.
Top Expert 2016

Author

Commented:
>>The only reason to roll your own, is to retain a copy of the pointer to the category,

That's not so much an issue. It's more the if statement pain in my second code example
In which case Infinity08's comment http:#32991929 is your solution
Top Expert 2009

Commented:
>> a. less code clutter by not issuing an if statement
>> b. avoiding the expense of pushing the arguments to log4c_category_debug on the stack (don't forget, some log messages can be long) if the category isn't debug-enabled anyway (a wasted call)

You can't have both. Either you split up the check and the debug output operation (as in the code you posted in your question), or you use one call (as in the log4c_category_debug approach). There's no other way.
> You can't have both.

Unless I am mistaken, it is inlined, which means the arguments are not pushed and will be optimised out.
Top Expert 2016

Author

Commented:
>>In which case Infinity08's comment http:#32991929 is your solution

OK, but doesn't that then take us back to the same problem (apologies if i'm missing something due to my C ignorance)? In order to make that call an allocation for the arguments has to be made and the call might fail anyway.
Top Expert 2016

Author

Commented:
>>Unless I am mistaken, it is inlined, which means the arguments are not pushed and will be optimised out.

This is crucial, and any doubt on this issue should be quashed
Top Expert 2009

Commented:
I don't understand what you mean. What do you mean by "allocation for the arguments" and by "fail" ?
> This is crucial, and any doubt on this issue should be quashed

You need to look at the generated code to check, inlining is a cue to the compiler. If you generate assembler, you can check.
Top Expert 2016

Author

Commented:
>>I don't understand what you mean. What do you mean by "allocation for the arguments" and by "fail" ?

In log4c_category_debug(foo) memory has to be allocated for foo on the stack. 'Fail' means that logging debug might not be enabled so the call will fail in the sense that logging won't occur, yet the allocations have been made.
Top Expert 2009

Commented:
That's not something you should be worrying about. Let the compiler worry about this kind of optimization - that's what it's good at.

I get the impression that you're basically asking how to do this without executing any code (you don't want an if check, and you don't want a function call) ... and that's obviously not possible.
Top Expert 2016

Author

Commented:
>>That's not something you should be worrying about. Let the compiler worry about this kind of optimization - that's what it's good at.

I'm just wondering whether it's possible to avoid writing

if(log_debug_is_enabled) // pseudo-code

without calling log4c_category_debug(foo)

Best practice in log4j (the original library on which this is based) recommends if(log.isDebugEnabled()), for the simple reason that the expense of evaluating the if is less than that of pushing the arguments in log.debug(foo) if the logger is disabled

The same applies here no doubt. But...

Surely the language features of C allow me to use some kind of function macro such that at least i don't have to write if(x) every time?
An inline function is what you want:

inline void do_if_x()
{
    if (x) do();
}

Inlining suggests to the copiler that it expands the function inline. You define these in headers.

Macros are frowned upon these post-C++ days, because they go awry in complex expressions, but here it is:

#define do_if_x() \
    if (x) do();
Top Expert 2009
Commented:
>> Surely the language features of C allow me to use some kind of function macro such that at least i don't have to write if(x) every time?

I could give you the same answer as in my previous post, but it seems I didn't explain my point very well.

There are actually a few points to be made :

(a) premature optimization is not a good idea (you can spend a lot of time on it, and it's not even sure that it'll actually improve performance - it might actually be doing the opposite). So, just write it however it seems most appropriate, and if performance tests show that it causes an issue, you can think about optimizing it then.

(b) inline functions are often (but not always) inlined by the compiler - ie. there won't be an actual function call, and the parameters won't actually be copied onto the stack. The compiler will make a judgment call on whether to inline or not. If you're working with a decent compiler, then leave this kind of decision to the compiler - in most cases, it is in a better position than you to make the right decision. Again, if it turns out the compiler made the wrong decision, this can be caught by performance tests, and can be fixed at that point.


And finally, one more thing : of course you could write a C style macro that does the if check and the debug write. But I'd advise against that (especially in C++).
Top Expert 2016

Author

Commented:
Thanks guys. Difficult to choose the exact responses to accept, but you've both been helpful