Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Add performance tracing facility #46

Closed
wants to merge 3 commits into from

Conversation

kblees
Copy link

@kblees kblees commented Dec 4, 2012

Followup to pull request #38 which got closed by dropping the devel branch.

@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #69 SUCCESS
This pull request looks good
(what's this?)

@kblees
Copy link
Author

kblees commented Jun 9, 2013

Rebased to 1.8.3

An installer including this PR is available at https://docs.google.com/file/d/0BxXUoUg2r8ftczNmVFAtYXllTEE/edit?usp=sharing

Add a high resolution timer function as basis for performance debugging.

Simple use case (measure one code section):
doube start = ticks();
/* measure this */
fprintf(stderr, "time: %g s\n", (ticks() - start));

Complex use case (measure several code sections):
double t = 0;
for (;;) {
  /* ignore */
  t -= ticks();
  /* measure this */
  t += ticks();
  /* ignore */
}
fprintf(stderr, "time: %g s\n", t);

Signed-off-by: Karsten Blees <blees@dcon.de>
Add trace_performance and trace_performance_since macros that print file
name, line number, time and an optional printf-formatted text to the file
specified in environment variable GIT_TRACE_PERFORMANCE.

Unless enabled via GIT_TRACE_PERFORMANCE, these macros have no noticeable
impact on performance, so that test code may be shipped in release builds.

MSVC: variadic macros (__VA_ARGS__) require VC++ 2005 or newer.

Simple use case (measure one code section):
doube start = ticks();
/* measure this */
trace_performance_since(start, "foobar");

Medium use case (measure consecutive code sections):
doube start = ticks();
/* measure this */
start = trace_performance_since(start, "foobar1");
/* measure this */
trace_performance_since(start, "foobar2");

Complex use case (measure several code sections):
double t = 0;
for (;;) {
  /* ignore */
  t -= ticks();
  /* measure this */
  t += ticks();
  /* ignore */
}
trace_performance(t, "frotz");

Signed-off-by: Karsten Blees <blees@dcon.de>
Especially scripted git commands are still quite slow on Windows. Add
performance tracing to identify which commands are called by a script and
how long they execute.

Usage: > GIT_TRACE_PERFORMANCE=C:/git.log git svn rebase
creates a log file like this:
trace: at compat/mingw.c:2068, time: 0.001000 s: command: git.exe rev-parse --show-prefix
trace: at compat/mingw.c:2068, time: 0.001219 s: command: git.exe config --bool --get svn.fetchall
...

Signed-off-by: Karsten Blees <blees@dcon.de>
@kblees
Copy link
Author

kblees commented Oct 4, 2013

Rebased to 1.8.4

An installer including this PR is available at https://docs.google.com/uc?id=0BxXUoUg2r8ftZ21mR2RyOG9NLWc&export=download

@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #132 SUCCESS
This pull request looks good
(what's this?)

@sschuberth
Copy link

I think this would be very nice to have in upstream. Have you tried to submit this to the upstream Git mailing list?

@sschuberth
Copy link

@kblees Any update regarding my question above?

@kblees
Copy link
Author

kblees commented Nov 27, 2013

@sschuberth I haven't tried to submit upstream, as the QueryPerformanceCounter API is Windows only. I didn't find the time yet to port to POSIX (using clock_gettime(CLOCK_MONOTONIC)...)

@sschuberth
Copy link

@kblees I don't see a problem in the code being Windows only, as only Windows has issues running shell scripts at decent speed. So it makes sense to have that code only for Windows in order to track down the bottle necks, or?

@dscho
Copy link
Member

dscho commented Dec 17, 2013

I would agree with @sschuberth. We could carry it in our fork until the time when @kblees has enough time to submit upstream (and address all the issues they will invariably find).

@dscho
Copy link
Member

dscho commented Apr 15, 2014

@kblees do you think this would still be valuable in Git for Windows? I would then try to find some time to work with you to get it merged...

@dscho
Copy link
Member

dscho commented Apr 22, 2014

@kblees Hmm?

@kblees
Copy link
Author

kblees commented Apr 28, 2014

I'll try to finish the posix compatible version soonish (using clock_gettime if available). The patch series as is doesn't apply upstream due to lack of 9206e7f "Win32: move main macro to a function".

@sschuberth
Copy link

Thanks @kblees. When you finish the POSIX version, can I ask you to send the patch upstream instead of here? I'm getting a bit worried about the amount of changes we seem to have merged recently, and about how to track them / get them upstream.

@kblees
Copy link
Author

kblees commented Apr 28, 2014

@sschuberth: yes, sending upstream was the idea, I'll close this PR when its done

@dscho
Copy link
Member

dscho commented May 16, 2014

@kblees any updates for the curious like me? ;-)

@t-b
Copy link

t-b commented Jul 21, 2014

@kblees Is that already submitted upstream? From a quick glance on the git ML I would say yes.

@kblees
Copy link
Author

kblees commented Jul 23, 2014

@t-b thanks for the reminder, the upstream patch series graduated to master yesterday

@kblees kblees closed this Jul 23, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants