Re: [PATCH 2/2] trace: improve performance while category is disabled

[Gennady Kupava]

> Spotted yet another.  This function in a header file, that is
included by many source files, must be made "static inline" (which I
already did as without the fix I couldn't get 'pu' to compile).

Thanks, missed that, seems my compiler inlined all calls and I didn't
notice the problem.


On 27 November 2017 at 03:25, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> Just in case others notice style and whitespace issues, I've applied
>> the following to fix them, so there is no need to reroll only to fix
>> these.
>> ...
>> -inline int trace_pass_fl(struct trace_key *key) {
>> +inline int trace_pass_fl(struct trace_key *key)
>> +{
>>   return key->fd || !key->initialized;
>>  }
>
> Spotted yet another.  This function in a header file, that is
> included by many source files, must be made "static inline" (which I
> already did as without the fix I couldn't get 'pu' to compile).
>
>
>

Re: [PATCH 2/2] trace: improve performance while category is disabled

[Junio C Hamano]

Junio C Hamano  writes:

> Just in case others notice style and whitespace issues, I've applied
> the following to fix them, so there is no need to reroll only to fix
> these.
> ...
> -inline int trace_pass_fl(struct trace_key *key) {
> +inline int trace_pass_fl(struct trace_key *key)
> +{
>   return key->fd || !key->initialized;
>  }

Spotted yet another.  This function in a header file, that is
included by many source files, must be made "static inline" (which I
already did as without the fix I couldn't get 'pu' to compile).

Re: [PATCH 2/2] trace: improve performance while category is disabled

[Junio C Hamano]

Junio C Hamano  writes:

> gennady.kup...@gmail.com writes:
>
>> From: Gennady Kupava 
>>
>> - Do the check if the trace key is enabled sooner in call chain.
>> - Move just enough code from trace.c into trace.h header so all code
>>   necessary to determine that trace is disabled could be inlined to
>>   calling functions.
>
> Makes sense.  Will queue.
>
> Thanks.

Just in case others notice style and whitespace issues, I've applied
the following to fix them, so there is no need to reroll only to fix
these.

diff --git a/trace.h b/trace.h
index db10f2afeb..05395242aa 100644
--- a/trace.h
+++ b/trace.h
@@ -85,29 +85,29 @@ extern void trace_performance_since(uint64_t start, const 
char *format, ...);
if (trace_pass_fl(key)) \
trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key,   \
__VA_ARGS__);   \
-   } while(0)
+   } while (0)
 
 #define trace_printf(...) trace_printf_key(_default_key, __VA_ARGS__)
 
 #define trace_argv_printf(argv, ...)   \
do {\
if (trace_pass_fl(_default_key))  \
-  trace_argv_printf_fl(TRACE_CONTEXT, __LINE__,\
+   trace_argv_printf_fl(TRACE_CONTEXT, __LINE__,   \
argv, __VA_ARGS__); \
-   } while(0)
+   } while (0)
 
 #define trace_strbuf(key, data)
\
do {\
if (trace_pass_fl(key)) \
trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data);\
-   } while(0)
+   } while (0)
 
 #define trace_performance(nanos, ...)  \
do {\
if (trace_pass_fl(_perf_key)) \
trace_performance_fl(TRACE_CONTEXT, __LINE__, nanos,\
-__VA_ARGS__);  \
-   } while(0)
+__VA_ARGS__);  \
+   } while (0)
 
 #define trace_performance_since(start, ...)\
do {\
@@ -115,7 +115,7 @@ extern void trace_performance_since(uint64_t start, const 
char *format, ...);
trace_performance_fl(TRACE_CONTEXT, __LINE__,   \
 getnanotime() - (start),   \
 __VA_ARGS__);  \
-   } while(0)
+   } while (0)
 
 /* backend functions, use non-*fl macros instead */
 __attribute__((format (printf, 4, 5)))
