unpack-trees: Make index lookahead less pessimal
authorBrian Downing <bdowning@lavos.net>
Fri, 11 Jun 2010 02:59:07 +0000 (21:59 -0500)
committerJunio C Hamano <gitster@pobox.com>
Fri, 18 Jun 2010 15:06:18 +0000 (08:06 -0700)
When traversing trees with an index, the current index pointer
(o->cache_bottom) occasionally has to be temporarily advanced forwards to
match the traversal order of the tree, which is not the same as the sort
order of the index.  The existing algorithm that did this (introduced in
730f72840cc50c523fe4cdd796ea2d2fc4571a28) would get "stuck" when the
cache_bottom was popped and then repeatedly check the same index entries
over and over.  This represents a serious performance regression for
large repositories compared to the old "broken" traversal order.

This commit makes a simple change to mitigate this.  Whenever
find_cache_pos sees that the current pos is also the cache_bottom, and
it has already been unpacked, it advances the cache_bottom as well as
the current pos.  This prevents the above "sticking" behavior without
dramatically changing the algorithm.

In addition, this commit moves the unpacked check above the
ce_in_traverse_path() check.  The simple bitmask check is cheaper, and
in the case described above will be firing quite a bit to advance the
cache_bottom after a tree pop.

This yields considerable performance improvements for large trees.
The following are the number of function calls for "git diff HEAD" on
the Linux kernel tree, with 33,307 files:

   Symbol               Calls Before   Calls After
   -------------------  ------------   -----------
   unpack_callback            35,332        35,332
   find_cache_pos             37,357        37,357
   ce_in_traverse_path     4,979,473        37,357
   do_compare_entry        6,828,181       251,925
   df_name_compare         6,828,181       251,925

And on a repository of 187,456 files:

   Symbol               Calls Before   Calls After
   -------------------  ------------   -----------
   unpack_callback           197,958       197,958
   find_cache_pos            208,460       208,460
   ce_in_traverse_path    37,308,336       208,460
   do_compare_entry      156,950,469     2,690,626
   df_name_compare       156,950,469     2,690,626

On the latter repository, user time for "git diff HEAD" was reduced from
5.58 to 0.42 seconds.  This is compared to 0.30 seconds before the
traversal order fix was implemented.

Signed-off-by: Brian Downing <bdowning@lavos.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

index 75f54ca..fe23408 100644 (file)
@@ -528,9 +528,17 @@ static int find_cache_pos(struct traverse_info *info,
                const char *ce_name, *ce_slash;
                int cmp, ce_len;
-               if (!ce_in_traverse_path(ce, info))
+               if (ce->ce_flags & CE_UNPACKED) {
+                       /*
+                        * cache_bottom entry is already unpacked, so
+                        * we can never match it; don't check it
+                        * again.
+                        */
+                       if (pos == o->cache_bottom)
+                               ++o->cache_bottom;
-               if (ce->ce_flags & CE_UNPACKED)
+               }
+               if (!ce_in_traverse_path(ce, info))
                ce_name = ce->name + pfxlen;
                ce_slash = strchr(ce_name, '/');