Issue metadata
Sign in to add a comment
|
fetching kernel changes from gerrit (GoB) is slow |
||||||||||||||||||||||||
Issue descriptionI'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...
,
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
,
May 25 2017
,
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
,
May 26 2017
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?
,
May 26 2017
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.
,
May 26 2017
@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?
,
May 28 2018
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
,
May 29 2018
,
May 29 2018
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 |
|||||||||||||||||||||||||
Comment 1 by davidri...@chromium.org
, May 24 2017