{"thread":{"id":"58684","subject":"Parallelism defaults and config options","startedAt":"2022-10-25T00:40:48Z","lastAt":"2022-10-25T19:17:11Z","messageCount":4,"participants":["Calvin Wan","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"465667","messageId":"CAFySSZAbsPuyPVX0+DQzArny2CEWs+GpQqJ3AOxUB_ffo8B3SQ@mail.gmail.com","threadId":"58684","inReplyTo":null,"subject":"Parallelism defaults and config options","fromName":"Calvin Wan","fromEmail":"calvinwan@google.com","sentAt":"2022-10-24T23:08:24Z","receivedAt":"2022-10-25T00:40:48Z","isPatch":false,"sender":{"key":"calvinwan@google.com","avatar":"https://avatars.githubusercontent.com/u/92547554?v=4"},"body":"While trying to figure out how I was going to set the default\nparallelism in cw/submodule-status-in-parallel, I noticed some\ndiscrepancies between how all the parallelism config options are set in\ngit. I wanted to discuss what we can do now to make them more consistent\nand also what the standard should be for the future. Here is a list of\nparallelism config options in git (let me know if I missed any) and how\nthey're set:\n\ngrep.threads: if unset or set to 0, git uses number of logical cores.\nindex.threads: if unset or set to 0/true, git uses number of logical\n  cores. If set to 1/false, multithreading is disabled\npack.threads: if unset or set to 0, git uses number of logical cores.\n  (documentation doesn't mention what default is)\ncheckout.workers: if unset, defaults to 1. If set to < 1, git uses\n  number of logical cores\nfetch.parallel: if unset, defaults to 1. If set to 0, git uses number of\n  logical cores (documentation says reasonable default)\nhttp.maxRequests: if unset, defaults to 5. If set to < 1, git uses the\n  default 5.\nsubmodule.fetchJobs: if unset, defaults to 1. If set to 0, git uses\n  number of logical cores (documentation says reasonable default)\n\nThe first inconsistency is the difference in language used to describe\nwhen each option is set to \"online_cpus()\". Some are explicit while\nothers omit it or use language such as \"reasonable default\". Being\nexplicit for all of the options is probably the easiest documentation\nfix.\n\nThe next inconsistency is for values < 1. Most options use online_cpus()\nwhen set to 0 except index.threads which is a special case of its own.\nSome options error out when set to a negative number while\ncheckout.workers falls back to online_cpus() and http.maxRequests falls\nback to 5. I don't think we can fix this retroactively unless we decide\nthat all config options will be set to online_cpus() if the value is\nnegative. Should that be the case going forward or should 0 be the only\nspecial cased value for now? I can see an argument for allowing other\nnegative values to be configured in the future for different defaulting\noptions.\n\nThe final inconsistency is how values are defaulted if unset. Some\ndefault to online_cpus() while others default to 1 (http.maxRequests is\n5). I want to call out grep.threads specifically here -- on my machine\nwith 48 cores, the default is actually SLOWER than using 1 thread. This\nis because the grep operation is heavily IO bound, so creating too many\nthreads adds overhead every time the read head changes. Interestingly,\nthis option runs optimally at 4 since that's how many PCIe lanes my SSD\nuses. While it makes sense to default processor heavy operations to\nonline_cpus(), does it make sense to do the same for IO heavy\noperations? (I wasn't able to find an equivalent of online_cpus() for\ndrive reading capabilities.) And what about operations that have a fair\nmix of each?\n\nThe safe option is to default to 1 process for many of these config\noptions, but we trade off in improving the experience for the average\nuser that is unaware of these options. If we're already defaulting to\nonline_cpus() for grep.threads and selecting 5 for http.maxRequests,\nthen why not do the same for other options? My suggestion would be\ndefaulting IO dominant operations to min(4, online_cpus()) since that\nseems like the standard number of lanes for people using SSDs. I would\nalso default operations that have a mix of both to\nmin(8, online_cpus()).\n-- \nCalvin Wan\n"},{"id":"465694","messageId":"221025.86sfjccjay.gmgdl@evledraar.gmail.com","threadId":"58684","inReplyTo":"CAFySSZAbsPuyPVX0+DQzArny2CEWs+GpQqJ3AOxUB_ffo8B3SQ@mail.gmail.com","subject":"Re: Parallelism defaults and config options","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-25T09:48:51Z","receivedAt":"2022-10-25T10:10:49Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Oct 24 2022, Calvin Wan wrote:\n\n> While trying to figure out how I was going to set the default\n> parallelism in cw/submodule-status-in-parallel, I noticed some\n> discrepancies between how all the parallelism config options are set in\n> git. I wanted to discuss what we can do now to make them more consistent\n> and also what the standard should be for the future. Here is a list of\n> parallelism config options in git (let me know if I missed any) and how\n> they're set:\n\nYeah, I've noticed this before & found it odd, thanks for trying to make\nit sensible.\n\n> grep.threads: if unset or set to 0, git uses number of logical cores.\n> index.threads: if unset or set to 0/true, git uses number of logical\n>   cores. If set to 1/false, multithreading is disabled\n> pack.threads: if unset or set to 0, git uses number of logical cores.\n>   (documentation doesn't mention what default is)\n> checkout.workers: if unset, defaults to 1. If set to < 1, git uses\n>   number of logical cores\n> fetch.parallel: if unset, defaults to 1. If set to 0, git uses number of\n>   logical cores (documentation says reasonable default)\n> http.maxRequests: if unset, defaults to 5. If set to < 1, git uses the\n>   default 5.\n> submodule.fetchJobs: if unset, defaults to 1. If set to 0, git uses\n>   number of logical cores (documentation says reasonable default)\n\nAside from anything we come up with, I think it's fine to change the\ndefaults, i.e. to make some of these parallel by default where they\naren't now.\n\n> The first inconsistency is the difference in language used to describe\n> when each option is set to \"online_cpus()\". Some are explicit while\n> others omit it or use language such as \"reasonable default\". Being\n> explicit for all of the options is probably the easiest documentation\n> fix.\n\nIdeally I think a documentation fix would be to just have these point at\nthe same text, e.g. a \"parent\" option could explain all the details, and\nthey could each point to that.\n\n> The next inconsistency is for values < 1. Most options use online_cpus()\n> when set to 0 except index.threads which is a special case of its own.\n> Some options error out when set to a negative number while\n> checkout.workers falls back to online_cpus() and http.maxRequests falls\n> back to 5. I don't think we can fix this retroactively unless we decide\n> that all config options will be set to online_cpus() if the value is\n> negative. Should that be the case going forward or should 0 be the only\n> special cased value for now? I can see an argument for allowing other\n> negative values to be configured in the future for different defaulting\n> options.\n\nWe also have other options, like warning() on negative values if we\nthink it's worth making these consistent while slightly annoying\nexisting users.\n\nI think it's safe to say that by far the majority of git users aren't\ntweaking these at all, so we have a lot of leeway for sensible changes.\n\n> The final inconsistency is how values are defaulted if unset. Some\n> default to online_cpus() while others default to 1 (http.maxRequests is\n> 5). I want to call out grep.threads specifically here -- on my machine\n> with 48 cores, the default is actually SLOWER than using 1 thread. This\n> is because the grep operation is heavily IO bound, so creating too many\n> threads adds overhead every time the read head changes. Interestingly,\n> this option runs optimally at 4 since that's how many PCIe lanes my SSD\n> uses. While it makes sense to default processor heavy operations to\n> online_cpus(), does it make sense to do the same for IO heavy\n> operations? (I wasn't able to find an equivalent of online_cpus() for\n> drive reading capabilities.) And what about operations that have a fair\n> mix of each?\n\nFirst, http.maxRequests is different from any of the rest, it'll impact\n3rd party servers. So we should treat it differently. I'd think unless\nwe have solid evidence for something else we should just leave that one\nbe, and certainly not have it be some function of online_cpus().\n\n> The safe option is to default to 1 process for many of these config\n> options, but we trade off in improving the experience for the average\n> user that is unaware of these options. If we're already defaulting to\n> online_cpus() for grep.threads and selecting 5 for http.maxRequests,\n> then why not do the same for other options? My suggestion would be\n> defaulting IO dominant operations to min(4, online_cpus()) since that\n> seems like the standard number of lanes for people using SSDs. I would\n> also default operations that have a mix of both to\n> min(8, online_cpus()).\n\nI haven't thought/tested what the defaults *should* be, but I think it's\na fair assumption that the current defaults were probably picked on the\nbasis of a few ad-hoc tests on some person's laptop :)\n\nI.e. the 48 core case you mention etc. is likely to be untested & wasn't\nthought of at the time.\n\nI think *structurally* the best approach is something like having a\nfamily of config variables like:\n\t\n\tcore.jobs: [(false | 1)|(0 | true) | [2..Inf] ]\n\tcore.jobs.IOBound: [(false | 1)|(0 | true) | [2..Inf]]\n\tcore.jobs.CPUBound: [(false | 1)|(0 | true) | [2..Inf]]\n\nNote that it's \"0 or true\" and \"1 or false\", not a mistake, i.e. that\nmatches our current defaults. You'd set it to \"true\" to get the \"yes, I\nwant it parallel\" setting.\n\nWe'd have these take priority from each other, so \"grep.threads\" would\noverride \"core.jobs.IOBound\", which in turn would override \"core.jobs\".\n\nThe common case would be that you wouldn't set either \"core.jobs\" or\n\"grep.threads\", so we'd default to \"core.jobs.IOBound\", which we'd set\nto some sensible default.\n\nAll of that's off-the-cuff, but hopefully has some worthwhile\nfeedback. In particular I think leaving our docs in a state where we\ndon't have to repeat ourselves (sometimes inaccurately) would be nice,\nwhether it's via includes or because we point to a \"parent\" variable.\n\n\n\t\n\n\n"},{"id":"465721","messageId":"CAFySSZBnuszT0iYdFThRzktBuMaCTfGCTz5nbhK6sbrt=QL+5w@mail.gmail.com","threadId":"58684","inReplyTo":"221025.86sfjccjay.gmgdl@evledraar.gmail.com","subject":"Re: Parallelism defaults and config options","fromName":"Calvin Wan","fromEmail":"calvinwan@google.com","sentAt":"2022-10-25T18:01:21Z","receivedAt":"2022-10-25T18:01:47Z","isPatch":false,"sender":{"key":"calvinwan@google.com","avatar":"https://avatars.githubusercontent.com/u/92547554?v=4"},"body":"> > The next inconsistency is for values < 1. Most options use online_cpus()\n> > when set to 0 except index.threads which is a special case of its own.\n> > Some options error out when set to a negative number while\n> > checkout.workers falls back to online_cpus() and http.maxRequests falls\n> > back to 5. I don't think we can fix this retroactively unless we decide\n> > that all config options will be set to online_cpus() if the value is\n> > negative. Should that be the case going forward or should 0 be the only\n> > special cased value for now? I can see an argument for allowing other\n> > negative values to be configured in the future for different defaulting\n> > options.\n>\n> We also have other options, like warning() on negative values if we\n> think it's worth making these consistent while slightly annoying\n> existing users.\n>\n> I think it's safe to say that by far the majority of git users aren't\n> tweaking these at all, so we have a lot of leeway for sensible changes.\n\nI like the warning() idea -- it doesn't break people's git and sets the correct\nprecedence for what to expect for similar parallelism config options.\n\n> > The final inconsistency is how values are defaulted if unset. Some\n> > default to online_cpus() while others default to 1 (http.maxRequests is\n> > 5). I want to call out grep.threads specifically here -- on my machine\n> > with 48 cores, the default is actually SLOWER than using 1 thread. This\n> > is because the grep operation is heavily IO bound, so creating too many\n> > threads adds overhead every time the read head changes. Interestingly,\n> > this option runs optimally at 4 since that's how many PCIe lanes my SSD\n> > uses. While it makes sense to default processor heavy operations to\n> > online_cpus(), does it make sense to do the same for IO heavy\n> > operations? (I wasn't able to find an equivalent of online_cpus() for\n> > drive reading capabilities.) And what about operations that have a fair\n> > mix of each?\n>\n> First, http.maxRequests is different from any of the rest, it'll impact\n> 3rd party servers. So we should treat it differently. I'd think unless\n> we have solid evidence for something else we should just leave that one\n> be, and certainly not have it be some function of online_cpus().\n\nI wasn't planning on touching that one either -- mostly brought it up as an\nexample of an arbitrarily picked default.\n\n> > The safe option is to default to 1 process for many of these config\n> > options, but we trade off in improving the experience for the average\n> > user that is unaware of these options. If we're already defaulting to\n> > online_cpus() for grep.threads and selecting 5 for http.maxRequests,\n> > then why not do the same for other options? My suggestion would be\n> > defaulting IO dominant operations to min(4, online_cpus()) since that\n> > seems like the standard number of lanes for people using SSDs. I would\n> > also default operations that have a mix of both to\n> > min(8, online_cpus()).\n>\n> I haven't thought/tested what the defaults *should* be, but I think it's\n> a fair assumption that the current defaults were probably picked on the\n> basis of a few ad-hoc tests on some person's laptop :)\n>\n> I.e. the 48 core case you mention etc. is likely to be untested & wasn't\n> thought of at the time.\n\nEven with 8 threads, git grep runs very slightly slower than with 1 thread\nfor me. Unless we have something along the lines of \"git setup-parallelism\",\nany default we pick will have different outcomes for different users, but I\nthink we can at least make a better guess than what we currently have.\n\n> I think *structurally* the best approach is something like having a\n> family of config variables like:\n>\n>         core.jobs: [(false | 1)|(0 | true) | [2..Inf] ]\n>         core.jobs.IOBound: [(false | 1)|(0 | true) | [2..Inf]]\n>         core.jobs.CPUBound: [(false | 1)|(0 | true) | [2..Inf]]\n>\n> Note that it's \"0 or true\" and \"1 or false\", not a mistake, i.e. that\n> matches our current defaults. You'd set it to \"true\" to get the \"yes, I\n> want it parallel\" setting.\n>\n> We'd have these take priority from each other, so \"grep.threads\" would\n> override \"core.jobs.IOBound\", which in turn would override \"core.jobs\".\n>\n> The common case would be that you wouldn't set either \"core.jobs\" or\n> \"grep.threads\", so we'd default to \"core.jobs.IOBound\", which we'd set\n> to some sensible default.\n\nWhile I like this concept very much, my worry is that some commands\nmight not fall nicely into IOBound or CPUBound. If they're a mix of the two\nor bound by possibly something else (like network for fetch.parallel? not\nsure about this one haven't looked too into it), then what bucket would we\nput them under?\n"},{"id":"465726","messageId":"221025.86k04nd89c.gmgdl@evledraar.gmail.com","threadId":"58684","inReplyTo":"CAFySSZBnuszT0iYdFThRzktBuMaCTfGCTz5nbhK6sbrt=QL+5w@mail.gmail.com","subject":"Re: Parallelism defaults and config options","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-25T18:47:45Z","receivedAt":"2022-10-25T19:17:11Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 25 2022, Calvin Wan wrote:\n>> > The safe option is to default to 1 process for many of these config\n>> > options, but we trade off in improving the experience for the average\n>> > user that is unaware of these options. If we're already defaulting to\n>> > online_cpus() for grep.threads and selecting 5 for http.maxRequests,\n>> > then why not do the same for other options? My suggestion would be\n>> > defaulting IO dominant operations to min(4, online_cpus()) since that\n>> > seems like the standard number of lanes for people using SSDs. I would\n>> > also default operations that have a mix of both to\n>> > min(8, online_cpus()).\n>>\n>> I haven't thought/tested what the defaults *should* be, but I think it's\n>> a fair assumption that the current defaults were probably picked on the\n>> basis of a few ad-hoc tests on some person's laptop :)\n>>\n>> I.e. the 48 core case you mention etc. is likely to be untested & wasn't\n>> thought of at the time.\n>\n> Even with 8 threads, git grep runs very slightly slower than with 1 thread\n> for me. Unless we have something along the lines of \"git setup-parallelism\",\n> any default we pick will have different outcomes for different users, but I\n> think we can at least make a better guess than what we currently have.\n\nFor me e.g.:\n\n\thyperfine -L f E,P -L n 0,1,2,3,4,5,6,7,8,9,10 'git -P -c grep.threads={n} grep -{f} a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\nGets this in git.git, this is on a 8 core box:\n\t\n\tSummary\n\t  'git -P -c grep.threads=4 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa' ran\n\t    1.01 ± 0.11 times faster than 'git -P -c grep.threads=3 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.03 ± 0.16 times faster than 'git -P -c grep.threads=5 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.14 ± 0.14 times faster than 'git -P -c grep.threads=6 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.22 ± 0.12 times faster than 'git -P -c grep.threads=7 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.29 ± 0.13 times faster than 'git -P -c grep.threads=0 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.29 ± 0.13 times faster than 'git -P -c grep.threads=8 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.31 ± 0.13 times faster than 'git -P -c grep.threads=2 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.33 ± 0.13 times faster than 'git -P -c grep.threads=9 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.40 ± 0.15 times faster than 'git -P -c grep.threads=10 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.49 ± 0.15 times faster than 'git -P -c grep.threads=6 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.50 ± 0.15 times faster than 'git -P -c grep.threads=7 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.57 ± 0.16 times faster than 'git -P -c grep.threads=0 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.59 ± 0.17 times faster than 'git -P -c grep.threads=8 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.59 ± 0.19 times faster than 'git -P -c grep.threads=5 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.64 ± 0.17 times faster than 'git -P -c grep.threads=9 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.66 ± 0.20 times faster than 'git -P -c grep.threads=4 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.73 ± 0.18 times faster than 'git -P -c grep.threads=10 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    1.94 ± 0.28 times faster than 'git -P -c grep.threads=3 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    2.15 ± 0.21 times faster than 'git -P -c grep.threads=1 grep -P a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    2.59 ± 0.25 times faster than 'git -P -c grep.threads=2 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\t    4.70 ± 0.46 times faster than 'git -P -c grep.threads=1 grep -E a?a?a?a?a?a?a?a?a?a?aaaaaaaaaa'\n\nYMMV.\n\nI think there's been some past discussion about having a probing command\nfigure this out for you, longer term that would be neat.\n\n>> I think *structurally* the best approach is something like having a\n>> family of config variables like:\n>>\n>>         core.jobs: [(false | 1)|(0 | true) | [2..Inf] ]\n>>         core.jobs.IOBound: [(false | 1)|(0 | true) | [2..Inf]]\n>>         core.jobs.CPUBound: [(false | 1)|(0 | true) | [2..Inf]]\n>>\n>> Note that it's \"0 or true\" and \"1 or false\", not a mistake, i.e. that\n>> matches our current defaults. You'd set it to \"true\" to get the \"yes, I\n>> want it parallel\" setting.\n>>\n>> We'd have these take priority from each other, so \"grep.threads\" would\n>> override \"core.jobs.IOBound\", which in turn would override \"core.jobs\".\n>>\n>> The common case would be that you wouldn't set either \"core.jobs\" or\n>> \"grep.threads\", so we'd default to \"core.jobs.IOBound\", which we'd set\n>> to some sensible default.\n>\n> While I like this concept very much, my worry is that some commands\n> might not fall nicely into IOBound or CPUBound. If they're a mix of the two\n> or bound by possibly something else (like network for fetch.parallel? not\n> sure about this one haven't looked too into it), then what bucket would we\n> put them under?\n\nJust have them use the top-level config if there's no \"intermediate\".\n\nOr maybe everything should go straight to the top-level default.\n\nOr maybe there shouldn't be a \"top-level\" at all, and nobody wants to\nconfigure these N at at time, I don't know.\n\nI was just trying to offer you a way out of the problem of wanting\ndifferent defaults for certain variables, we could \"hardcode\" them\nacross the different variables, or if we find commonalities we could\nhave them fall back to another parent config variable.\n"}]}