Re: [PATCH v2 3/3] grep: disable threading in all but worktree case

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 02.12.2011 14:07, schrieb Thomas Rast:
Measuring grep performance showed that in all but the worktree case
(as opposed to --cached,<committish>  or<treeish>), threading
actually slows things down.  For example, on my dual-core
hyperthreaded i7 in a linux-2.6.git at v2.6.37-rc2, I got:

Threads       worktree case                 | --cached case
--------------------------------------------------------------------------
8 (default) | 2.17user 0.15sys 0:02.20real  | 0.11user 0.26sys 0:00.11real
4           | 2.06user 0.17sys 0:02.08real  | 0.11user 0.26sys 0:00.12real
2           | 2.02user 0.25sys 0:02.08real  | 0.15user 0.37sys 0:00.28real
NO_PTHREADS | 1.57user 0.05sys 0:01.64real  | 0.09user 0.12sys 0:00.22real

Are the columns mixed up?

I conjecture that this is caused by contention on read_sha1_mutex.

Yeah, and I wonder why we need to have this lock in the first place. In theory, multiple readers shouldn't have to affect each other at all, right? The lock could be pushed down into read_sha1_file(), or a thread-safe variant of the function added.

In pratice, however, the code in sha1_file.c etc. scares me. ;-)

So disable threading entirely when not scanning the worktree, to get
the NO_PTHREADS performance in that case.  This obsoletes all code
related to grep_sha1_async.  The thread startup must be delayed until
after all arguments have been parsed, but this does not have a
measurable effect.

This is a bit radical. I think the underlying issue that read_sha1_file() is not thread-safe can be solved eventually and then we'd need to readd that code.

How about adding a parameter to control the number of threads (--threads?) instead that defaults to eight (or five) for the worktree and one for the rest? That would also make benchmarking easier.

René

PS: Patches one and three missed a signoff.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]