Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1423993007:
CT Perf recipe to run benchmarks on the top 1k sites using swarming (Closed)
|
Created:
5 years, 1 month ago by rmistry Modified:
5 years ago CC:
chromium-reviews, infra-reviews+build_chromium.org, kjellander-cc_chromium.org, stip+watch_chromium.org, nednguyen Base URL:
https://chromium.googlesource.com/chromium/tools/build@master Target Ref:
refs/heads/master Project:
build Visibility:
Public. |
DescriptionCT Perf recipe to run benchmarks on the top 1k sites using swarming.
An explanation of what the recipe does:
* Downloads the chrome build from the parent build.
* For each shard we want to run:
* Downloads all required page sets and archives from Google Storage.
* Batch archives everything on the isolate server using the isolate file added in https://codereview.chromium.org/1410353007/
* Triggers a swarming task.
* The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com
* Waits for swarming task to complete.
I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)-
1 slave: 7 mins
2 slaves: 8 mins.
3 slaves: 9 mins
50 slaves: 38 mins (during MTV peak time)
100 slaves: 68 mins (early EST so not during MTV peak time)
Each swarming task appears to take around 6-7 mins.
BUG=skia:4503
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=297686
Patch Set 1 : Initial upload #Patch Set 2 : Working e2e but perf upload remaining #Patch Set 3 : Ready for review after cleanup #
Total comments: 24
Patch Set 4 : Address feedback #
Total comments: 2
Patch Set 5 : Use batcharchive #
Total comments: 12
Patch Set 6 : Address comments #Patch Set 7 : Extract reusable functionality into recipe modules #
Total comments: 6
Patch Set 8 : Use isolate and swarming recipes #
Total comments: 10
Patch Set 9 : Address comments #Patch Set 10 : Create placeholder for tests #Patch Set 11 : Add task_output_dir #Patch Set 12 : Use extra vars instead of isolate templates #Patch Set 13 : Set executable bit on binary #Patch Set 14 : Use file api for writing and inline python for exec bit #Patch Set 15 : Add tests and cleanup #Patch Set 16 : Undo changes to master.cfg and slaves.cfg #Patch Set 17 : Rename ct_top1k_rr_perf to ct_top1k_perf #
Total comments: 10
Patch Set 18 : Address comments #
Total comments: 2
Patch Set 19 : Use enumerate #Patch Set 20 : Fix indentation in slaves.cfg #Dependent Patchsets: Messages
Total messages: 76 (46 generated)
Description was changed from ========== Checkpoint BUG= ========== to ========== CT + Swarming + Perf waterfall Prototype BUG= ==========
Description was changed from ========== CT + Swarming + Perf waterfall Prototype BUG= ========== to ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. A high level explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task will run the archives in the isolate file and will upload results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. BUG=skia:4503 ==========
Description was changed from ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. A high level explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task will run the archives in the isolate file and will upload results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. BUG=skia:4503 ========== to ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. A high level explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. BUG=skia:4503 ==========
Description was changed from ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. A high level explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. BUG=skia:4503 ========== to ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. A high level explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) BUG=skia:4503 ==========
Description was changed from ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. A high level explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) BUG=skia:4503 ========== to ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) BUG=skia:4503 ==========
Description was changed from ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) BUG=skia:4503 ========== to ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) Each swarming task appears to take around 6-7 mins. BUG=skia:4503 ==========
rmistry@google.com changed reviewers: + maruel@chromium.org
Hi Marc-Antoine, This is still unpolished. I would like to extract some things into recipe_modules and add unit tests. I wanted to show you what I have so far to get your input on the design and if I am way off the mark or not. Disclaimer: I am very new to both swarming and writing recipes outside Skia.
Description was changed from ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) Each swarming task appears to take around 6-7 mins. BUG=skia:4503 ========== to ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) Each swarming task appears to take around 6-7 mins. BUG=skia:4503 COMMIT=false This is still unpolished. I would like to extract some things into recipe_modules and add unit tests. ==========
Can you delete patchsets #1 to #19? Thanks. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:5: import os You are not supposed to import anything in recipes. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:35: # Number of slaves to shard CT runs to. s/slaves/bots/ https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:73: downloads_dir = os.path.join(chromium_src_dir, 'content', 'test', 'ct') Why not put this in tmp too? https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:75: swarming_temp_dir = str(api.path.mkdtemp('swarming-temp-dir')) Use self.m.path['tmp_base'] instead. That's what _gtest_collect_step() does. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:92: page_sets_dir = os.path.join(slave_dir, 'page_sets') I don't understand why you need to download files CT_NUM_SLAVES times. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:127: with open(isolate_template_path, 'rt') as fin: use b instead of t, we don't want any CR character. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:135: fout.close() When using with open() ..., you shouldn't close the handle. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:148: '--path-variable', 'PRODUCT_DIR', tempfile.gettempdir(), Can you add more details about what fails? https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:191: # TODO(rmistry): Delete me! :) https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:199: shutil.rmtree(swarming_temp_dir) git gs "def rmtree" gave me recipe_modules/file/api.py:140: def rmtree(self, name, path, **kwargs):
Patchset #21 (id:400001) has been deleted
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
Patchset #1 (id:80001) has been deleted
Patchset #1 (id:100001) has been deleted
Patchset #1 (id:120001) has been deleted
Patchset #1 (id:140001) has been deleted
Patchset #1 (id:160001) has been deleted
Patchset #2 (id:200001) has been deleted
Patchset #2 (id:220001) has been deleted
Patchset #2 (id:240001) has been deleted
Patchset #2 (id:260001) has been deleted
Patchset #2 (id:280001) has been deleted
Patchset #3 (id:320001) has been deleted
Patchset #3 (id:340001) has been deleted
Patchset #3 (id:360001) has been deleted
On 2015/11/06 18:56:06, M-A Ruel wrote: > Can you delete patchsets #1 to #19? Thanks. Deleted majority of the patchsets. > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:5: import os > You are not supposed to import anything in recipes. > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:35: # Number of slaves to shard > CT runs to. > s/slaves/bots/ > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:73: downloads_dir = > os.path.join(chromium_src_dir, 'content', 'test', 'ct') > Why not put this in tmp too? > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:75: swarming_temp_dir = > str(api.path.mkdtemp('swarming-temp-dir')) > Use self.m.path['tmp_base'] instead. That's what _gtest_collect_step() does. > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:92: page_sets_dir = > os.path.join(slave_dir, 'page_sets') > I don't understand why you need to download files CT_NUM_SLAVES times. > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:127: with > open(isolate_template_path, 'rt') as fin: > use b instead of t, we don't want any CR character. > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:135: fout.close() > When using with open() ..., you shouldn't close the handle. > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:148: '--path-variable', > 'PRODUCT_DIR', tempfile.gettempdir(), > Can you add more details about what fails? > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:191: # TODO(rmistry): Delete me! > :) > > https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:199: > shutil.rmtree(swarming_temp_dir) > git gs "def rmtree" > gave me > recipe_modules/file/api.py:140: def rmtree(self, name, path, **kwargs):
https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:5: import os On 2015/11/06 18:56:06, M-A Ruel wrote: > You are not supposed to import anything in recipes. Done. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:35: # Number of slaves to shard CT runs to. On 2015/11/06 18:56:06, M-A Ruel wrote: > s/slaves/bots/ I used "slaves" here to be consistent with CT's nomenclature where it calls its workers/bots "slaves". The GS directory it downloads artifacts from is also called "slave{1..100}". I can use slaves->bots everywhere here if you prefer, but the GS directory name will still be "slave{1..100}". https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:73: downloads_dir = os.path.join(chromium_src_dir, 'content', 'test', 'ct') On 2015/11/06 18:56:06, M-A Ruel wrote: > Why not put this in tmp too? I initially went that route but it made the isolate template file messy. I then opted into storing things in the chromium checkout because it seemed much easier for the isolate file to find artifacts when they were relative to the chromium checkout. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:75: swarming_temp_dir = str(api.path.mkdtemp('swarming-temp-dir')) On 2015/11/06 18:56:06, M-A Ruel wrote: > Use self.m.path['tmp_base'] instead. That's what _gtest_collect_step() does. Done. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:92: page_sets_dir = os.path.join(slave_dir, 'page_sets') On 2015/11/06 18:56:06, M-A Ruel wrote: > I don't understand why you need to download files CT_NUM_SLAVES times. The purpose of this recipe is to run benchmarks on the top 1000 web pages. This is done by sharding 10 web pages to each swarming bot. slave1 will process the top 1-10 web pages. slave2 will process 11-20 webpages and so on. Sharding these web pages involves sending each bot the pageset file of the web pages and the archive of the web pages. Below you will see that these artifacts are downloaded from a GS directory that is unique for each slave (contains slave1,slave2,...). Note: this follows the current architecture that CT uses (https://skia.org/dev/testing/ct#system_diagram) but is attempting to use swarming bots instead of the machines from CT's pool. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:127: with open(isolate_template_path, 'rt') as fin: On 2015/11/06 18:56:06, M-A Ruel wrote: > use b instead of t, we don't want any CR character. Agreed. Done. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:135: fout.close() On 2015/11/06 18:56:06, M-A Ruel wrote: > When using with open() ..., you shouldn't close the handle. Done. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:148: '--path-variable', 'PRODUCT_DIR', tempfile.gettempdir(), On 2015/11/06 18:56:06, M-A Ruel wrote: > Can you add more details about what fails? If I do not specify --path-variable then it fails with: IsolateError: Variable "PRODUCT_DIR" was not found in {'EXECUTABLE_SUFFIX': ''}. Did you forget to specify --path-variable? Also if I do not create a dummy /tmp/bitmaptools then it fails with: MappingError: Input file /tmp/bitmaptools doesn't exist I believe this is due to https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te... and I do not know how to make it pass without my above hack. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:191: # TODO(rmistry): Delete me! On 2015/11/06 18:56:06, M-A Ruel wrote: > :) Deleted. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:199: shutil.rmtree(swarming_temp_dir) On 2015/11/06 18:56:06, M-A Ruel wrote: > git gs "def rmtree" > gave me > recipe_modules/file/api.py:140: def rmtree(self, name, path, **kwargs): Done.
https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:35: # Number of slaves to shard CT runs to. On 2015/11/09 16:23:49, rmistry wrote: > On 2015/11/06 18:56:06, M-A Ruel wrote: > > s/slaves/bots/ > > I used "slaves" here to be consistent with CT's nomenclature where it calls its > workers/bots "slaves". The GS directory it downloads artifacts from is also > called "slave{1..100}". I can use slaves->bots everywhere here if you prefer, > but the GS directory name will still be "slave{1..100}". There's two points of view: - the word slave was maybe used because it implied buildbot slave? - if it's moving to swarming bots and it effectively represent swarming bots, long term switching to bots is a better choice. I don't mind too much. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:148: '--path-variable', 'PRODUCT_DIR', tempfile.gettempdir(), On 2015/11/09 16:23:49, rmistry wrote: > On 2015/11/06 18:56:06, M-A Ruel wrote: > > Can you add more details about what fails? > > If I do not specify --path-variable then it fails with: > IsolateError: Variable "PRODUCT_DIR" was not found in {'EXECUTABLE_SUFFIX': ''}. > Did you forget to specify --path-variable? > > Also if I do not create a dummy /tmp/bitmaptools then it fails with: > MappingError: Input file /tmp/bitmaptools doesn't exist > > I believe this is due to > https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te... > and I do not know how to make it pass without my above hack. Do you need this file at all? If not, we should fix that. https://codereview.chromium.org/1423993007/diff/420001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/420001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:131: # Archive everything on the isolate server. Actually, you *really* want to use batcharchive and archive everything at once. That's really important performance wise. This means: - moving this call after the loop. - creating a second loop to trigger tasks after. This may complicate things a bit but this will greatly improve performance. batcharchive basically takes a bunch of .gen.json files, which specify the command line arguments of each isolate call. Then it does magics to make this fast.
https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:35: # Number of slaves to shard CT runs to. On 2015/11/09 16:51:21, M-A Ruel wrote: > On 2015/11/09 16:23:49, rmistry wrote: > > On 2015/11/06 18:56:06, M-A Ruel wrote: > > > s/slaves/bots/ > > > > I used "slaves" here to be consistent with CT's nomenclature where it calls > its > > workers/bots "slaves". The GS directory it downloads artifacts from is also > > called "slave{1..100}". I can use slaves->bots everywhere here if you prefer, > > but the GS directory name will still be "slave{1..100}". > > There's two points of view: > - the word slave was maybe used because it implied buildbot slave? > - if it's moving to swarming bots and it effectively represent swarming bots, > long term switching to bots is a better choice. > > I don't mind too much. The word slave was used in CT independently of buildbot. It was used because in the original architecture there was 1 master and 100 slaves. I will leave "slave" here for now because changing it will require changing lots of documentation. https://codereview.chromium.org/1423993007/diff/380001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:148: '--path-variable', 'PRODUCT_DIR', tempfile.gettempdir(), On 2015/11/09 16:51:21, M-A Ruel wrote: > On 2015/11/09 16:23:49, rmistry wrote: > > On 2015/11/06 18:56:06, M-A Ruel wrote: > > > Can you add more details about what fails? > > > > If I do not specify --path-variable then it fails with: > > IsolateError: Variable "PRODUCT_DIR" was not found in {'EXECUTABLE_SUFFIX': > ''}. > > Did you forget to specify --path-variable? > > > > Also if I do not create a dummy /tmp/bitmaptools then it fails with: > > MappingError: Input file /tmp/bitmaptools doesn't exist > > > > I believe this is due to > > > https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te... > > and I do not know how to make it pass without my above hack. > > Do you need this file at all? If not, we should fix that. +nednyugen Ned, is this file required? if yes then how do I specify a path to it? https://codereview.chromium.org/1423993007/diff/420001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/420001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:131: # Archive everything on the isolate server. On 2015/11/09 16:51:21, M-A Ruel wrote: > Actually, you *really* want to use batcharchive and archive everything at once. > That's really important performance wise. This means: > - moving this call after the loop. > - creating a second loop to trigger tasks after. > > This may complicate things a bit but this will greatly improve performance. > > batcharchive basically takes a bunch of .gen.json files, which specify the > command line arguments of each isolate call. Then it does magics to make this > fast. Done. PTAL.
https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:39: build_archive_url = api.properties['parent_build_archive_url'] I think you could inline this and it'd fit 80 cols ? https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:70: downloads_dir = chromium_src_dir.join('content', 'test', 'ct') Is it in the .gitignore? Otherwise it'll get continuously deleted. Is is possible to keep a local cache to speed things up or it's not an issue in practice? https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:164: for isolated_gen_json in isolated_gen_json_files: batcharchive_args.extend(str(i) for i in isolated_gen_json_files) https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:168: api.swarming_client.path.join('isolate.py'), Use luci-go implementation which is an order of magnitude faster. You should be able to use it via the isolate recipe.
nednguyen@google.com changed reviewers: + nednguyen@google.com
https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:143: # requires bitmaptools in PRODUCT_DIR. Bitmaptools build is used by telemetry's image processing code. It's required, unless you can prebuilt the binary on multiple platforms & upload them to cloud storage so telemetry can fetch the file. I honestly don't know how to reference the path to bitmap tool in this context. Can you just reuse the isolate defined for telemetry+chrome in https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/chrome_...
https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:39: build_archive_url = api.properties['parent_build_archive_url'] On 2015/11/09 19:46:45, M-A Ruel wrote: > I think you could inline this and it'd fit 80 cols ? Done. https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:70: downloads_dir = chromium_src_dir.join('content', 'test', 'ct') On 2015/11/09 19:46:45, M-A Ruel wrote: > Is it in the .gitignore? Otherwise it'll get continuously deleted. Is is > possible to keep a local cache to speed things up or it's not an issue in > practice? I think its ok to get continuously deleted for now. I can look into optimizing this if needed once things are up and running. https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:143: # requires bitmaptools in PRODUCT_DIR. On 2015/11/09 23:04:32, nednguyen wrote: > Bitmaptools build is used by telemetry's image processing code. It's required, > unless you can prebuilt the binary on multiple platforms & upload them to cloud > storage so telemetry can fetch the file. > > I honestly don't know how to reference the path to bitmap tool in this context. > Can you just reuse the isolate defined for telemetry+chrome in > https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/chrome_... I tried using it and it failed with: IsolateError: These configuration variables were missing from the command line: CONFIGURATION_NAME, asan, component, disable_nacl, fastbuild, icu_use_data_file_flag, kasko, lsan, msan, msvs_version, target_arch, tsan, use_custom_libcxx, use_instrumented_libraries, use_prebuilt_instrumented_libraries, v8_use_external_startup_data thats a lot of missing variables, I do not know how to specify them here. As a workaround I removed the reference to telemetry.isolate in https://codereview.chromium.org/1410353007/diff2/220001:240001/chrome/ct_top1... for now. https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:164: for isolated_gen_json in isolated_gen_json_files: On 2015/11/09 19:46:45, M-A Ruel wrote: > batcharchive_args.extend(str(i) for i in isolated_gen_json_files) Done. https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:168: api.swarming_client.path.join('isolate.py'), On 2015/11/09 19:46:45, M-A Ruel wrote: > Use luci-go implementation which is an order of magnitude faster. You should be > able to use it via the isolate recipe. How do I use it using https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... ? (sorry, I must have missed something).
nednguyen@google.com changed reviewers: + kbr@chromium.org
My build knowledge is very questionable. +Kbr: how did your gpu test use telemetry.isolate without encountering problem with bitmaptools build?
Patchset #7 (id:480001) has been deleted
Patchset #7 (id:500001) has been deleted
In @Patchset7 I extracted reusable functionality into a ct_swarming recipe module. On 2015/11/10 12:58:06, rmistry wrote: > https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... > File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): > > https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:39: build_archive_url = > api.properties['parent_build_archive_url'] > On 2015/11/09 19:46:45, M-A Ruel wrote: > > I think you could inline this and it'd fit 80 cols ? > > Done. > > https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:70: downloads_dir = > chromium_src_dir.join('content', 'test', 'ct') > On 2015/11/09 19:46:45, M-A Ruel wrote: > > Is it in the .gitignore? Otherwise it'll get continuously deleted. Is is > > possible to keep a local cache to speed things up or it's not an issue in > > practice? > > I think its ok to get continuously deleted for now. I can look into optimizing > this if needed once things are up and running. > > https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:143: # requires > bitmaptools in PRODUCT_DIR. > On 2015/11/09 23:04:32, nednguyen wrote: > > Bitmaptools build is used by telemetry's image processing code. It's required, > > unless you can prebuilt the binary on multiple platforms & upload them to > cloud > > storage so telemetry can fetch the file. > > > > I honestly don't know how to reference the path to bitmap tool in this > context. > > Can you just reuse the isolate defined for telemetry+chrome in > > > https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/chrome_... > > I tried using it and it failed with: > IsolateError: These configuration variables were missing from the command line: > CONFIGURATION_NAME, asan, component, disable_nacl, fastbuild, > icu_use_data_file_flag, kasko, lsan, msan, msvs_version, target_arch, tsan, > use_custom_libcxx, use_instrumented_libraries, > use_prebuilt_instrumented_libraries, v8_use_external_startup_data > > thats a lot of missing variables, I do not know how to specify them here. > > As a workaround I removed the reference to telemetry.isolate in > https://codereview.chromium.org/1410353007/diff2/220001:240001/chrome/ct_top1... > for now. > > https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:164: for isolated_gen_json in > isolated_gen_json_files: > On 2015/11/09 19:46:45, M-A Ruel wrote: > > batcharchive_args.extend(str(i) for i in isolated_gen_json_files) > > Done. > > https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... > scripts/slave/recipes/perf/ct_top1k_rr_perf.py:168: > api.swarming_client.path.join('isolate.py'), > On 2015/11/09 19:46:45, M-A Ruel wrote: > > Use luci-go implementation which is an order of magnitude faster. You should > be > > able to use it via the isolate recipe. > > How do I use it using > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > ? > (sorry, I must have missed something).
https://codereview.chromium.org/1423993007/diff/520001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/ct_swarming/api.py (right): https://codereview.chromium.org/1423993007/diff/520001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:5: import urllib Remove https://codereview.chromium.org/1423993007/diff/520001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:144: json_output = self.swarming_temp_dir.join('ct-task-%s.json' % slave_num) I don't think duplicating swarming.py is a good idea. On the other hand I don't want to block you. So rubberstamp lgtm but if I break the python script, which I know I'll do in the future, you'll keep the pieces. Also you'll have to switch to the Go implementation of the batcharchive. But to get something up and running to verify that things works it's fine. https://codereview.chromium.org/1423993007/diff/520001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:151: '--dump-json', json_output Specify: '--priority', '90' This will run a quite low priority using whatever left over capacity there is.
https://codereview.chromium.org/1423993007/diff/520001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/ct_swarming/api.py (right): https://codereview.chromium.org/1423993007/diff/520001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:144: json_output = self.swarming_temp_dir.join('ct-task-%s.json' % slave_num) On 2015/11/11 23:53:36, M-A Ruel wrote: > I don't think duplicating swarming.py is a good idea. On the other hand I don't > want to block you. > > So rubberstamp lgtm but if I break the python script, which I know I'll do in > the future, you'll keep the pieces. > > Also you'll have to switch to the Go implementation of the batcharchive. But to > get something up and running to verify that things works it's fine. I would like to switch to the Go implementation so that things do not break in the future. But I have not been able to figure out how to do that, see my comment here: https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/...
https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:168: api.swarming_client.path.join('isolate.py'), On 2015/11/10 12:58:06, rmistry wrote: > On 2015/11/09 19:46:45, M-A Ruel wrote: > > Use luci-go implementation which is an order of magnitude faster. You should > be > > able to use it via the isolate recipe. > > How do I use it using > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > ? > (sorry, I must have missed something). isolate_tests() is the function you need. It is hardcoded to glob all the *.isolated.gen.json in the output directory but this could be changed if necessary. Otherwise you can write the temporary gen files in the output directory. Overall it's kind of hardcoded for our use case so it's fairly possible that it'll need refactoring.
Patchset #8 (id:540001) has been deleted
https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/440001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:168: api.swarming_client.path.join('isolate.py'), On 2015/11/12 00:36:01, M-A Ruel wrote: > On 2015/11/10 12:58:06, rmistry wrote: > > On 2015/11/09 19:46:45, M-A Ruel wrote: > > > Use luci-go implementation which is an order of magnitude faster. You should > > be > > > able to use it via the isolate recipe. > > > > How do I use it using > > > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > > ? > > (sorry, I must have missed something). > > isolate_tests() is the function you need. It is hardcoded to glob all the > *.isolated.gen.json in the output directory but this could be changed if > necessary. Otherwise you can write the temporary gen files in the output > directory. Overall it's kind of hardcoded for our use case so it's fairly > possible that it'll need refactoring. > I could use it without refactoring. Also used trigger and collect from the swarming recipe. Done. PTAL. https://codereview.chromium.org/1423993007/diff/520001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/ct_swarming/api.py (right): https://codereview.chromium.org/1423993007/diff/520001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:5: import urllib On 2015/11/11 23:53:36, M-A Ruel wrote: > Remove Done. https://codereview.chromium.org/1423993007/diff/520001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:151: '--dump-json', json_output On 2015/11/11 23:53:36, M-A Ruel wrote: > Specify: '--priority', '90' > > This will run a quite low priority using whatever left over capacity there is. Done. I brought up a job to see if this worked but it still showed up as priority 100: https://chromium-swarm.appspot.com/user/task/2b14d294d7c6ef10 I saw the following message in my logs: "Priority was reset to 100". Any suggestions?
Sorry for the delays as I'm travelling. My main goal here is to reduce the size of the recipe as much as we can to simplify maintenance. https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/ct_swarming/api.py (right): https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:19: self.chromium_src_dir = None If you want to keep this, I'd prefer @property def chromium_dir(self): return self.m.path['checkout'] but I don't see this much useful. https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:22: self.downloads_dir = None Make this a property, this reduces aliasing of the root variables. https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:54: self._created_dirs.append(d) IIUC, 'tmp_base' is deleted automatically, so no need to keep track of these. In particular what you want is file.mkdtemp(): https://github.com/luci/recipes-py/blob/master/recipe_modules/path/api.py#L189 https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:160: for d in self._created_dirs: This shouldn't be needed in practice, if it is, we should refactor until it's not needed. https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:60: api.ct_swarming.download_page_artifacts(CT_PAGE_TYPE, slave_num, slave_dir) IMHO You shouldn't pass an arbitrary path, the path should be returned instead.
On 2015/11/10 14:05:39, nednguyen wrote: > My build knowledge is very questionable. > +Kbr: how did your gpu test use telemetry.isolate without encountering problem > with bitmaptools build? Not sure whether you've already figured this out, but telemetry_gpu_test_run (used by the gyp build) depends indirectly on telemetry_base, which build bitmaptools. https://code.google.com/p/chromium/codesearch#chromium/src/content/content_te...
On 2015/11/12 23:41:49, Ken Russell wrote: > On 2015/11/10 14:05:39, nednguyen wrote: > > My build knowledge is very questionable. > > +Kbr: how did your gpu test use telemetry.isolate without encountering problem > > with bitmaptools build? > > Not sure whether you've already figured this out, but telemetry_gpu_test_run > (used by the gyp build) depends indirectly on telemetry_base, which build > bitmaptools. > https://code.google.com/p/chromium/codesearch#chromium/src/content/content_te... ah k, thanks for the information!
Patchset #9 (id:580001) has been deleted
Patchset #9 (id:600001) has been deleted
https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/ct_swarming/api.py (right): https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:19: self.chromium_src_dir = None On 2015/11/12 17:33:41, M-A Ruel wrote: > If you want to keep this, I'd prefer > > @property > def chromium_dir(self): > return self.m.path['checkout'] > > but I don't see this much useful. Agreed. Done. https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:22: self.downloads_dir = None On 2015/11/12 17:33:41, M-A Ruel wrote: > Make this a property, this reduces aliasing of the root variables. Done. https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:54: self._created_dirs.append(d) On 2015/11/12 17:33:41, M-A Ruel wrote: > IIUC, 'tmp_base' is deleted automatically, so no need to keep track of these. > > In particular what you want is file.mkdtemp(): > https://github.com/luci/recipes-py/blob/master/recipe_modules/path/api.py#L189 Yes looks like tmp_base is deleted automatically. https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:160: for d in self._created_dirs: On 2015/11/12 17:33:41, M-A Ruel wrote: > This shouldn't be needed in practice, if it is, we should refactor until it's > not needed. Yes I do not think this is needed, I thought it was because I saw directories pile up in my local testing when I had commented out the part that cleaned the local checkout. Removed. https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_rr_perf.py (right): https://codereview.chromium.org/1423993007/diff/560001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_rr_perf.py:60: api.ct_swarming.download_page_artifacts(CT_PAGE_TYPE, slave_num, slave_dir) On 2015/11/12 17:33:41, M-A Ruel wrote: > IMHO You shouldn't pass an arbitrary path, the path should be returned instead. Done. Not returning the path because the downloads_dir is already available.
Patchset #12 (id:680001) has been deleted
Description was changed from ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) Each swarming task appears to take around 6-7 mins. BUG=skia:4503 COMMIT=false This is still unpolished. I would like to extract some things into recipe_modules and add unit tests. ========== to ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) Each swarming task appears to take around 6-7 mins. BUG=skia:4503 ==========
Patchset #15 (id:760001) has been deleted
Patchset #15 (id:770001) has been deleted
Description was changed from ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Uses the isolate file template in https://codereview.chromium.org/1410353007/ to dynamically create isolate files from the downloads. * Archives everything on the isolate server. * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. * Cleans up. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) Each swarming task appears to take around 6-7 mins. BUG=skia:4503 ========== to ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Batch archives everything on the isolate server using the isolate file added in https://codereview.chromium.org/1410353007/ * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) Each swarming task appears to take around 6-7 mins. BUG=skia:4503 ==========
Description was changed from ========== CT Perf recipe to run rasterize_and_record_micro on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Batch archives everything on the isolate server using the isolate file added in https://codereview.chromium.org/1410353007/ * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) Each swarming task appears to take around 6-7 mins. BUG=skia:4503 ========== to ========== CT Perf recipe to run benchmarks on the top 1k sites using swarming. An explanation of what the recipe does: * Downloads the chrome build from the parent build. * For each shard we want to run: * Downloads all required page sets and archives from Google Storage. * Batch archives everything on the isolate server using the isolate file added in https://codereview.chromium.org/1410353007/ * Triggers a swarming task. * The swarming task runs benchmark on the archives and uploads results to chromeperf.appspot.com * Waits for swarming task to complete. I have triggered tasks on swarming and things appear to be working. Here are some timings with 1 page repeat (this might be upped to 3 page repeats for more reliable results)- 1 slave: 7 mins 2 slaves: 8 mins. 3 slaves: 9 mins 50 slaves: 38 mins (during MTV peak time) 100 slaves: 68 mins (early EST so not during MTV peak time) Each swarming task appears to take around 6-7 mins. BUG=skia:4503 ==========
* Cleaned up things after https://codereview.chromium.org/1410353007/ landed in chromium/src. * Added tests. * Removed changes to master.cfg and slaves.cfg. They will be made after we get a new perf bot, tracked in https://code.google.com/p/chromium/issues/detail?id=557269 PTAL.
On 2015/11/19 16:17:17, rmistry wrote: > * Cleaned up things after https://codereview.chromium.org/1410353007/ landed in > chromium/src. > * Added tests. > * Removed changes to master.cfg and slaves.cfg. They will be made after we get a > new perf bot, tracked in > https://code.google.com/p/chromium/issues/detail?id=557269 > > PTAL. Friendly ping incase anybody has comments. I would like to land this early next week if possible.
https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/ct_swarming/api.py (right): https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:30: return self._downloads_dir I'd prefer return self.m.path['checkout'].join('content', 'test', 'ct') so you can delete lines 19 and 44 and migrate comment at line 17-18 here as a docstring. Same thing for _swarming_temp_dir and tasks_output_dir. https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:167: self.m.swarming.trigger(self._swarming_tasks) You should return the tasks and remove self._swarming_tasks. It is closer to what recipes_modules/swarming/api.py does (yet still different, but closer). https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:169: def collect_swarming_tasks(self): Accept the tasks as a parameter. https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_perf.py (right): https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_perf.py:80: api.ct_swarming.trigger_swarming_tasks( tasks = api.ct_swarming.trigger_swarming_tasks( https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_perf.py:85: api.ct_swarming.collect_swarming_tasks() api.ct_swarming.collect_swarming_tasks(tasks)
Patchset #18 (id:840001) has been deleted
Also kept back changes to master.cfg and slaves.cfg because https://code.google.com/p/chromium/issues/detail?id=557269 was resolved. https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/ct_swarming/api.py (right): https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:30: return self._downloads_dir On 2015/11/20 18:14:01, M-A Ruel wrote: > I'd prefer > return self.m.path['checkout'].join('content', 'test', 'ct') > > so you can delete lines 19 and 44 and migrate comment at line 17-18 here as a > docstring. > > Same thing for _swarming_temp_dir and tasks_output_dir. Done. https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:167: self.m.swarming.trigger(self._swarming_tasks) On 2015/11/20 18:14:01, M-A Ruel wrote: > You should return the tasks and remove self._swarming_tasks. It is closer to > what recipes_modules/swarming/api.py does (yet still different, but closer). Done. https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:169: def collect_swarming_tasks(self): On 2015/11/20 18:14:01, M-A Ruel wrote: > Accept the tasks as a parameter. Done. https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipes/... File scripts/slave/recipes/perf/ct_top1k_perf.py (right): https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_perf.py:80: api.ct_swarming.trigger_swarming_tasks( On 2015/11/20 18:14:01, M-A Ruel wrote: > tasks = api.ct_swarming.trigger_swarming_tasks( Done. https://codereview.chromium.org/1423993007/diff/820001/scripts/slave/recipes/... scripts/slave/recipes/perf/ct_top1k_perf.py:85: api.ct_swarming.collect_swarming_tasks() On 2015/11/20 18:14:01, M-A Ruel wrote: > api.ct_swarming.collect_swarming_tasks(tasks) Done.
lgtm Thanks for bearing with me. I think the resulting CTSwarmingApi is easier to reason about as there's no internal state anymore. https://codereview.chromium.org/1423993007/diff/860001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/ct_swarming/api.py (right): https://codereview.chromium.org/1423993007/diff/860001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:150: for swarm_hash in swarm_hashes: for task_num, swarm_hash in enumerate(swarm_hashes): then remove lines 148 and 151
Patchset #19 (id:880001) has been deleted
Thanks for the thorough review! https://codereview.chromium.org/1423993007/diff/860001/scripts/slave/recipe_m... File scripts/slave/recipe_modules/ct_swarming/api.py (right): https://codereview.chromium.org/1423993007/diff/860001/scripts/slave/recipe_m... scripts/slave/recipe_modules/ct_swarming/api.py:150: for swarm_hash in swarm_hashes: On 2015/11/23 15:19:57, M-A Ruel wrote: > for task_num, swarm_hash in enumerate(swarm_hashes): > > then remove lines 148 and 151 Done.
nednguyen@google.com changed reviewers: + dtu@chromium.org
+Dave
The CQ bit was checked by rmistry@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1423993007/920001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1423993007/920001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by rmistry@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from maruel@chromium.org Link to the patchset: https://codereview.chromium.org/1423993007/#ps920001 (title: "Fix indentation in slaves.cfg")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1423993007/920001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1423993007/920001
Message was sent while issue was closed.
Committed patchset #20 (id:920001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=297686 |
