From 706a9974a297ea1b38c6aab886b54598409725e8 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 17 May 2012 13:05:17 -0700 Subject: [PATCH 1/4] Basic setup for profiling This fixes the examples so they will build and adds a PROFILE option to the CMakeFile that enabled gprof info on non-Windows --- CMakeLists.txt | 5 +++++ examples/diff.c | 8 +++++++- examples/general.c | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d30d09df9..bfbabc0a5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -53,6 +53,7 @@ OPTION (BUILD_SHARED_LIBS "Build Shared Library (OFF for Static)" ON) OPTION (THREADSAFE "Build libgit2 as threadsafe" OFF) OPTION (BUILD_CLAR "Build Tests using the Clar suite" ON) OPTION (TAGS "Generate tags" OFF) +OPTION (PROFILE "Generate profiling information" OFF) # Platform specific compilation flags IF (MSVC) @@ -74,6 +75,10 @@ ELSE () IF (NOT MINGW) # MinGW always does PIC and complains if we tell it to SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC") ENDIF () + IF (PROFILE) + SET(CMAKE_C_FLAGS "-pg ${CMAKE_C_FLAGS}") + SET(CMAKE_EXE_LINKER_FLAGS "-pg ${CMAKE_EXE_LINKER_FLAGS}") + ENDIF () ENDIF() # Build Debug by default diff --git a/examples/diff.c b/examples/diff.c index 20e14e511..1b4ab549b 100644 --- a/examples/diff.c +++ b/examples/diff.c @@ -61,7 +61,13 @@ char *colors[] = { "\033[36m" /* cyan */ }; -int printer(void *data, char usage, const char *line) +int printer( + void *data, + git_diff_delta *delta, + git_diff_range *range, + char usage, + const char *line, + size_t line_len) { int *last_color = data, color = 0; diff --git a/examples/general.c b/examples/general.c index 0a908bc48..46f2009bc 100644 --- a/examples/general.c +++ b/examples/general.c @@ -273,7 +273,7 @@ int main (int argc, char** argv) // Once you have the entry object, you can access the content or subtree (or commit, in the case // of submodules) that it points to. You can also get the mode if you want. - git_tree_entry_2object(&objt, repo, entry); // blob + git_tree_entry_to_object(&objt, repo, entry); // blob // Remember to close the looked-up object once you are done using it git_object_free(objt); From b59c73d39a0bb3ddb6fd4e81f796018c2b3a0579 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 17 May 2012 13:06:20 -0700 Subject: [PATCH 2/4] Optimize away git_text_gather_stats in diff GProf shows `git_text_gather_stats` as the most expensive call in large diffs. The function calculates a lot of information that is not actually used and does not do so in a optimal order. This introduces a tuned `git_buf_is_binary` function that executes the same algorithm in a fraction of the time. --- src/buffer.c | 18 ++++++++++++++++++ src/buffer.h | 3 +++ src/diff_output.c | 9 ++------- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/buffer.c b/src/buffer.c index ef95839f6..29aaf3fec 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -445,3 +445,21 @@ int git_buf_common_prefix(git_buf *buf, const git_strarray *strings) return 0; } + +bool git_buf_is_binary(const git_buf *buf) +{ + int i, printable = 0, nonprintable = 0; + + for (i = 0; i < buf->size; i++) { + unsigned char c = buf->ptr[i]; + if (c > 0x1F && c < 0x7f) + printable++; + else if (c == '\0') + return true; + else if (!git__isspace(c)) + nonprintable++; + } + + return ((printable >> 7) < nonprintable); +} + diff --git a/src/buffer.h b/src/buffer.h index af760f901..090b43548 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -125,4 +125,7 @@ int git_buf_cmp(const git_buf *a, const git_buf *b); /* Fill buf with the common prefix of a array of strings */ int git_buf_common_prefix(git_buf *buf, const git_strarray *strings); +/* Check if buffer looks like it contains binary data */ +bool git_buf_is_binary(const git_buf *buf); + #endif diff --git a/src/diff_output.c b/src/diff_output.c index 9c8e07972..4ad736e26 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -174,15 +174,12 @@ static int file_is_binary_by_content( git_map *new_data) { git_buf search; - git_text_stats stats; if ((delta->old_file.flags & BINARY_DIFF_FLAGS) == 0) { search.ptr = old_data->data; search.size = min(old_data->len, 4000); - git_text_gather_stats(&stats, &search); - - if (git_text_is_binary(&stats)) + if (git_buf_is_binary(&search)) delta->old_file.flags |= GIT_DIFF_FILE_BINARY; else delta->old_file.flags |= GIT_DIFF_FILE_NOT_BINARY; @@ -192,9 +189,7 @@ static int file_is_binary_by_content( search.ptr = new_data->data; search.size = min(new_data->len, 4000); - git_text_gather_stats(&stats, &search); - - if (git_text_is_binary(&stats)) + if (git_buf_is_binary(&search)) delta->new_file.flags |= GIT_DIFF_FILE_BINARY; else delta->new_file.flags |= GIT_DIFF_FILE_NOT_BINARY; From a0d959628f2a9eff65088c3247f237aef5205e73 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 17 May 2012 13:14:17 -0700 Subject: [PATCH 3/4] Other optimization and warning fixes This fixes a warning left by the earlier optimization and addresses one of the other hotspots identified by GProf. --- src/buffer.c | 27 ++++++++++++++++----------- src/buffer.h | 10 +++++++--- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/buffer.c b/src/buffer.c index 29aaf3fec..f28aa216b 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -12,9 +12,9 @@ /* Used as default value for git_buf->ptr so that people can always * assume ptr is non-NULL and zero terminated even for new git_bufs. */ -char git_buf_initbuf[1]; +char git_buf__initbuf[1]; -static char git_buf__oom; +char git_buf__oom[1]; #define ENSURE_SIZE(b, d) \ if ((d) > buf->asize && git_buf_grow(b, (d)) < 0)\ @@ -25,7 +25,7 @@ void git_buf_init(git_buf *buf, size_t initial_size) { buf->asize = 0; buf->size = 0; - buf->ptr = git_buf_initbuf; + buf->ptr = git_buf__initbuf; if (initial_size) git_buf_grow(buf, initial_size); @@ -35,7 +35,7 @@ int git_buf_grow(git_buf *buf, size_t target_size) { int error = git_buf_try_grow(buf, target_size); if (error != 0) - buf->ptr = &git_buf__oom; + buf->ptr = git_buf__oom; return error; } @@ -44,7 +44,7 @@ int git_buf_try_grow(git_buf *buf, size_t target_size) char *new_ptr; size_t new_size; - if (buf->ptr == &git_buf__oom) + if (buf->ptr == git_buf__oom) return -1; if (target_size <= buf->asize) @@ -85,7 +85,7 @@ void git_buf_free(git_buf *buf) { if (!buf) return; - if (buf->ptr != git_buf_initbuf && buf->ptr != &git_buf__oom) + if (buf->ptr != git_buf__initbuf && buf->ptr != git_buf__oom) git__free(buf->ptr); git_buf_init(buf, 0); @@ -98,11 +98,15 @@ void git_buf_clear(git_buf *buf) buf->ptr[0] = '\0'; } +/* Moved to inline function: + bool git_buf_oom(const git_buf *buf) { - return (buf->ptr == &git_buf__oom); + return (buf->ptr == git_buf__oom); } +*/ + int git_buf_set(git_buf *buf, const char *data, size_t len) { if (len == 0 || data == NULL) { @@ -164,7 +168,7 @@ int git_buf_vprintf(git_buf *buf, const char *format, va_list ap) if (len < 0) { git__free(buf->ptr); - buf->ptr = &git_buf__oom; + buf->ptr = git_buf__oom; return -1; } @@ -244,7 +248,7 @@ char *git_buf_detach(git_buf *buf) { char *data = buf->ptr; - if (buf->asize == 0 || buf->ptr == &git_buf__oom) + if (buf->asize == 0 || buf->ptr == git_buf__oom) return NULL; git_buf_init(buf, 0); @@ -448,11 +452,12 @@ int git_buf_common_prefix(git_buf *buf, const git_strarray *strings) bool git_buf_is_binary(const git_buf *buf) { - int i, printable = 0, nonprintable = 0; + size_t i; + int printable = 0, nonprintable = 0; for (i = 0; i < buf->size; i++) { unsigned char c = buf->ptr[i]; - if (c > 0x1F && c < 0x7f) + if (c > 0x1F && c < 0x7F) printable++; else if (c == '\0') return true; diff --git a/src/buffer.h b/src/buffer.h index 090b43548..50c75f64e 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -15,9 +15,10 @@ typedef struct { size_t asize, size; } git_buf; -extern char git_buf_initbuf[]; +extern char git_buf__initbuf[]; +extern char git_buf__oom[]; -#define GIT_BUF_INIT { git_buf_initbuf, 0, 0 } +#define GIT_BUF_INIT { git_buf__initbuf, 0, 0 } /** * Initialize a git_buf structure. @@ -61,7 +62,10 @@ void git_buf_attach(git_buf *buf, char *ptr, size_t asize); * * @return false if no error, true if allocation error */ -bool git_buf_oom(const git_buf *buf); +GIT_INLINE(bool) git_buf_oom(const git_buf *buf) +{ + return (buf->ptr == git_buf__oom); +} /* * Functions below that return int value error codes will return 0 on From e3557172aff5814e32e58bceb015465dcbe9e5f3 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 17 May 2012 14:44:17 -0700 Subject: [PATCH 4/4] No point in keeping commented out fn --- src/buffer.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/buffer.c b/src/buffer.c index f28aa216b..783a36eb8 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -98,15 +98,6 @@ void git_buf_clear(git_buf *buf) buf->ptr[0] = '\0'; } -/* Moved to inline function: - -bool git_buf_oom(const git_buf *buf) -{ - return (buf->ptr == git_buf__oom); -} - -*/ - int git_buf_set(git_buf *buf, const char *data, size_t len) { if (len == 0 || data == NULL) {