New issue
Advanced search Search tips

Issue 725220 link

Starred by 2 users

Issue metadata

Status: Fixed
Merged: issue 725233
Owner: ----
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

fetching kernel changes from gerrit (GoB) is slow

Project Member Reported by diand...@chromium.org, May 22 2017

Issue description

I'm not sure if there's anything we can do about this, but filing in case there is...

===

Basically the issue is that cherry-picking changes to the kernel from gerrit is slow.  

For instance, if I got to:

https://chromium-review.googlesource.com/c/508834/

...and I go to "Download" and get the cherry-pick text, I see:

  git fetch https://chromium.googlesource.com/chromiumos/third_party/kernel refs/changes/34/508834/1 && git cherry-pick FETCH_HEAD

---

Running this takes 18 seconds:

$ time git fetch https://chromium.googlesource.com/chromiumos/third_party/kernel refs/changes/34/508834/1
remote: Counting objects: 58060, done
remote: Finding sources: 100% (4/4)
remote: Total 4 (delta 0), reused 2 (delta 0)
Unpacking objects: 100% (4/4), done.
From https://chromium.googlesource.com/chromiumos/third_party/kernel
 * branch                      refs/changes/34/508834/1 -> FETCH_HEAD

real    0m18.625s
user    0m5.572s
sys     0m6.455s

---

OTOH, running this is much faster:

$ time curl https://chromium-review.googlesource.com/changes/508834/revisions/1/patch?download > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  2792    0  2792    0     0  15931      0 --:--:-- --:--:-- --:--:-- 15954

real    0m1.829s
user    0m0.012s
sys     0m0.006s

---

It appears that fetching the same change a 2nd time is a bit faster, but still not quite as fast.  Re-running the two commands above:

$ time git fetch https://chromium.googlesource.com/chromiumos/third_party/kernel refs/changes/34/508834/1
From https://chromium.googlesource.com/chromiumos/third_party/kernel
 * branch                      refs/changes/34/508834/1 -> FETCH_HEAD

real    0m3.982s
user    0m1.249s
sys     0m0.324s

$ time curl https://chromium-review.googlesource.com/changes/508834/revisions/1/patch?download > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  2792    0  2792    0     0  10801      0 --:--:-- --:--:-- --:--:-- 10821

real    0m0.273s
user    0m0.011s
sys     0m0.011s

===

This speed (plus the stupid way that our builder cherry-picks changes) is likely the root cause of the slow Commit Queue Sync of guado moblab build 5814.  A small snippet from there:

[patch_branch 0b095aa7129c] UPSTREAM: Bluetooth: Introduce helper functions for socket cookie handling
 Author: Marcel Holtmann <marcel@holtmann.org>
 Date: Tue Aug 30 05:00:34 2016 +0200
 1 file changed, 29 insertions(+), 12 deletions(-)
13:56:54: INFO: RunCommand: git fetch -f https://chromium-review.googlesource.com/chromiumos/third_party/kernel refs/changes/14/475414/7 in /b/cbuild/repository/src/third_party/kernel/v3.14
13:57:14: INFO: Attempting to cherry-pick change chromiumos/third_party/kernel:refs/changes/14/475414/7:ca8d1056 "UPSTREAM: Bluetooth: Use numbers for subsystem version string"
Trying simple merge.

[patch_branch f878a33be370] UPSTREAM: Bluetooth: Use numbers for subsystem version string
 Author: Marcel Holtmann <marcel@holtmann.org>
 Date: Tue Aug 30 05:00:35 2016 +0200
 3 files changed, 11 insertions(+), 6 deletions(-)
13:57:16: INFO: RunCommand: git fetch -f https://chromium-review.googlesource.com/chromiumos/third_party/kernel refs/changes/15/475415/7 in /b/cbuild/repository/src/third_party/kernel/v3.14
13:57:36: INFO: Attempting to cherry-pick change chromiumos/third_party/kernel:refs/changes/15/475415/7:d71f71b2 "UPSTREAM: Bluetooth: Send control open and close only when cookie is present"
Trying simple merge.

[patch_branch 9e8267473a35] UPSTREAM: Bluetooth: Send control open and close only when cookie is present
 Author: Marcel Holtmann <marcel@holtmann.org>
 Date: Tue Aug 30 05:00:36 2016 +0200
 1 file changed, 16 insertions(+), 2 deletions(-)
13:57:39: INFO: RunCommand: git fetch -f https://chromium-review.googlesource.com/chromiumos/third_party/kernel refs/changes/16/475416/7 in /b/cbuild/repository/src/third_party/kernel/v3.14
13:58:00: INFO: Attempting to cherry-pick change chromiumos/third_party/kernel:refs/changes/16/475416/7:b3c05b98 "UPSTREAM: Bluetooth: Assign the channel early when binding HCI sockets"
Trying simple merge.

[patch_branch ba8845c2e927] UPSTREAM: Bluetooth: Assign the channel early when binding HCI sockets
 Author: Marcel Holtmann <marcel@holtmann.org>
 Date: Tue Aug 30 05:00:37 2016 +0200
 1 file changed, 11 insertions(+), 5 deletions(-)
13:58:02: INFO: RunCommand: git fetch -f https://chromium-review.googlesource.com/chromiumos/third_party/kernel refs/changes/17/475417/7 in /b/cbuild/repository/src/third_party/kernel/v3.14
13:58:24: INFO: Attempting to cherry-pick change chromiumos/third_party/kernel:refs/changes/17/475417/7:123eeba3 "UPSTREAM: Bluetooth: Add extra channel checks for control open/close messages"
Trying simple merge.

[patch_branch b9140edf5125] UPSTREAM: Bluetooth: Add extra channel checks for control open/close messages
 Author: Marcel Holtmann <marcel@holtmann.org>
 Date: Tue Aug 30 05:00:38 2016 +0200
 1 file changed, 19 insertions(+), 5 deletions(-)
