Make gclient sync use cpu count for default jobs

Make gclient sync use cpu count for default jobs

- On high core, fast machines, jobs=8 is does not offer good
  parallelization.  Switch to use number of local cpus.

R=maruel@chromium.org
BUG=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=162072

[M-A Ruel, 2012-10-15 15:07:58]

I don't think that in the general case, the number of cores gives a good
approximation of the number of parallel fetches that should be done;

In the subversion case, the fetch is highly bound by disk I/O, that least on
magnetic disks. CPU count is barely related. And most of our slaves uses 4 cores
but have very fast local disk I/O and network I/O, so they would use less
parallel checkout with this CL, which seems inefficient. On SSD, it's probably
bound by the server's throughput, since is more or less single-homed.

In the git case, the fetch is differently bound because of the different phases
in which a git fetch/git clone passes through. Also, git naturally lends itself
to be multi-homed. But overall, most of the time, it is primarily network I/O
bound. AFAIK git already uses [number of cores] threads so git is already using
all the available CPU processing power available. So increasing this number
would have negative benefit memory wise.

So overall, I think it rarely gets to the point where the number of CPUs is a
good metric for the --jobs argument.

Did you cold and warm sync performance analysis?

[Isaac (use chromium), 2012-10-15 16:27:33]

It definitely speeds things up for git; the basis of the change was that the
recommended jobs for repo sync is:
-j <cores> for clean sync
-j 100 for incremental sync

I'll did up some benchmarks.  I'd be pretty surprised if subversion sync was
negatively affected here. However in the case of the bots, since we know they
have fast network io and are generally small incremental updates, we could pass
-j <high> in sync step. How did you decide on -j8 as default?

Isaac

On Oct 15, 2012, at 8:07 AM, maruel@chromium.org wrote:

> I don't think that in the general case, the number of cores gives a good
> approximation of the number of parallel fetches that should be done;
> 
> In the subversion case, the fetch is highly bound by disk I/O, that least on
> magnetic disks. CPU count is barely related. And most of our slaves uses 4
cores
> but have very fast local disk I/O and network I/O, so they would use less
> parallel checkout with this CL, which seems inefficient. On SSD, it's
probably
> bound by the server's throughput, since is more or less single-homed.
> 
> In the git case, the fetch is differently bound because of the different
phases
> in which a git fetch/git clone passes through. Also, git naturally lends
itself
> to be multi-homed. But overall, most of the time, it is primarily network I/O
> bound. AFAIK git already uses [number of cores] threads so git is already
using
> all the available CPU processing power available. So increasing this number
> would have negative benefit memory wise.
> 
> So overall, I think it rarely gets to the point where the number of CPUs is a
> good metric for the --jobs argument.
> 
> Did you cold and warm sync performance analysis?
> 
> https://codereview.chromium.org/11140019/

[M-A Ruel, 2012-10-15 17:03:02]

On 2012/10/15 16:27:33, Isaac (use chromium acct) wrote:
> It definitely speeds things up for git; the basis of the change was that the
> recommended jobs for repo sync is:
> -j <cores> for clean sync
> -j 100 for incremental sync
> 
> I'll did up some benchmarks.  I'd be pretty surprised if subversion sync was
> negatively affected here. However in the case of the bots, since we know they
> have fast network io and are generally small incremental updates, we could
pass
> -j <high> in sync step. How did you decide on -j8 as default?

At the time I had benchmarked on svn on Windows at 4, 6, 8, 16.

svn is totally disk I/O bound when there's a large number of files on Windows.
It's due to the way it stores the file: it stores two files for each files in
the checkout. For linux, the story is significantly different since stat() is
faster. 

For incremental sync with git, I see it as being bottlenecked by connection
time. Note that for git repositories, no network access is done for a DEPS which
already has the relevant git commit so overall it's pretty fast already for
small incremental updates. In practice, people are mostly bottlenecked by
Webkit.git so increasing --jobs to an arbitrarily high value won't help that
much; no other dependency is waiting on Webkit.git to be done. But on the other
hand, you'd kill performance for someone doing a sync on a multi-core
workstation with magnetic disk on Windows with subversion with some AV getting
in the way.

So just to clarify, *I'm totally fine with increasing the limit*. I just don't
want it to be slower for some use cases, like in particular the one all the
Chromium Try Server slaves are using! My gut feeling is that correlation with
amount of physical RAM and/or OS would be better. Also, I'm fine to change the
default if .DEPS.git/only git dependencies are detected or some other signals
like that.

[Isaac (away), 2012-10-16 01:12:33]

Hi M.A.?  It wasn't clear to me if you thought
-j=max(8, NumCpus()) could actually be harmful for the bots.  If not, I'd
suggest we use NumCpus instead of NumCpus/2.  For incremental syncs it will be
the same thing, because gclient won't need to fetch more than 16 jobs at once
anyway.  But for full syncs, especially syncs onto an ssd it could improve sync
speed.

ptal!

Cheers,
Isaac

[M-A Ruel, 2012-10-16 01:20:18]

lgtm

https://chromiumcodereview.appspot.com/11140019/diff/2002/gclient_utils.py
File gclient_utils.py (right):

https://chromiumcodereview.appspot.com/11140019/diff/2002/gclient_utils.py#ne...
gclient_utils.py:785: return int(os.sysconf('SC_NPROCESSORS_ONLN'))
You want to # pylint: disable=E1101 here on Windows.

[Isaac (away), 2012-10-16 04:23:32]

committing

https://chromiumcodereview.appspot.com/11140019/diff/2002/gclient_utils.py
File gclient_utils.py (right):

https://chromiumcodereview.appspot.com/11140019/diff/2002/gclient_utils.py#ne...
gclient_utils.py:785: return int(os.sysconf('SC_NPROCESSORS_ONLN'))
On 2012/10/16 01:20:18, Marc-Antoine Ruel wrote:
> You want to # pylint: disable=E1101 here on Windows.

Done, but I cribbed this function from swarm_client/run_test_cases.py.  Does
that also need this pylint disable?

[commit-bot: I haz the power, 2012-10-16 04:24:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ilevy@chromium.org/11140019/3002

[commit-bot: I haz the power, 2012-10-16 04:26:39]

Change committed as 162072

[M-A Ruel, 2012-10-16 11:18:21]

https://chromiumcodereview.appspot.com/11140019/diff/2002/gclient_utils.py
File gclient_utils.py (right):

https://chromiumcodereview.appspot.com/11140019/diff/2002/gclient_utils.py#ne...
gclient_utils.py:785: return int(os.sysconf('SC_NPROCESSORS_ONLN'))
On 2012/10/16 04:23:32, Isaac wrote:
> On 2012/10/16 01:20:18, Marc-Antoine Ruel wrote:
> > You want to # pylint: disable=E1101 here on Windows.
> 
> Done, but I cribbed this function from swarm_client/run_test_cases.py.  Does
> that also need this pylint disable?

I fixed it there a few hours ago. :) I rarely run the presubmit checks on
Windows, only ~once a month.