sha1_file: use strbuf_add() instead of strbuf_addf()

Replace use of strbuf_addf() with strbuf_add() when enumerating loose objects in for_each_file_in_obj_subdir(). Since we already check the length and hex-values of the string before consuming the path, we can prevent extra computation by using the lower- level method. One consumer of for_each_file_in_obj_subdir() is the abbreviation code. OID abbreviations use a cached list of loose objects (per object subdirectory) to make repeated queries fast, but there is significant cache load time when there are many loose objects. Most repositories do not have many loose objects before repacking, but in the GVFS case the repos can grow to have millions of loose objects. Profiling 'git log' performance in GitForWindows on a GVFS-enabled repo with ~2.5 million loose objects revealed 12% of the CPU time was spent in strbuf_addf(). Add a new performance test to p4211-line-log.sh that is more sensitive to this cache-loading. By limiting to 1000 commits, we more closely resemble user wait time when reading history into a pager. For a copy of the Linux repo with two ~512 MB packfiles and ~572K loose objects, running 'git log --oneline --parents --raw -1000' had the following performance: HEAD~1 HEAD ---------------------------------------- 7.70(7.15+0.54) 7.44(7.09+0.29) -3.4% Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Dec 4, 2017 at 09:06 UTC 163ee5e635233f8614c29c11c9b8ee02902d65c4
2 files changed +11 -5
sha1_file.c
+7 -5
@@ -1903,7 +1903,6 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
1903 origlen = path->len;
1904 strbuf_complete(path, '/');
1905 strbuf_addf(path, "%02x", subdir_nr);
1906 - baselen = path->len;
1906
1907 dir = opendir(path->buf);
1908 if (!dir) {
@@ -1914,15 +1913,18 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
1913 }
1914
1915 oid.hash[0] = subdir_nr;
1916 + strbuf_addch(path, '/');
1917 + baselen = path->len;
1918
1919 while ((de = readdir(dir))) {
1920 + size_t namelen;
1921 if (is_dot_or_dotdot(de->d_name))
1922 continue;
1923
1924 + namelen = strlen(de->d_name);
1925 strbuf_setlen(path, baselen);
1923 - strbuf_addf(path, "/%s", de->d_name);
1924 -
1925 - if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 &&
1926 + strbuf_add(path, de->d_name, namelen);
1927 + if (namelen == GIT_SHA1_HEXSZ - 2 &&
1928 !hex_to_bytes(oid.hash + 1, de->d_name,
1929 GIT_SHA1_RAWSZ - 1)) {
1930 if (obj_cb) {
@@ -1941,7 +1943,7 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
1943 }
1944 closedir(dir);
1945
1944 - strbuf_setlen(path, baselen);
1946 + strbuf_setlen(path, baselen - 1);
1947 if (!r && subdir_cb)
1948 r = subdir_cb(subdir_nr, path->buf, data);
1949
t/perf/p4211-line-log.sh
+4
@@ -35,4 +35,8 @@ test_perf 'git log --oneline --raw --parents' '
35 git log --oneline --raw --parents >/dev/null
36 '
37
38 +test_perf 'git log --oneline --raw --parents -1000' '
39 + git log --oneline --raw --parents -1000 >/dev/null
40 +'
41 +
42 test_done