git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date
Dec 2, 2011, 16:15 UTC
Message-ID
<4ED8F9AE.8030605@lsrfire.ath.cx>
In-Reply-To
<5328add8b32f83b4cdbd2e66283f77c125ec127a.1322830368.git.trast@student.ethz.ch>
Am 02.12.2011 14:07, schrieb Thomas Rast:
Show 11 quoted lines
> 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. ;-)
Show 5 quoted lines
> 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.
Previous: Thomas RastNext: Thomas Rast
Message 5 of 35 in “grep multithreading and scaling”
  1. 0/3 grep multithreading and scalingThomas Rast, Dec 2, 2011
  2. 1/3 grep: load funcname patterns for -WThomas Rast, Dec 2, 2011
  3. 2/3 grep: enable threading with -p and -W using lazy attribute lookupThomas Rast, Dec 2, 2011
  4. 3/3 grep: disable threading in all but worktree caseThomas Rast, Dec 2, 2011
  5. René ScharfeDec 2, 2011
  6. Thomas RastDec 5, 2011
  7. René ScharfeDec 6, 2011
  8. 4/2 grep: turn off threading for non-worktreeRené Scharfe, Dec 6, 2011
  9. Jeff KingDec 7, 2011
  10. René ScharfeDec 7, 2011
  11. Jeff KingDec 7, 2011
  12. J. Bruce FieldsDec 7, 2011
  13. Jeff KingDec 7, 2011
  14. Thomas RastDec 7, 2011
  15. René ScharfeDec 7, 2011
  16. Pete WyckoffDec 10, 2011
  17. René ScharfeDec 12, 2011
  18. Jeff KingDec 7, 2011
  19. René ScharfeDec 7, 2011
  20. Jeff KingDec 7, 2011
  21. Thomas RastDec 7, 2011
  22. René ScharfeDec 7, 2011
  23. Ævar Arnfjörð BjarmasonDec 23, 2011
  24. Thomas RastDec 23, 2011
  25. Ævar Arnfjörð BjarmasonDec 24, 2011
  26. Jeff KingDec 24, 2011
  27. Nguyen Thai Ngoc DuyDec 24, 2011
  28. Nguyen Thai Ngoc DuyDec 24, 2011
  29. Jeff KingDec 24, 2011
  30. Nguyen Thai Ngoc DuyDec 25, 2011
  31. Jeff KingDec 2, 2011
  32. Thomas RastDec 5, 2011
  33. Thomas RastDec 5, 2011
  34. Jeff KingDec 6, 2011
  35. Eric HermanDec 2, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.