vsankarl / dca

0 stars 0 forks source link

Logging with variadic templates #3

Open pcooksey opened 4 years ago

pcooksey commented 4 years ago

https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L23-L49

A rather interesting logging procedure. Are you reinventing sprintf?

https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L73

Also I'm still confused on what T value is in your custom logging? Why are you separating the first argument out the variadic list? I think I must be missing something. Oh Format is recursive! Got it. Why are you separating the variable out in Log?

What assumption is your template Format and Log function making about the arguments?

These functions: https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L73 https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L23 Can you improve them to be more efficient?

Following that can you improve these call? https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L76 https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L46

Imagine I pass in a rather large class of data that has an overload for printing it. What happens to my data with the current functions?

vsankarl commented 4 years ago

https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L23-L49

A rather interesting logging procedure. Are you reinventing sprintf?

{Venkat] With sprintf we need to pass in a pre-allocated large buffer as a parameter to hold the entire formatted data. Over here, that's precisely the problem we are trying to avoid, and it operates more so as "printf". Essentially fmt - would have the string with multiple formats, output allocates necessary buffer on the fly as formats get evaluated. Ex - fmt = "Today is %s" args = "wednesday" output = "Today is wednesday"

https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L73

Also I'm still confused on what T value is in your custom logging? Why are you separating the first argument out the variadic list? I think I must be missing something. Oh Format is recursive! Got it. Why are you separating the variable out in Log?

[Venkat] By "out", I take it as "output" variable. It is needed to store the formatted data. Note: fmt don't have enough space to hold the expanded formatted data. fmt is equivalent to printf format specifier.

What assumption is your template Format and Log function making about the arguments?

[Venkat]I I had been supporting the stringstream operator << overloads. All the basic data types without any width specifier. For custom data (ex - vector) I need to provide overload.

These functions: https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L73

https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L23

Can you improve them to be more efficient? Following that can you improve these call?

[Venkat] Since all the parameter expansion and template instantiation happens compile-time, it's likely( need to find article or documentation ) that compiler creates efficient object code. As such, there is not much runtime overhead. Technically there isn't a regression happening(read further), instead compiler does inlining of instantiated functions. Here is a random internet article - https://eli.thegreenplace.net/2014/variadic-templates-in-c/

However certainly this logic is not quite intuitive and I can consider using c-style va macros and dynamically allocating a buffer to get the format. But this again can be cumbersome.

Not sure where in the performance can be improved,

Here is an example of how it gets expanded.

Calling Format(" Foobar %d %d %s", output, 1, 2, "test" );

1st iteration ... <line 44> Output = Foobar 1 <line 46>Format(" %d %s", output, 2, "test" ) // note the parameter is decreased. this is done by the compiler. fmt is also moved to account.

2nd iteration .... <line 44> Output = Foobar 1 2 <line 46> Format(" %s", output, "test" ) // note the parameter is decreased. this is done by the compiler. fmt is also moved to account.

3rd iteration ... <line 44> Output = Foobar 1 2 test <line 46> Format("", output ) // note the parameter is decreased. this is done by the compiler.fmt is also moved to account. Calls the base case which does nothing.

4th iteration ( base case ) void Format(const char *s, std::string & output) // does nothing Format("", "Foobar 1 2 test" )

https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L76

https://github.com/svrvsm/dca/blob/4ffee5c516946a2f5e39e018c17c7b317ae48c40/src/include/utils.h#L46

Imagine I pass in a rather large class of data that has an overload for printing it. What happens to my data with the current functions? If proper overload exists, then it should be logged properly.

pcooksey commented 4 years ago

Ah I see that makes sense. Its a convenient way to remove the pre-allocated buffer.

So my issue with T is that you are just taking it by value. The same with the Args. With simple types int, double, char then not a concern but if you are passing larger data types then there may be some unnecessary copying (depending on compiler and optimization).

Your article which I've read before points out what I was suggesting in "Variadic templates for forwarding". Most of your functions are just passing along arguments so it would be more efficient to perfect forward them. With forwarding, you would handle rvalue properly, e.g., Log("%s &d", "string", Class())

Ah my mistake by "out" I meant why does T value exist in the log function declaration. You should be able to just forward the arguments to the next function which will then start the separation of arguments. I think original T value in the logging function was what confused me at first because it didn't have a purpose.

Oh I wasn't saying using marco. I like variadic templates and this is one of their prime use-cases. I just wanted to get your thoughts on their usage in your logging functions and if you had any thoughts on improving it.

Thank you for the explanation.

vsankarl commented 4 years ago

Oh, I see your point on performance. Sorry about it - too much coffee and late-night effects :) And thanks for taking a detailed look at the code.

Definitely forwarding reference makes sense and it helps to mitigate the unnecessary copy. I agree "value" can also be removed in the Log function. Let me see if I can scrap some time over the days to update the code.

pcooksey commented 4 years ago

No worries. I was mostly happy to see variadic templates and thought there could be some good discussion on your usage of them. Thanks for answering my questions.

Also no worries on updating the code (unless you really feel like doing it). You solved the original problem. I just thought your logging function was interesting and I saw a potential to improve it.