add performance tracing facility
v6 cover add performance tracing facility
Changes since v5:
[05/11]: GIT_TRACE_BARE=1 disables 'timestamp file:line' output for
unit tests that rely on trace output (t1510 and t5503)
[08/11]: Align original trace output at col 40
[09/11]: Dropped '(div 10e9)' from the commit message. [10/11]: Dropped trace_performance[_since]() return value and the
respective usage example. Renamed trace_performance_vfl to trace_performance_vprintf_fl.
The other patches are the versions from pu.
Karsten Blees (11):
trace: move trace declarations from cache.h to new trace.h
trace: consistently name the format parameter
trace: remove redundant printf format attribute
trace: factor out printing to the trace file
trace: add infrastructure to augment trace output with additional info
trace: add current timestamp to all trace output
trace: move code around, in preparation to file:line output
trace: add 'file:line' to all trace output
trace: add high resolution timer function to debug performance issues
trace: add trace_performance facility to debug performance issues
git: add performance tracing for git's main() function to debug
scripts
Makefile | 7 ++ cache.h | 13 +-- config.mak.uname | 1 + git-compat-util.h | 4 + git.c | 2 + t/t1510-repo-setup.sh | 2 +- t/t5503-tagfollow.sh | 8 +- trace.c | 313 ++++++++++++++++++++++++++++++++++++++++++++------ trace.h | 90 +++++++++++++++ 9 files changed, 387 insertions(+), 53 deletions(-) create mode 100644 trace.h
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 1/11 trace: move trace declarations from cache.h to new trace.h
Also include direct dependencies (strbuf.h and git-compat-util.h for __attribute__) so that trace.h can be used independently of cache.h, e.g. in test programs.
--- cache.h | 13 ++----------- trace.h | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 11 deletions(-) create mode 100644 trace.h
cache.h +2 -11
@@ -7,6 +7,7 @@#include "advice.h" #include "gettext.h" #include "convert.h" +#include "trace.h" #include SHA1_HEADER #ifndef git_SHA_CTX
@@ -1377,17 +1378,7 @@ extern void *alloc_tag_node(void);extern void *alloc_object_node (void );extern void alloc_report (void ); -/* trace.c */ -__attribute__((format (printf,1 ,2 ))) -extern void trace_printf (const char *format, ...); -__attribute__((format (printf,2 ,3 ))) -extern void trace_argv_printf (const char **argv,const char *format, ...); -extern void trace_repo_setup (const char *prefix); -extern int trace_want (const char *key); -__attribute__((format (printf,2 ,3 ))) -extern void trace_printf_key (const char *key,const char *fmt, ...); -extern void trace_strbuf (const char *key,const struct strbuf *buf); - +/* pkt-line.c */ void packet_trace_identity (const char *prog);/* add */
trace.h new file +17 -0
@@ -0,0 +1,17 @@ +#ifndef TRACE_H +#define TRACE_H + +#include "git-compat-util.h" +#include "strbuf.h" + +__attribute__((format (printf,1 ,2 ))) +extern void trace_printf (const char *format, ...); +__attribute__((format (printf,2 ,3 ))) +extern void trace_argv_printf (const char **argv,const char *format, ...); +extern void trace_repo_setup (const char *prefix); +extern int trace_want (const char *key); +__attribute__((format (printf,2 ,3 ))) +extern void trace_printf_key (const char *key,const char *fmt, ...); +extern void trace_strbuf (const char *key,const struct strbuf *buf); + +#endif /* TRACE_H */
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 2/11 trace: consistently name the format parameter
In reply to Karsten BleesThe format parameter to trace_printf functions is sometimes abbreviated 'fmt'. Rename to 'format' everywhere (consistent with POSIX' printf specification).
--- trace.c | 22 +++++++++++----------- trace.h | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-)
trace.c +11 -11
@@ -62,7 +62,7 @@ static int get_trace_fd(const char *key, int *need_close)static const char err_msg[] ="Could not trace into fd given by " "GIT_TRACE environment variable" ; -static void trace_vprintf (const char *key,const char *fmt,va_list ap) +static void trace_vprintf (const char *key,const char *format,va_list ap) {struct strbuf buf =STRBUF_INIT ;
@@ -70,25 +70,25 @@ static void trace_vprintf(const char *key, const char *fmt, va_list ap)return ;set_try_to_free_routine (NULL );/* is never reset */ -strbuf_vaddf (&buf, fmt, ap); +strbuf_vaddf (&buf, format, ap);trace_strbuf (key, &buf);strbuf_release (&buf); } __attribute__((format (printf,2 ,3 ))) -void trace_printf_key (const char *key,const char *fmt, ...) +void trace_printf_key (const char *key,const char *format, ...) {va_list ap; -va_start (ap, fmt); -trace_vprintf (key, fmt, ap); +va_start (ap, format); +trace_vprintf (key, format, ap);va_end (ap); } -void trace_printf (const char *fmt, ...) +void trace_printf (const char *format, ...) {va_list ap; -va_start (ap, fmt); -trace_vprintf ("GIT_TRACE" , fmt, ap); +va_start (ap, format); +trace_vprintf ("GIT_TRACE" , format, ap);va_end (ap); }
@@ -106,7 +106,7 @@ void trace_strbuf(const char *key, const struct strbuf *buf)close (fd); } -void trace_argv_printf (const char **argv,const char *fmt, ...) +void trace_argv_printf (const char **argv,const char *format, ...) {struct strbuf buf =STRBUF_INIT ;va_list ap;
@@ -117,8 +117,8 @@ void trace_argv_printf(const char **argv, const char *fmt, ...)return ;set_try_to_free_routine (NULL );/* is never reset */ -va_start (ap, fmt); -strbuf_vaddf (&buf, fmt, ap); +va_start (ap, format); +strbuf_vaddf (&buf, format, ap);va_end (ap);sq_quote_argv (&buf, argv,0 );
trace.h +1 -1
@@ -11,7 +11,7 @@ extern void trace_argv_printf(const char **argv, const char *format, ...);extern void trace_repo_setup (const char *prefix);extern int trace_want (const char *key); __attribute__((format (printf,2 ,3 ))) -extern void trace_printf_key (const char *key,const char *fmt, ...); +extern void trace_printf_key (const char *key,const char *format, ...);extern void trace_strbuf (const char *key,const struct strbuf *buf);#endif /* TRACE_H */
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 3/11 trace: remove redundant printf format attribute
In reply to Karsten Bleestrace_printf_key() is the only non-static function that duplicates the printf format attribute in the .c file, remove it for consistency.
--- trace.c | 1 - 1 file changed, 1 deletion(-)
trace.c +0 -1
@@ -75,7 +75,6 @@ static void trace_vprintf(const char *key, const char *format, va_list ap)strbuf_release (&buf); } -__attribute__((format (printf,2 ,3 )))void trace_printf_key (const char *key,const char *format, ...) {va_list ap;
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 4/11 trace: factor out printing to the trace file
In reply to Karsten BleesOpening and writing to the trace file is currently duplicated in trace_strbuf() and trace_argv_printf(). Factor out this logic to prepare for adding timestamp and file:line to trace output.
In case of trace_argv_printf(), this adds an additional trace_want() check to prevent unnecessary string formatting.
--- trace.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)
trace.c +19 -18
@@ -62,6 +62,21 @@ static int get_trace_fd(const char *key, int *need_close)static const char err_msg[] ="Could not trace into fd given by " "GIT_TRACE environment variable" ; +/* Print 'buf' verbatim to trace file designated by env var 'key' */ +static void do_trace_print (const char *key,const struct strbuf *buf) +{ +int fd, need_close =0 ; + + fd =get_trace_fd (key, &need_close); +if (!fd) +return ; + +write_or_whine_pipe (fd, buf->buf, buf->len, err_msg); + +if (need_close) +close (fd); +} +static void trace_vprintf (const char *key,const char *format,va_list ap) {struct strbuf buf =STRBUF_INIT ;
@@ -71,7 +86,7 @@ static void trace_vprintf(const char *key, const char *format, va_list ap)set_try_to_free_routine (NULL );/* is never reset */ strbuf_vaddf (&buf, format, ap); -trace_strbuf (key, &buf); +do_trace_print (key, &buf);strbuf_release (&buf); }
@@ -93,26 +108,15 @@ void trace_printf(const char *format, ...)void trace_strbuf (const char *key,const struct strbuf *buf) { -int fd, need_close =0 ; - - fd =get_trace_fd (key, &need_close); -if (!fd) -return ; - -write_or_whine_pipe (fd, buf->buf, buf->len, err_msg); - -if (need_close) -close (fd); +do_trace_print (key, buf); }void trace_argv_printf (const char **argv,const char *format, ...) {struct strbuf buf =STRBUF_INIT ;va_list ap; -int fd, need_close =0 ; - fd =get_trace_fd ("GIT_TRACE" , &need_close); -if (!fd) +if (!trace_want ("GIT_TRACE" ))return ;set_try_to_free_routine (NULL );/* is never reset */
@@ -122,11 +126,8 @@ void trace_argv_printf(const char **argv, const char *format, ...)sq_quote_argv (&buf, argv,0 );strbuf_addch (&buf,'\n' ); -write_or_whine_pipe (fd, buf.buf, buf.len, err_msg); +do_trace_print ("GIT_TRACE" , &buf);strbuf_release (&buf); - -if (need_close) -close (fd); }static const char *quote_crnl (const char *path)
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 5/11 trace: add infrastructure to augment trace output with additional info
In reply to Karsten BleesTo be able to add a common prefix or suffix to all trace output (e.g. a timestamp or file:line of the caller), factor out common setup and cleanup tasks of the trace* functions.
Some unit-tests use trace output to verify internal state, and variable output such as timestamps and line numbers are not useful there. Disable additional trace output if GIT_TRACE_BARE is set.
When adding a common prefix, it makes sense that the output of each trace call starts on a new line. Add '\n' in case the caller forgot.
Note that this explicitly limits trace output to line-by-line, it is no longer possible to trace-print just part of a line. Until now, this was just an implicit assumption (trace-printing part of a line worked, but messed up the trace file if multiple threads or processes were involved).
Thread-safety / inter-process-safety is also the reason why we need to do the prefixing and suffixing in memory rather than issuing multiple write() calls. Write_or_whine_pipe() / xwrite() is atomic unless the size exceeds MAX_IO_SIZE (8MB, see wrapper.c). In case of trace_strbuf, this costs an additional string copy (which should be irrelevant for performance in light of actual file IO).
While we're at it, rename trace_strbuf's 'buf' argument, which suggests that the function is modifying the buffer. Trace_strbuf() currently is the only trace API that can print arbitrary binary data (without barfing on '%' or stopping at '\0'), so 'data' seems more appropriate.
--- t/t1510-repo-setup.sh | 2 +- t/t5503-tagfollow.sh | 8 ++++---- trace.c | 53 ++++++++++++++++++++++++++++++++++++++++----------- trace.h | 2 +- 4 files changed, 48 insertions(+), 17 deletions(-)
t/t1510-repo-setup.sh +1 -1
@@ -57,7 +57,7 @@ test_repo () { export GIT_WORK_TREE fi && rm -f trace && - GIT_TRACE_SETUP="$(pwd)/trace" git symbolic-ref HEAD >/dev/null && + GIT_TRACE_BARE=1 GIT_TRACE_SETUP="$(pwd)/trace" git symbolic-ref HEAD >/dev/null && grep '^setup: ' trace >result && test_cmp expected result )
t/t5503-tagfollow.sh +4 -4
@@ -56,7 +56,7 @@ test_expect_success 'fetch A (new commit : 1 connection)' ' rm -f $U && ( cd cloned && - GIT_TRACE_PACKET=$UPATH git fetch && + GIT_TRACE_BARE=1 GIT_TRACE_PACKET=$UPATH git fetch && test $A = $(git rev-parse --verify origin/master) ) && get_needs $U >actual &&
@@ -86,7 +86,7 @@ test_expect_success 'fetch C, T (new branch, tag : 1 connection)' ' rm -f $U && ( cd cloned && - GIT_TRACE_PACKET=$UPATH git fetch && + GIT_TRACE_BARE=1 GIT_TRACE_PACKET=$UPATH git fetch && test $C = $(git rev-parse --verify origin/cat) && test $T = $(git rev-parse --verify tag1) && test $A = $(git rev-parse --verify tag1^0)
@@ -122,7 +122,7 @@ test_expect_success 'fetch B, S (commit and tag : 1 connection)' ' rm -f $U && ( cd cloned && - GIT_TRACE_PACKET=$UPATH git fetch && + GIT_TRACE_BARE=1 GIT_TRACE_PACKET=$UPATH git fetch && test $B = $(git rev-parse --verify origin/master) && test $B = $(git rev-parse --verify tag2^0) && test $S = $(git rev-parse --verify tag2)
@@ -146,7 +146,7 @@ test_expect_success 'new clone fetch master and tags' ' cd clone2 && git init && git remote add origin .. && - GIT_TRACE_PACKET=$UPATH git fetch && + GIT_TRACE_BARE=1 GIT_TRACE_PACKET=$UPATH git fetch && test $B = $(git rev-parse --verify origin/master) && test $S = $(git rev-parse --verify tag2) && test $B = $(git rev-parse --verify tag2^0) &&
trace.c +42 -11
86 lines not on this page · raw message
trace.h +1 -1
@@ -12,6 +12,6 @@ extern void trace_repo_setup(const char *prefix);extern int trace_want (const char *key); __attribute__((format (printf,2 ,3 )))extern void trace_printf_key (const char *key,const char *format, ...); -extern void trace_strbuf (const char *key,const struct strbuf *buf); +extern void trace_strbuf (const char *key,const struct strbuf *data);#endif /* TRACE_H */
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH v6 05/11] trace: add infrastructure to augment trace output with additional info
Quoted 7 lines · Karsten Blees
To be able to add a common prefix or suffix to all trace output (e.g. a timestamp or file:line of the caller), factor out common setup and cleanup tasks of the trace* functions.
Some unit-tests use trace output to verify internal state, and variable output such as timestamps and line numbers are not useful there. Disable additional trace output if
GIT_TRACE_BAREis set.
Hmph, this makes me wonder if we are better off making these additional trace output optional, i.e. not disabling with GIT_TRACE_BARE like this, but show the new output only when explicitly asked for by setting GIT_TRACE_PERF or something.
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH v6 05/11] trace: add infrastructure to augment trace output with additional info
Am 21.06.2014 00:33, schrieb Junio C Hamano:
Quoted 6 lines
Quoted 4 lines · Karsten Blees
To be able to add a common prefix or suffix to all trace output (e.g. a timestamp or file:line of the caller), factor out common setup and cleanup tasks of the trace* functions.
Quoted 9 lines
Quoted 3 lines
Some unit-tests use trace output to verify internal state, and variable output such as timestamps and line numbers are not useful there. Disable additional trace output if
GIT_TRACE_BAREis set.Hmph, this makes me wonder if we are better off making these additional trace output optional, i.e. not disabling with
GIT_TRACE_BARElike this, but show the new output only when explicitly asked for by settingGIT_TRACE_PERFor something.
GIT_TRACE_VERBOSE perhaps? It affects all trace output, not just GIT_TRACE_PERFORMANCE. The tests would still have to disable it explicitly, though, in case someone sets it in their profile.
However, IIRC conclusion of v4 discussion was that this would be useful for all trace output [1], so I think it should be the default.
[1] https://groups.google.com/forum/#!topic/msysgit/UMKTvmZX5aI
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH v6 05/11] trace: add infrastructure to augment trace output with additional info
Quoted 11 lines · Karsten Blees
Am 21.06.2014 00:33, schrieb Junio C Hamano:
Quoted 1 line
Karsten Blees <[email protected]> writes:
GIT_TRACE_VERBOSEperhaps? It affects all trace output, not justGIT_TRACE_PERFORMANCE. The tests would still have to disable it explicitly, though, in case someone sets it in their profile.However, IIRC conclusion of v4 discussion was that this would be useful for all trace output [1], so I think it should be the default.
[1] https://groups.google.com/forum/#!topic/msysgit/UMKTvmZX5aI
Hmph, after re-reading of that thread, I recall that I thought Peff's
Quoted 3 lines
Quoted 3 lines
Would it make
sense to just tie this to regular trace_* calls, and ifGIT_TRACE_PERFORMANCEis set, add a timestamp to each line?
was the conclusion (i.e. by default no change, ask with GIT_TRACE_PERFORMANCE), and that I found that reasonable back then.
As to the need to disable in the tests, yes, you are right. Just like we give predictiable
environment to tests by setting EDITOR, HOME, etc. in test-lib.sh, we should do the
same for these new settings there.
Thanks.
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 6/11 trace: add current timestamp to all trace output
In reply to Karsten BleesThis is useful to tell apart trace output of separate test runs.
It can also be used for basic, coarse-grained performance analysis. Note that the accuracy is tainted by writing to the trace file, and you have to calculate the deltas yourself (which is next to impossible if multiple threads or processes are involved).
--- trace.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
trace.c +10 -1
@@ -81,6 +81,10 @@ static int trace_bare = -1;static int prepare_trace_line (const char *key,struct strbuf *buf) { +struct timeval tv; +struct tm tm; +time_t secs; +if (!trace_want (key))return 0 ;
@@ -92,7 +96,12 @@ static int prepare_trace_line(const char *key, struct strbuf *buf)if (trace_bare)return 1 ; -/* add line prefix here */ +/* print current timestamp */ +gettimeofday (&tv,NULL ); + secs = tv.tv_sec; +localtime_r (&secs, &tm); +strbuf_addf (buf,"%02d:%02d:%02d.%06ld " , tm.tm_hour, tm.tm_min, + tm.tm_sec, tv.tv_usec);return 1 ; }
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 7/11 trace: move code around, in preparation to file:line output
In reply to Karsten BleesNo functional changes, just move stuff around so that the next patch isn't that ugly...
--- trace.c | 36 ++++++++++++++++++------------------ trace.h | 12 ++++++++---- 2 files changed, 26 insertions(+), 22 deletions(-)
trace.c +18 -18
@@ -127,20 +127,20 @@ static void trace_vprintf(const char *key, const char *format, va_list ap)print_trace_line (key, &buf); } -void trace_printf_key (const char *key,const char *format, ...) +void trace_argv_printf (const char **argv,const char *format, ...) { +struct strbuf buf =STRBUF_INIT ;va_list ap; -va_start (ap, format); -trace_vprintf (key, format, ap); -va_end (ap); -} -void trace_printf (const char *format, ...) -{ -va_list ap; +if (!prepare_trace_line ("GIT_TRACE" , &buf)) +return ; +va_start (ap, format); -trace_vprintf ("GIT_TRACE" , format, ap); +strbuf_vaddf (&buf, format, ap);va_end (ap); + +sq_quote_argv (&buf, argv,0 ); +print_trace_line ("GIT_TRACE" , &buf); }void trace_strbuf (const char *key,const struct strbuf *data)
@@ -154,20 +154,20 @@ void trace_strbuf(const char *key, const struct strbuf *data)print_trace_line (key, &buf); } -void trace_argv_printf (const char **argv,const char *format, ...) +void trace_printf (const char *format, ...) { -struct strbuf buf =STRBUF_INIT ;va_list ap; - -if (!prepare_trace_line ("GIT_TRACE" , &buf)) -return ; -va_start (ap, format); -strbuf_vaddf (&buf, format, ap); +trace_vprintf ("GIT_TRACE" , format, ap);va_end (ap); +} -sq_quote_argv (&buf, argv,0 ); -print_trace_line ("GIT_TRACE" , &buf); +void trace_printf_key (const char *key,const char *format, ...) +{ +va_list ap; +va_start (ap, format); +trace_vprintf (key, format, ap); +va_end (ap); }static const char *quote_crnl (const char *path)
trace.h +8 -4
@@ -4,14 +4,18 @@#include "git-compat-util.h" #include "strbuf.h" -__attribute__((format (printf,1 ,2 ))) -extern void trace_printf (const char *format, ...); -__attribute__((format (printf,2 ,3 ))) -extern void trace_argv_printf (const char **argv,const char *format, ...);extern void trace_repo_setup (const char *prefix);extern int trace_want (const char *key); + +__attribute__((format (printf,1 ,2 ))) +extern void trace_printf (const char *format, ...); + __attribute__((format (printf,2 ,3 )))extern void trace_printf_key (const char *key,const char *format, ...); + +__attribute__((format (printf,2 ,3 ))) +extern void trace_argv_printf (const char **argv,const char *format, ...); +extern void trace_strbuf (const char *key,const struct strbuf *data);#endif /* TRACE_H */
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 8/11 trace: add 'file:line' to all trace output
In reply to Karsten BleesThis is useful to see where trace output came from.
Add 'const char *file, int line' parameters to the printing functions and rename them to *_fl.
Add trace_printf* and trace_strbuf macros resolving to the *_fl functions and let the preprocessor fill in __FILE__ and __LINE__.
As the trace_printf* functions take a variable number of arguments, this requires variadic macros (i.e. '#define foo(...) foo_impl(__VA_ARGS__)'. Though part of C99, it is unclear whether older compilers support this. Thus keep the old functions and only enable variadic macros for GNUC and MSVC 2005+ (_MSC_VER 1400). This has the nice side effect that the old C-style declarations serve as documentation how the macros are to be used.
Print 'file:line ' as prefix to each trace line. Align the remaining trace output at column 40 to accommodate 18 char file names + 4 digit line number (currently there are 30 *.c files of length 18 and just 11 of 19). Trace output from longer source files (e.g. builtin/receive-pack.c) will not be aligned.
--- git-compat-util.h | 4 ++++ trace.c | 72 +++++++++++++++++++++++++++++++++++++++++++++---------- trace.h | 49 +++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 12 deletions(-)
git-compat-util.h +4 -0
@@ -704,6 +704,10 @@ void git_qsort(void *base, size_t nmemb, size_t size,#endif #endif +#if defined(__GNUC__) || (_MSC_VER >=1400 ) +#define HAVE_VARIADIC_MACROS 1 +#endif +/* * Preserves errno, prints a message, but gives no warning for ENOENT. * Always returns the return value of unlink(2).
trace.c +60 -12
132 lines not on this page · raw message
trace.h +49 -0
59 lines not on this page · raw message
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 9/11 trace: add high resolution timer function to debug performance issues
In reply to Karsten BleesAdd a getnanotime() function that returns nanoseconds since 01/01/1970 as unsigned 64-bit integer (i.e. overflows in july 2554). This is easier to work with than e.g. struct timeval or struct timespec. Basing the timer on the epoch allows using the results with other time-related APIs.
To simplify adaption to different platforms, split the implementation into a common getnanotime() and a platform-specific highres_nanos() function.
The common getnanotime() function handles errors, falling back to gettimeofday() if highres_nanos() isn't implemented or doesn't work.
getnanotime() is also responsible for normalizing to the epoch. The offset to the system clock is calculated only once on initialization, i.e. manually setting the system clock has no impact on the timer (except if the fallback gettimeofday() is in use). Git processes are typically short lived, so we don't need to handle clock drift.
The highres_nanos() function returns monotonically increasing nanoseconds relative to some arbitrary point in time (e.g. system boot), or 0 on failure. Providing platform-specific implementations should be relatively easy, e.g. adapting to clock_gettime() as defined by the POSIX realtime extensions is seven lines of code.
This version includes highres_nanos() implementations for:
* Linux: using clock_gettime(CLOCK_MONOTONIC)
* Windows: using QueryPerformanceCounter()
Todo:
* enable clock_gettime() on more platforms
* add Mac OSX version, e.g. using mach_absolute_time + mach_timebase_info
--- Makefile | 7 +++++ config.mak.uname | 1 + trace.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ trace.h | 1 + 4 files changed, 91 insertions(+)
Makefile +7 -0
@@ -340,6 +340,8 @@ all:: # # Define GMTIME_UNRELIABLE_ERRORS if your gmtime() function does not # return NULL when it receives a bogus time_t. +# +# Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1497,6 +1499,11 @@ ifdef GMTIME_UNRELIABLE_ERRORS BASIC_CFLAGS += -DGMTIME_UNRELIABLE_ERRORS endif +ifdef HAVE_CLOCK_GETTIME + BASIC_CFLAGS += -DHAVE_CLOCK_GETTIME + EXTLIBS += -lrt +endif + ifeq ($(TCLTK_PATH),) NO_TCLTK = NoThanks endif
config.mak.uname +1 -0
@@ -34,6 +34,7 @@ ifeq ($(uname_S),Linux) HAVE_PATHS_H = YesPlease LIBC_CONTAINS_LIBINTL = YesPlease HAVE_DEV_TTY = YesPlease + HAVE_CLOCK_GETTIME = YesPlease endif ifeq ($(uname_S),GNU/kFreeBSD) HAVE_ALLOCA_H = YesPlease
trace.c +82 -0
85 lines not on this page · raw message
trace.h +1 -0
@@ -6,6 +6,7 @@extern void trace_repo_setup (const char *prefix);extern int trace_want (const char *key); +extern uint64_t getnanotime (void );#ifndef HAVE_VARIADIC_MACROS
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 10/11 trace: add trace_performance facility to debug performance issues
In reply to Karsten BleesAdd trace_performance and trace_performance_since macros that print a duration and an optional printf-formatted text to the file specified in environment variable GIT_TRACE_PERFORMANCE.
These macros, in conjunction with getnanotime(), are intended to simplify performance measurements from within the application (i.e. profiling via manual instrumentation, rather than using an external profiling tool).
Unless enabled via GIT_TRACE_PERFORMANCE, these macros have no noticeable impact on performance, so that test code for well known time killers may be shipped in release builds. Alternatively, a developer could provide an additional performance patch (not meant for master) that allows reviewers to reproduce performance tests more easily, e.g. on other platforms or using their own repositories.
Usage examples:
Simple use case (measure one code section):
uint64_t start = getnanotime(); /* code section to measure */ trace_performance_since(start, "foobar");
Complex use case (measure repetitive code sections):
uint64_t t = 0;
for (;;) {
/* ignore */
t -= getnanotime();
/* code section to measure */
t += getnanotime();
/* ignore */
}
trace_performance(t, "frotz");--- trace.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ trace.h | 18 ++++++++++++++++++ 2 files changed, 65 insertions(+)
trace.c +47 -0
@@ -164,6 +164,27 @@ void trace_strbuf_fl(const char *file, int line, const char *key,print_trace_line (key, &buf); } +static const char *GIT_TRACE_PERFORMANCE ="GIT_TRACE_PERFORMANCE" ; + +static void trace_performance_vprintf_fl (const char *file,int line, +uint64_t nanos,const char *format, +va_list ap) +{ +struct strbuf buf =STRBUF_INIT ; + +if (!prepare_trace_line (file, line,GIT_TRACE_PERFORMANCE , &buf)) +return ; + +strbuf_addf (&buf,"performance: %.9f s" , (double ) nanos /1000000000 ); + +if (format && *format) { +strbuf_addstr (&buf,": " ); +strbuf_vaddf (&buf, format, ap); + } + +print_trace_line (GIT_TRACE_PERFORMANCE , &buf); +} +#ifndef HAVE_VARIADIC_MACROS void trace_printf (const char *format, ...)
@@ -195,6 +216,23 @@ void trace_strbuf(const char *key, const struct strbuf *data)trace_strbuf_fl (NULL ,0 , key, data); } +void trace_performance (uint64_t nanos,const char *format, ...) +{ +va_list ap; +va_start (ap, format); +trace_performance_vprintf_fl (NULL ,0 , nanos, format, ap); +va_end (ap); +} + +void trace_performance_since (uint64_t start,const char *format, ...) +{ +va_list ap; +va_start (ap, format); +trace_performance_vprintf_fl (NULL ,0 ,getnanotime () - start, + format, ap); +va_end (ap); +} +#else void trace_printf_key_fl(const char *file, int line, const char *key,
@@ -215,6 +253,15 @@ void trace_argv_printf_fl(const char *file, int line, const char **argv, va_end(ap); } +void trace_performance_fl (const char *file,int line,uint64_t nanos, +const char *format, ...) +{ +va_list ap; +va_start (ap, format); +trace_performance_vprintf_fl (file, line, nanos, format, ap); +va_end (ap); +} +#endif /* HAVE_VARIADIC_MACROS */
trace.h +18 -0
@@ -21,6 +21,14 @@ extern void trace_argv_printf(const char **argv, const char *format, ...);extern void trace_strbuf (const char *key,const struct strbuf *data); +/* Prints elapsed time (in nanoseconds) if GIT_TRACE_PERFORMANCE is enabled. */ +__attribute__((format (printf,2 ,3 ))) +extern void trace_performance (uint64_t nanos,const char *format, ...); + +/* Prints elapsed time since 'start' if GIT_TRACE_PERFORMANCE is enabled. */ +__attribute__((format (printf,2 ,3 ))) +extern void trace_performance_since (uint64_t start,const char *format, ...); +#else /*
@@ -56,6 +64,13 @@ extern void trace_strbuf(const char *key, const struct strbuf *data);#define trace_strbuf (key, data) \ trace_strbuf_fl(__FILE__, __LINE__, key, data) +#define trace_performance (nanos, ...) \ + trace_performance_fl(__FILE__, __LINE__, nanos, __VA_ARGS__) + +#define trace_performance_since (start, ...) \ + trace_performance_fl(__FILE__, __LINE__, getnanotime() - (start), \ + __VA_ARGS__) +/* backend functions, use non-*fl macros instead */ __attribute__((format (printf, 4, 5))) extern void trace_printf_key_fl(const char *file, int line, const char *key,
@@ -65,6 +80,9 @@ extern void trace_argv_printf_fl(const char *file, int line, const char **argv, const char *format, ...);extern void trace_strbuf_fl (const char *file,int line,const char *key,const struct strbuf *data); +__attribute__((format (printf,4 ,5 ))) +extern void trace_performance_fl (const char *file,int line, +uint64_t nanos,const char *fmt, ...);#endif /* HAVE_VARIADIC_MACROS */
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
PATCH v6 11/11 git: add performance tracing for git's main() function to debug scripts
In reply to Karsten BleesUse trace_performance to measure and print execution time and command line arguments of the entire main() function. In constrast to the shell's 'time' utility, which measures total time of the parent process, this logs all involved git commands recursively. This is particularly useful to debug performance issues of scripted commands (i.e. which git commands were called with which parameters, and how long did they execute).
Due to git's deliberate use of exit(), the implementation uses an atexit routine rather than just adding trace_performance_since() at the end of main().
Usage example: > GIT_TRACE_PERFORMANCE=~/git-trace.log git stash list
Creates a log file like this:
23:57:38.638765 trace.c:405 performance: 0.000310107 s: git command: 'git' 'rev-parse' '--git-dir' 23:57:38.644387 trace.c:405 performance: 0.000261759 s: git command: 'git' 'rev-parse' '--show-toplevel' 23:57:38.646207 trace.c:405 performance: 0.000304468 s: git command: 'git' 'config' '--get-colorbool' 'color.interactive' 23:57:38.648491 trace.c:405 performance: 0.000241667 s: git command: 'git' 'config' '--get-color' 'color.interactive.help' 'red bold' 23:57:38.650465 trace.c:405 performance: 0.000243063 s: git command: 'git' 'config' '--get-color' '' 'reset' 23:57:38.654850 trace.c:405 performance: 0.025126313 s: git command: 'git' 'stash' 'list'
--- git.c | 2 ++ trace.c | 22 ++++++++++++++++++++++ trace.h | 1 + 3 files changed, 25 insertions(+)
git.c +2 -0
@@ -568,6 +568,8 @@ int main(int argc, char **av)git_setup_gettext (); +trace_command_performance (argv); +/* * "git-xxxx" is the same as "git xxxx", but we obviously: *
trace.c +22 -0
@@ -404,3 +404,25 @@ inline uint64_t getnanotime(void)return now; } } + +static uint64_t command_start_time; +static struct strbuf command_line =STRBUF_INIT ; + +static void print_command_performance_atexit (void ) +{ +trace_performance_since (command_start_time,"git command:%s" , + command_line.buf); +} + +void trace_command_performance (const char **argv) +{ +if (!trace_want (GIT_TRACE_PERFORMANCE )) +return ; + +if (!command_start_time) +atexit (print_command_performance_atexit); + +strbuf_reset (&command_line); +sq_quote_argv (&command_line, argv,0 ); + command_start_time =getnanotime (); +}
trace.h +1 -0
@@ -7,6 +7,7 @@extern void trace_repo_setup (const char *prefix);extern int trace_want (const char *key);extern uint64_t getnanotime (void ); +extern void trace_command_performance (const char **argv);#ifndef HAVE_VARIADIC_MACROS
Signature · 21 lines
2.0.0.402.g13b8b25 -- -- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
From: "Karsten Blees" <[email protected]>
Quoted 26 lines
Changes since v5:
[05/11]: GIT_TRACE_BARE=1 disables 'timestamp file:line' output for
unit tests that rely on trace output (t1510 and t5503)
[08/11]: Align original trace output at col 40
[09/11]: Dropped '(div 10e9)' from the commit message. [10/11]: Dropped trace_performance[_since]() return value and the
respective usage example. Renamedtrace_performance_vfltotrace_performance_vprintf_fl.The other patches are the versions from pu.
Karsten Blees (11):
trace: move trace declarations fromcache.hto newtrace.h
trace: consistently name the format parameter
trace: remove redundant printf format attribute
trace: factor out printing to the trace file
trace: add infrastructure to augment trace output with additional
info
trace: add current timestamp to all trace output
trace: move code around, in preparation to file:line output
trace: add 'file:line' to all trace output
trace: add high resolution timer function to debug performance issues
trace: addtrace_performancefacility to debug performance issues
git: add performance tracing for git'smain()function to debug
scripts
Quoted 11 lines
Makefile | 7 ++ cache.h | 13 +-- config.mak.uname | 1 + git-compat-util.h | 4 + git.c | 2 + t/t1510-repo-setup.sh | 2 +- t/t5503-tagfollow.sh | 8 +- trace.c | 313 ++++++++++++++++++++++++++++++++++++++++++++------ trace.h | 90 +++++++++++++++ 9 files changed, 387 insertions(+), 53 deletions(-)
Should there be some documentation as well? Perhaps in t/README, or in Documentation/howto.
Quoted 3 lines
create mode 100644
trace.h
Philip
Signature · 17 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
Am 21.06.2014 00:49, schrieb Philip Oakley:
Quoted 2 lines
Should there be some documentation as well? Perhaps in t/README, or in
Documentation/howto.
I'll add Documentation/technical/api-trace.txt when I find the time. But lets settle on the final API first.
Signature · 18 lines
-- *** Please reply-to-all at all times *** *** (do not pretend to know who is subscribed and who is not) *** *** Please avoid top-posting. *** The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free. You received this message because you are subscribed to the Google Groups "msysGit" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, and view previous threads, visit this group at http://groups.google.com/group/msysgit?hl=en_US?hl=en --- You received this message because you are subscribed to the Google Groups "msysGit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.