@@ -129,7 +129,8 @@ extern void trace_strbuf_fl(const char *file, int line, 
struct trace_key *key,
 __attribute__((format (printf, 4, 5)))
 extern void trace_performance_fl(const char *file, int line,
 uint64_t nanos, const char *fmt, ...);
-inline int trace_pass_fl(struct trace_key *key) {
+inline int trace_pass_fl(struct trace_key *key)
+{
return key->fd || !key->initialized;
 }
 

Re: [PATCH 2/2] trace: improve performance while category is disabled

[Junio C Hamano]

gennady.kup...@gmail.com writes:

> From: Gennady Kupava 
>
> - Do the check if the trace key is enabled sooner in call chain.
> - Move just enough code from trace.c into trace.h header so all code
>   necessary to determine that trace is disabled could be inlined to
>   calling functions.

Makes sense.  Will queue.

Thanks.

> +inline int trace_pass_fl(struct trace_key *key) {
> + return key->fd || !key->initialized;
> +}

;-)

[PATCH 2/2] trace: improve performance while category is disabled

[gennady . kupava]

From: Gennady Kupava 

- Do the check if the trace key is enabled sooner in call chain.
- Move just enough code from trace.c into trace.h header so all code
  necessary to determine that trace is disabled could be inlined to
  calling functions.

Signed-off-by: Gennady Kupava 
---
 trace.c |  3 +--
 trace.h | 58 --
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/trace.c b/trace.c
index d47ea28e8..b7530b51a 100644
--- a/trace.c
+++ b/trace.c
@@ -25,6 +25,7 @@
 #include "quote.h"
 
 struct trace_key trace_default_key = { "GIT_TRACE", 0, 0, 0 };
+struct trace_key trace_perf_key = TRACE_KEY_INIT(PERFORMANCE);
 
 /* Get a trace file descriptor from "key" env variable. */
 static int get_trace_fd(struct trace_key *key)
@@ -172,8 +173,6 @@ void trace_strbuf_fl(const char *file, int line, struct 
trace_key *key,
print_trace_line(key, );
 }
 
-static struct trace_key trace_perf_key = TRACE_KEY_INIT(PERFORMANCE);
-
 static void trace_performance_vprintf_fl(const char *file, int line,
 uint64_t nanos, const char *format,
 va_list ap)
diff --git a/trace.h b/trace.h
index 24b32f8f4..db10f2afe 100644
--- a/trace.h
+++ b/trace.h
@@ -14,6 +14,7 @@ struct trace_key {
 extern struct trace_key trace_default_key;
 
 #define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name, 0, 0, 0 }
+extern struct trace_key trace_perf_key;
 
 extern void trace_repo_setup(const char *prefix);
 extern int trace_want(struct trace_key *key);
@@ -79,24 +80,42 @@ extern void trace_performance_since(uint64_t start, const 
char *format, ...);
  * comma, but this is non-standard.
  */
 
-#define trace_printf(...) \
-   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, _default_key, 
__VA_ARGS__)
-
-#define trace_printf_key(key, ...) \
-   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key, __VA_ARGS__)
-
-#define trace_argv_printf(argv, ...) \
-   trace_argv_printf_fl(TRACE_CONTEXT, __LINE__, argv, __VA_ARGS__)
-
-#define trace_strbuf(key, data) \
-   trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data)
-
-#define trace_performance(nanos, ...) \
-   trace_performance_fl(TRACE_CONTEXT, __LINE__, nanos, __VA_ARGS__)
-
-#define trace_performance_since(start, ...) \
-   trace_performance_fl(TRACE_CONTEXT, __LINE__, getnanotime() - (start), \
-__VA_ARGS__)
+#define trace_printf_key(key, ...) \
+   do {\
+   if (trace_pass_fl(key)) \
+   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key,   \
+   __VA_ARGS__);   \
+   } while(0)
+
+#define trace_printf(...) trace_printf_key(_default_key, __VA_ARGS__)
+
+#define trace_argv_printf(argv, ...)   \
+   do {\
+   if (trace_pass_fl(_default_key))  \
+  trace_argv_printf_fl(TRACE_CONTEXT, __LINE__,\
+   argv, __VA_ARGS__); \
+   } while(0)
+
+#define trace_strbuf(key, data)
\
+   do {\
+   if (trace_pass_fl(key)) \
+   trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data);\
+   } while(0)
+
+#define trace_performance(nanos, ...)  \
+   do {\
+   if (trace_pass_fl(_perf_key)) \
+   trace_performance_fl(TRACE_CONTEXT, __LINE__, nanos,\
+__VA_ARGS__);  \
+   } while(0)
+
+#define trace_performance_since(start, ...)\
+   do {\
+   if (trace_pass_fl(_perf_key)) \
+   trace_performance_fl(TRACE_CONTEXT, __LINE__,   \
+getnanotime() - (start),   \
+__VA_ARGS__);  \
+   } while(0)
 
 /* backend functions, use non-*fl macros instead */
 __attribute__((format (printf, 4, 5)))
@@ -110,6 +129,9 @@ extern void trace_strbuf_fl(const char *file, int line, 
struct trace_key *key,
 __attribute__((format (printf, 4, 5)))
 extern void trace_performance_fl(const char *file, int line,
 uint64_t nanos, const char *fmt, ...);
+inline int