Re: [PATCH] Avoid running lstat(2) on the same cache entry.

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Linus Torvalds <torvalds@...>
Cc: Johannes Schindelin <Johannes.Schindelin@...>, Junio C Hamano <gitster@...>, Git Mailing List <git@...>
Date: Sunday, January 20, 2008 - 5:40 pm

On Jan 20, 2008, at 7:20 PM, Linus Torvalds wrote:


I cloned the kernel tree and measured these numbers:

Best time before:

     $ time git commit >/dev/null
     real    0m1.487s
     user    0m0.786s
     sys     0m0.696s

Best time after:

     $ time git commit >/dev/null
     real    0m1.087s
     user    0m0.578s
     sys     0m0.508s



[...]

I used Mac OS X's Sampler to collect some statistics.

It took me some time to find out about and work around an
annoying limitation.  Sampler is not capable of simply sampling a
full run of a process but needs to be manually stopped before the
process terminates.  You literally have to push a button before
the process exits--what a crazy tool.  I worked around this by
adding a sleep(100) instruction.  Sampler allows you to prune the
samples collected in mach_wait_until() and the relative numbers
should be fine.

Here is what I have.

Before Junio's and your patch, lstat dominates with 25%:

# Report - Timed Samples for git-commit
# Samples < 20 have been filtered
# Generated from the visible portion of the outline view
+ 25% lstat
| + 25% lstat
| | - 17% wt_status_print
| | - 8.4% refresh_index
+ 16% getdirentries
| + 16% getdirentries
| | - 16% read_directory_recursive
+ 10% fnmatch
| + 10% fnmatch
| | + 10% excluded
| | | - 10% read_directory_recursive
+ 9.7% sha1_block_data_order
| + 9.7% sha1_block_data_order
| | + 9.7% SHA1_Update
| | | + 8.0% read_index_from
| | | | - 6.8% wt_read_cache
| | | - 1.6% ce_write
+ 8.6% cache_name_compare
| + 8.6% cache_name_compare
| | - 6.9% cmp_cache_name_compare
| | - 1.6% index_name_pos
- 5.2% __mmap
- 3.2% open
+ 2.0% excluded
| + 2.0% excluded
| | - 2.0% read_directory_recursive
- 1.6% mbrtowc_l
- 1.5% find_pack_entry_one
- 1.4% cmp_cache_name_compare
- 1.3% qsort



After your patches getdirentries and fnmatch dominate:

# Report - Timed Samples for git-commit
# Samples < 20 have been filtered
# Generated from the visible portion of the outline view
+ 29% getdirentries
| + 29% getdirentries
| | - 29% read_directory_recursive
+ 19% fnmatch
| + 19% fnmatch
| | + 19% excluded
| | | - 19% read_directory_recursive
+ 9.6% lstat
| + 9.6% lstat
| | - 9.5% refresh_index
+ 5.5% open
| + 5.5% open
| | + 3.9% excluded
| | | - 3.9% read_directory_recursive
| | - 1.6% read_directory_recursive
+ 3.9% cache_name_compare
| + 3.9% cache_name_compare
| | - 3.8% index_name_pos
+ 3.0% sha1_block_data_order
| + 3.0% sha1_block_data_order
| | + 3.0% SHA1_Update
| | | - 1.6% read_index_from
+ 2.8% excluded
| + 2.8% excluded
| | - 2.8% read_directory_recursive
- 2.5% __memcpy
- 1.9% strcmp
- 1.6% close
- 1.4% fnmatch1


The reports above were created with Sampler's "Invert call tree"
option, while the next one is generated without this option.

# Report - Timed Samples for git-commit
# Samples < 20 have been filtered
# Generated from the visible portion of the outline view
+ 85% run_status
| + 85% wt_status_print
| | + 73% read_directory
| | | + 73% read_directory_recursive
| | | | + 73% read_directory_recursive
| | | | | + 71% read_directory_recursive
| | | | | | + 44% read_directory_recursive
| | | | | | | + 15% excluded
| | | | | | | | - 12% fnmatch
| | | | | | | | - 1.6% open
| | | | | | | - 12% getdirentries
| | | | | | | + 10% read_directory_recursive
| | | | | | | | - 4.0% getdirentries
| | | | | | | | - 2.6% excluded
| | | | | | | |   1.7% read_directory_recursive
| | | | | | | - 2.6% dir_add_name
| | | | | | + 14% excluded
| | | | | | | - 11% fnmatch
| | | | | | - 8.0% getdirentries
| | | | | | - 1.6% dir_add_name
| | - 11% run_diff_index
+ 15% prepare_index
| + 10% refresh_index
| | - 9.7% lstat
| - 2.5% write_index
| - 1.9% read_index

With this kind of report you can see how the running time is
distributed over the different functions called by cmd_commit().



So roughly 50% of the running time is spent in getdirentries
and fnmatch on the MacBook Pro I used to run these tests.

	Steffen

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Updated in-memory index cleanup, Linus Torvalds, (Fri Jan 18, 11:25 pm)
[PATCH] Avoid running lstat(2) on the same cache entry., Junio C Hamano, (Sat Jan 19, 3:45 am)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Johannes Schindelin, (Sat Jan 19, 10:42 pm)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Johannes Schindelin, (Sat Jan 19, 9:48 pm)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Steffen Prohaska, (Sun Jan 20, 11:10 am)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Steffen Prohaska, (Sun Jan 20, 5:40 pm)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Johannes Schindelin, (Sun Jan 20, 11:18 am)
[PATCH] Try to resurrect the handling for 'diff-index -m', Johannes Schindelin, (Sun Jan 20, 11:21 am)
[PATCH] Also use unpack_trees() in do_diff_cache(), Johannes Schindelin, (Sun Jan 20, 11:19 am)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Linus Torvalds, (Sun Jan 20, 4:32 pm)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Linus Torvalds, (Sun Jan 20, 5:53 pm)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Johannes Schindelin, (Sun Jan 20, 7:34 pm)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Linus Torvalds, (Sun Jan 20, 7:58 pm)
Re: [PATCH] Also use unpack_trees() in do_diff_cache(), Johannes Schindelin, (Sun Jan 20, 8:19 pm)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Steffen Prohaska, (Sun Jan 20, 6:33 am)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Johannes Schindelin, (Sun Jan 20, 10:15 am)
Re: [PATCH] Avoid running lstat(2) on the same cache entry., Linus Torvalds, (Sat Jan 19, 10:02 pm)
[PATCH] index: be careful when handling long names, Junio C Hamano, (Sat Jan 19, 3:42 am)