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: Steffen Prohaska <prohaska@...>
Cc: Johannes Schindelin <Johannes.Schindelin@...>, Junio C Hamano <gitster@...>, Git Mailing List <git@...>
Date: Sunday, January 20, 2008 - 2:20 pm

On Sun, 20 Jan 2008, Steffen Prohaska wrote:

Ok, very interesting.

That's a 28% reduction in running time (or, alternatively, if you want to 
make it sound better, a 39% performance improvement ;), but quite frankly, 
I was hoping for even more. On my tests (with the kernel tree), the patch 
series literally removed half of the system calls, and I was really hoping 
that performance would be almost linear in system calls, especially on 
Windows where they are more expensive.

But I guess that was a bit optimistic. On Linux, we still had about 50% 
user time, and I guess the same is _largely_ true on Windows. Cutting the 
system time in half only gives you about a quarter performance 
improvement, and I was just incorrect in expecting system calls to be the 
limiting factor.

That's still a nice performance improvement, of course, but I was really 
hoping for more, since people have claimed that git is slow on windows. 
Maybe that's simply not true any more now that (a) people use the thinner 
mingw layers and (b) many things - like git-commit - are built-in, so that 
there simply isn't the horrible process creation overhead.

I have absolutely no idea how to do performance analysis or even something 
simple as getting a list of system calls from Windows (even if I had a 
Windows machine, which I obviously don't ;), so I'm afraid I have no clue 
why git might be considered slow there. I was hoping this was it


Well, you're testing something with 7k files, and I was testing something 
with 23k files. The changes in question only affect the number of lstat's 
on those files, they don't affect things like the basic setup overhead we 
have for doign things like checking that refs are unique.

So your test-case does have relatively more overhead (compared to what got 
optimized) than the numbers I quoted. 

We also do know that while Linux has a very low-overhead lstat(), the real 
problem on OS X has been the memory management, not the filesystem. We had 
some case where the page fault overhead was something like two orders of 
magnitude bigger, didn't we?

(Yeah - just checked. Commit 6d2fa7f1b489c65e677c18eda5c144dbc5d614ab: 
"index-pack usage of mmap() is unacceptably slower.."). That took a minute 
on Linux, and an hour on OS X)

It would be good to have a system-level profile of something like this. On 
Linux, it's now mostly in user space with "git commit", and oprofile 
shows:

	samples  %        image name               app name                 symbol name
	23318    11.5388  git                      git                      cache_name_compare
	11910     5.8936  git                      git                      excluded
	9775      4.8371  libcrypto.so.0.9.8b      libcrypto.so.0.9.8b      (no symbols)
	7990      3.9538  libc-2.7.so              libc-2.7.so              strlen
	7468      3.6955  vmlinux                  vmlinux                  __d_lookup
	7137      3.5317  libc-2.7.so              libc-2.7.so              internal_fnmatch
	7047      3.4872  libz.so.1.2.3            libz.so.1.2.3            (no symbols)
	6317      3.1259  git                      git                      index_name_pos
	5537      2.7400  git                      git                      unpack_trees_rec
	5079      2.5133  vmlinux                  vmlinux                  ext3fs_dirhash
	4774      2.3624  libc-2.7.so              libc-2.7.so              strcmp
	4005      1.9819  vmlinux                  vmlinux                  __link_path_walk
	3436      1.7003  vmlinux                  vmlinux                  ext3_htree_store_dirent
	3421      1.6929  vmlinux                  vmlinux                  __kmalloc
	3188      1.5776  vmlinux                  vmlinux                  _atomic_dec_and_lock
	3041      1.5048  libc-2.7.so              libc-2.7.so              _int_malloc
	2904      1.4370  libc-2.7.so              libc-2.7.so              fnmatch@@GLIBC_2.2.5
	2727      1.3494  vmlinux                  vmlinux                  str2hashbuf
	...

and one thing to look out for would be that some of these things might be 
relatively much more costly on other systems.

fnmatch? It's certainly visible on Linux, I wonder if the Windows/OSX 
version is more expensive due to trying to be case-insensitive or 
something?

				Linus
-
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., Linus Torvalds, (Sun Jan 20, 2:20 pm)
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)