Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(354)

Issue 2291273005: Add counting the number of CPU's in goma module (Closed)

Created:
4 years, 3 months ago by tikuta
Modified:
4 years, 3 months ago
CC:
chromium-reviews, infra-reviews+build_chromium.org, kjellander-cc_chromium.org
Target Ref:
refs/heads/master
Project:
build
Visibility:
Public.

Description

Add counting the number of CPU's in goma module This will be used to set -j option when running build system (e.g. make, ninja, scons). After landing this, I will set -j of chromeoffice buildbots using this function. BUG=544330 Committed: https://chromium.googlesource.com/chromium/tools/build/+/11785fb7f116f51bb6442bbbe5b4096055448e57

Patch Set 1 #

Total comments: 6

Patch Set 2 : catch exception #

Patch Set 3 : use 80 for linux #

Total comments: 4

Patch Set 4 : add docstring #

Patch Set 5 : stop to calc in ensure_goma #

Total comments: 4

Patch Set 6 : fix comment #

Total comments: 2

Patch Set 7 : use job_limit in inline #

Total comments: 2

Patch Set 8 : add comment why I use python.inline #

Unified diffs Side-by-side diffs Delta from patch set Stats (+293 lines, -1 line) Patch
M scripts/slave/recipe_modules/goma/api.py View 1 2 3 4 5 6 7 2 chunks +41 lines, -0 lines 0 comments Download
M scripts/slave/recipe_modules/goma/example.py View 1 2 3 4 1 chunk +6 lines, -1 line 0 comments Download
M scripts/slave/recipe_modules/goma/example.expected/linux.json View 1 2 3 4 5 6 1 chunk +41 lines, -0 lines 0 comments Download
M scripts/slave/recipe_modules/goma/example.expected/linux_upload_logs.json View 1 2 3 4 5 6 1 chunk +41 lines, -0 lines 0 comments Download
M scripts/slave/recipe_modules/goma/example.expected/mac.json View 1 2 3 4 5 6 1 chunk +41 lines, -0 lines 0 comments Download
M scripts/slave/recipe_modules/goma/example.expected/mac_upload_logs.json View 1 2 3 4 5 6 1 chunk +41 lines, -0 lines 0 comments Download
M scripts/slave/recipe_modules/goma/example.expected/win.json View 1 2 3 4 5 6 1 chunk +41 lines, -0 lines 0 comments Download
M scripts/slave/recipe_modules/goma/example.expected/win_upload_logs.json View 1 2 3 4 5 6 1 chunk +41 lines, -0 lines 0 comments Download

Messages