13:58:26: INFO: RunCommand: git fetch -f https://chromium-review.googlesource.com/chromiumos/third_party/kernel refs/changes/18/475418/7 in /b/cbuild/repository/src/third_party/kernel/v3.14
13:58:46: INFO: Attempting to cherry-pick change chromiumos/third_party/kernel:refs/changes/18/475418/7:44a2123c "UPSTREAM: Bluetooth: Send control open and close messages for HCI raw sockets"
Trying simple merge.


That run has 106 kernel changes to pick (plus some other changes).  At ~23 seconds a change it means we spent ~39 minutes patching in kernel CLs.  Checking against reality:

13:54:18 - start of patching in changes (not just kernel changes).
14:44:54 - end of patching in changes (not just kernel changes).

...so that's about 50 minutes.

===

I'll file a separate bug about making our build bot smarter (there are some tricks we can do) and put a reference here...

 
Blockedon: 725968
Project Member

Comment 2 by bugdroid1@chromium.org, May 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/78ea03cd8699bd6fda622fb9f8ea8a09df53dfec

commit 78ea03cd8699bd6fda622fb9f8ea8a09df53dfec
Author: David Riley <davidriley@chromium.org>
Date: Thu May 25 12:55:29 2017

validation_pool: Pre-fetch patches when syncing slaves.

BUG= chromium:725220 
BUG= chromium:725233 
TEST=validation_pool_unittest; run_tests

Change-Id: I7be8749aaf018f69c75ad0bd6f9f58108dd585ed
Reviewed-on: https://chromium-review.googlesource.com/513397
Tested-by: David Riley <davidriley@chromium.org>
Trybot-Ready: David Riley <davidriley@chromium.org>
Reviewed-by: David James <davidjames@chromium.org>

[modify] https://crrev.com/78ea03cd8699bd6fda622fb9f8ea8a09df53dfec/cbuildbot/patch_series.py
[modify] https://crrev.com/78ea03cd8699bd6fda622fb9f8ea8a09df53dfec/cbuildbot/validation_pool.py
[modify] https://crrev.com/78ea03cd8699bd6fda622fb9f8ea8a09df53dfec/cbuildbot/validation_pool_unittest.py

Mergedinto: 725233
Status: Duplicate (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, May 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/5c15b33f5f051da0715f3d769692b28ad6bd26c9

commit 5c15b33f5f051da0715f3d769692b28ad6bd26c9
Author: David Riley <davidriley@chromium.org>
Date: Fri May 26 00:16:02 2017

Revert "Put sleep between apply_change operations."

This reverts commit 250bc41528aa5f90b8aadab998ad4818ddb97499.

By pre-fetching the sleep hopefully isn't necessary.

BUG= chromium:725233 
BUG= chromium:725220 

Change-Id: Ibb95ca96daa6fd66d53de7553e60f600b8f4700c
Reviewed-on: https://chromium-review.googlesource.com/513421
Commit-Ready: David Riley <davidriley@chromium.org>
Tested-by: David Riley <davidriley@chromium.org>
Reviewed-by: Aviv Keshet <akeshet@chromium.org>

[modify] https://crrev.com/5c15b33f5f051da0715f3d769692b28ad6bd26c9/cbuildbot/validation_pool.py

Cc: akes...@chromium.org davidri...@chromium.org
Status: Available (was: Duplicate)
I'm confused about why this was marked as a duplicate.  While this is related to  bug #725233 , it's not the same.

* This bug is about making gerrit behave faster in general, if possible.  It would be useful for individual developers who might want to pick changes.

* The other bug is about optimizing cbuildbot to work around any gerrit slowness that we can't avoid.

It's unclear why this change here would be blocked on bug #725968, but maybe I don't understand?
Well I guess I was confused because it's talking about gerrit doing cherry-picking, but there isn't any gerrit cherry-picking here.

There are some GoB operations with the git fetches, and then some local git cherry-pick operations, and then some sleep(1)s.  Are you meaning that this is to improve GoB fetch operations?

I blocked it on issue 725968 because one of the comments I had when it came to removing the sleep(1) was that it was related to being out of quota.
Blockedon: -725968
Summary: fetching kernel changes from gerrit (GoB) is slow (was: cherry-picking kernel changes from gerrit is slow)
@6: Ah, good point.  I updated the title.

For me, I often go to a gerrit patch in the web UI and click the "Download", then copy paste the "cherry-pick" command.  Thus, I think of this whole process as "cherry picking from gerrit".  ...but, as you said, the "fetch" is the part that's actually slow.  I knew it was the fetch part (which is why I used buildbot as more examples), but I didn't update my terminology in the title of the bug.

Anyway, fixed now.


Note that I don't feel like it's an impossible task to make fetch from gerrit faster.  Why do I say this?

* Historically, fetches got slower the more crud I had in my local repo.  AKA: doing a "git gc" sped up fetches.  I haven't confirmed this recently, but to me it seemed like local crud shouldn't be making fetches from the server slower.  That implied there would be a way to optimize it out.

* Fetches for the kernel are much slower than from other repos.  This means there's not some fixed cost with the fetch but instead the slowness has to do with the sheer amount of crud in the kernel.  It seems like perhaps all the work associated with the kernel crud could be cached at least somewhat?
Project Member

Comment 8 by sheriffbot@chromium.org, May 28 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: -akes...@chromium.org
Components: Infra>Client>ChromeOS>CI
Status: Fixed (was: Untriaged)
Gerrit/GoB are focusing on performance so I think we'll see performance improvements if we haven't already. Please reopen if you're seeing new performance regressions.

Sign in to add a comment