Total messages: 32 (13 generated)
tikuta
goma/api.py is most important file.
4 years, 3 months ago (2016-09-01 09:16:24 UTC) #2
tikuta
On 2016/09/01 09:16:24, tikuta wrote: > goma/api.py is most important file. Also see goma/example.py, please.
4 years, 3 months ago (2016-09-01 09:25:08 UTC) #3
shinyak
Could you explain the motivation? I feel this too much. https://codereview.chromium.org/2291273005/diff/1/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/1/scripts/slave/recipe_modules/goma/api.py#newcode77 ...
4 years, 3 months ago (2016-09-01 09:38:28 UTC) #4
tikuta
I'll introduce goma module for some buildbots currently use ninja or some buildsystem directly. For ...
4 years, 3 months ago (2016-09-01 10:28:50 UTC) #6
ukai
https://codereview.chromium.org/2291273005/diff/1/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/1/scripts/slave/recipe_modules/goma/api.py#newcode17 scripts/slave/recipe_modules/goma/api.py:17: self._goma_jobs = 50 can recipe_module user configure specific number ...
4 years, 3 months ago (2016-09-02 05:02:14 UTC) #7
tikuta
It took more than 5 minutes for `git cl upload`. https://codereview.chromium.org/2291273005/diff/1/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/1/scripts/slave/recipe_modules/goma/api.py#newcode77 ...
4 years, 3 months ago (2016-09-05 03:01:37 UTC) #8
tikuta
https://codereview.chromium.org/2291273005/diff/1/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/1/scripts/slave/recipe_modules/goma/api.py#newcode17 scripts/slave/recipe_modules/goma/api.py:17: self._goma_jobs = 50 On 2016/09/02 05:02:14, ukai wrote: > ...
4 years, 3 months ago (2016-09-05 03:07:27 UTC) #9
ukai
lgtm https://codereview.chromium.org/2291273005/diff/20001/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/20001/scripts/slave/recipe_modules/goma/api.py#newcode36 scripts/slave/recipe_modules/goma/api.py:36: def goma_jobs(self): better to have docstring? https://codereview.chromium.org/2291273005/diff/20001/scripts/slave/recipe_modules/goma/api.py#newcode90 scripts/slave/recipe_modules/goma/api.py:90: ...
4 years, 3 months ago (2016-09-05 05:30:32 UTC) #10
tikuta
https://codereview.chromium.org/2291273005/diff/20001/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/20001/scripts/slave/recipe_modules/goma/api.py#newcode36 scripts/slave/recipe_modules/goma/api.py:36: def goma_jobs(self): On 2016/09/05 05:30:32, ukai wrote: > better ...
4 years, 3 months ago (2016-09-05 09:01:56 UTC) #11
Yoshisato Yanagisawa
lgtm w/ nits https://codereview.chromium.org/2291273005/diff/40001/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/40001/scripts/slave/recipe_modules/goma/api.py#newcode38 scripts/slave/recipe_modules/goma/api.py:38: Return recommended the number of jobs ...
4 years, 3 months ago (2016-09-06 06:03:52 UTC) #12
tikuta
https://codereview.chromium.org/2291273005/diff/40001/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/40001/scripts/slave/recipe_modules/goma/api.py#newcode38 scripts/slave/recipe_modules/goma/api.py:38: Return recommended the number of jobs for parallel build ...
4 years, 3 months ago (2016-09-06 06:09:52 UTC) #13
Paweł Hajdan Jr.
LGTM
4 years, 3 months ago (2016-09-06 14:18:49 UTC) #18
shinyak
https://codereview.chromium.org/2291273005/diff/60001/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/60001/scripts/slave/recipe_modules/goma/api.py#newcode47 scripts/slave/recipe_modules/goma/api.py:47: return 80 Instead of using constant 80, could you ...
4 years, 3 months ago (2016-09-07 04:45:54 UTC) #19
tikuta
https://codereview.chromium.org/2291273005/diff/60001/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/60001/scripts/slave/recipe_modules/goma/api.py#newcode47 scripts/slave/recipe_modules/goma/api.py:47: return 80 On 2016/09/07 04:45:54, shinyak wrote: > Instead ...
4 years, 3 months ago (2016-09-07 04:53:08 UTC) #20
shinyak
lgtm code itself is ok for me. please write a comment? https://codereview.chromium.org/2291273005/diff/80001/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): ...
4 years, 3 months ago (2016-09-07 05:04:00 UTC) #21
tikuta
https://codereview.chromium.org/2291273005/diff/80001/scripts/slave/recipe_modules/goma/api.py File scripts/slave/recipe_modules/goma/api.py (right): https://codereview.chromium.org/2291273005/diff/80001/scripts/slave/recipe_modules/goma/api.py#newcode45 scripts/slave/recipe_modules/goma/api.py:45: step_result = self.m.python.inline( On 2016/09/07 05:03:59, shinyak wrote: > ...
4 years, 3 months ago (2016-09-07 05:12:44 UTC) #22
tikuta
After passing dry run, I'll commit this CL.
4 years, 3 months ago (2016-09-07 06:40:26 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2291273005/100001
4 years, 3 months ago (2016-09-07 06:52:40 UTC) #30
commit-bot: I haz the power
4 years, 3 months ago (2016-09-07 06:56:24 UTC) #32
Message was sent while issue was closed.
Committed patchset #8 (id:100001) as
https://chromium.googlesource.com/chromium/tools/build/+/11785fb7f116f51bb644...

Powered by Google App Engine
This is Rietveld 408576698