New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 681689 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android
Pri: 2
Type: Bug

Blocking:
issue 482401
issue 680986



Sign in to add a comment

Perf builds archiving .o files for no reason

Project Member Reported by agrieve@chromium.org, Jan 16 2017

Issue description

At least, it seems like we shouldn't be doing this. From what I can tell:
* Android uses scripts/slave/android/archive_build.py
  e.g.: https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Fchromium.perf%2FAndroid_Builder%2F133430%2F%2B%2Frecipes%2Fsteps%2Fzip_build_product%2F0%2Fstdout
* Others use scripts/slave/zip_build.py
  e.g.: https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Fchromium.perf%2FLinux_Builder%2F88308%2F%2B%2Frecipes%2Fsteps%2Fpackage_build%2F0%2Fstdout

No idea why these are separate...

zip_build.py runs with some exclusions:
  Inclusions: []
  Exclusions: ['.landmines', 'obj', 'gen', '.ninja_deps', '.ninja_log', 'appcache', 'glue', 'lib.host', 'obj.host', 'obj.target', 'src', '.deps', '.sconsign.dblite', 'mksnapshot']
  Whitelist regex: '$NO_FILTER^'
  Blacklist regex: '^.+\.(o|a|d)$'
  Zip unmatched files: True
  Exclude extra: False
  Custom Whitelist regex: ''

However, you can see from the log that it doesn't actually exclude the .o files.

I'd guess we should:
- Standardize on a single archive script
- Fix exclusions of .o files (as well as .o.whitelist on Android). We're producing 2gb per build on Android, which seems excessive.
 
Blocking: 680986
Blocking: 482401
Cc: mikec...@chromium.org rnep...@chromium.org martiniss@chromium.org
Labels: -Pri-3 Pri-2
Could one of you guys pick this up?
Looked at this a bit and found:
1. zip_build.py properly excludes .o files within obj/ (primary toolchain), but is not excluding form clang_*/obj (secondary toolchains)

2. Android Builder uses ./scripts/slave/android/archive_build.py, which doesn't have .o exclusion logic, but the up-and-comping Android Compile uses the normal zip_build.py:
https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Fchromium.perf%2FAndroid_Compile%2F39792%2F%2B%2Frecipes%2Fsteps%2Fpackage_build%2F0%2Fstdout

In terms of standardizing on fewer scripts, looks like there are actually *three* scripts that zip and upload:

./scripts/slave/zip_build.py  # Zips and uploads
   * Used by archive.api.zip_and_upload_build()
      * Used by perf android builds, but not other android builders.
./scripts/slave/chromium/archive_build.py  # Zips and uploads. include changelog  
   * called by chromium_factory.AddArchiveBuild()
   * called by chromium.api.archive_build()
./scripts/slave/android/archive_build.py  # Wrapper around zip, upload happens in separate step.
   * called by chromium_android/api.make_zip_archive()


gclient_factory.py:
  306     if not skip_archive_steps:
  307       # Archive the full output directory if the machine is a builder.
  308       if slave_type in ['Builder', 'TrybotBuilder']:
  309         if build_url or factory_properties.get('build_url'):
  310           # There are some builders that are classified as builders, but only
  311           # because we only need to see if the patch compiles, and not
  312           # actually because triggered testers needs the build. So if we don't
  313           # supply a build_url, we assume that the build artifact doesn't
  314           # need to be uploaded and we can bypass this step.
  315           factory_cmd_obj.AddZipBuild(build_url,
  316                                       factory_properties=factory_properties)

chromium_factory.py: (extends gclient_factory.py)
  724     factory = self.BuildFactory(target, clobber, tests_for_build, mode,
  725                                 slave_type, options, compile_timeout, build_url,
  726                                 project, factory_properties,
  727                                 gclient_deps=gclient_deps,
  728                                 skip_archive_steps=False)
…
  736     # Add this archive build step.
  737     if factory_properties.get('archive_build'):
  738       chromium_cmd_obj.AddArchiveBuild(factory_properties=factory_properties)


android/builder.py:
  103       # Perf bisect builders uses custom file names for binaries with
  104       # DEPS changes, and the logic for this is in zip_build.py.
  105       'zip_and_upload': {
  106         'bucket': 'chrome-perf',
  107       },
  275   if upload_config:
  276     droid.zip_and_upload_build(upload_config['bucket'])  # Wrapper around archive.zip_and_upload_build()

Also - there's a script for filtering build directory:
scripts/slave/recipe_modules/archive/resources/filter_build_files.py

which is used only by scripts/slave/recipe_modules/archive/api: clusterfuzz_archive()
Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40

commit 3a99c2ed795b1f46d38e744d46f1eb6f5e290b40
Author: Andrew Grieve <agrieve@chromium.org>
Date: Tue May 02 20:20:57 2017

Add --exclude filters for android builder archives

Most other ways of archiving the out directory already had these
filters. See bug for more details.

This should dramatically reduce size of the archives (currently ~2gb).

BUG= 681689 

Change-Id: I8cbf6dfa270860e21efa430b566f00b6ed45a682
Reviewed-on: https://chromium-review.googlesource.com/491706
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>

[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipe_modules/chromium_android/example.py
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipes/android/builder.expected/full_chromium_perf_Android_Builder.json
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipes/android/builder.expected/full_chromium_perf_Android_arm64_Builder.json
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipe_modules/chromium_android/tests/upload_build.expected/basic.json
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipe_modules/chromium_android/example.expected/restart_usb_builder_basic.json
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/android/archive_build.py
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/android/archive_build_unittest.py
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipe_modules/chromium_android/example.expected/gerrit_try_builder_basic.json
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipes/android/builder.expected/full_client_v8_fyi_Android_Builder.json
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipe_modules/chromium_android/api.py
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipe_modules/chromium_android/example.expected/gerrit_refs.json
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipe_modules/chromium_android/example.expected/basic_builder_basic.json
[modify] https://crrev.com/3a99c2ed795b1f46d38e744d46f1eb6f5e290b40/scripts/slave/recipe_modules/chromium_android/example.expected/coverage_builder_basic.json

Owner: agrieve@chromium.org
Status: Started (was: Available)
Project Member

Comment 8 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/1c881be4fb63733820a29955a7e0772f1898921e

commit 1c881be4fb63733820a29955a7e0772f1898921e
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed May 03 13:14:10 2017

zip_build.py: Exclude secondary toolchain obj and gen, as well as .ninja

BUG= 681689 

Change-Id: I91ffb62cb05144d1198e95347166528ece778390
Reviewed-on: https://chromium-review.googlesource.com/491806
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>

[modify] https://crrev.com/1c881be4fb63733820a29955a7e0772f1898921e/scripts/slave/zip_build.py

Status: Fixed (was: Started)
Android perf builders archive is now 907.29 MiB (down from 1.99GB)
Project Member

Comment 10 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/1819fe2afd36da781454626345ea87dedae14643

commit 1819fe2afd36da781454626345ea87dedae14643
Author: Michael Achenbach <machenbach@chromium.org>
Date: Wed May 03 14:10:02 2017

Revert "zip_build.py: Exclude secondary toolchain obj and gen, as well as .ninja"

This reverts commit 1c881be4fb63733820a29955a7e0772f1898921e.

Reason for revert: Makes all chromium builders purple, example:
https://build.chromium.org/p/chromium.win/builders/Win%20Builder/builds/39078

Original change's description:
> zip_build.py: Exclude secondary toolchain obj and gen, as well as .ninja
> 
> BUG= 681689 
> 
> Change-Id: I91ffb62cb05144d1198e95347166528ece778390
> Reviewed-on: https://chromium-review.googlesource.com/491806
> Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> Reviewed-by: Stephen Martinis <martiniss@chromium.org>
> 

TBR=agrieve@chromium.org,martiniss@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 681689 

Change-Id: Iebe43a7071497bbab043415aaa41d90f738d9c09
Reviewed-on: https://chromium-review.googlesource.com/494589
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/1819fe2afd36da781454626345ea87dedae14643/scripts/slave/zip_build.py

Status: Started (was: Fixed)
Project Member

Comment 12 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/f956104a4ef19f1d93cf51de2fb66f858e2f62be

commit f956104a4ef19f1d93cf51de2fb66f858e2f62be
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed May 03 18:33:45 2017

Reland: zip_build.py: Exclude secondary toolchain obj and gen, as well as .ninja

Previously reverted in 1819fe2afd36da781454626345ea87dedae14643

Reason for reland:
Hopefully fixed up logic repro'ed failure locally.

BUG= 681689 

Change-Id: I4459805add7d0f61220225f315d3224314bf6e4b
Reviewed-on: https://chromium-review.googlesource.com/493905
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>

[modify] https://crrev.com/f956104a4ef19f1d93cf51de2fb66f858e2f62be/scripts/slave/zip_build.py

Status: Fixed (was: Started)
Project Member

Comment 14 by bugdroid1@chromium.org, May 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/745e8d9fbc8344d522a84131ffa9b43472dbb06d

commit 745e8d9fbc8344d522a84131ffa9b43472dbb06d
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu May 18 18:29:30 2017

Revert "Reland: zip_build.py: Exclude secondary toolchain obj and gen, as well as .ninja"

This reverts commit f956104a4ef19f1d93cf51de2fb66f858e2f62be.

Reason for revert:
Likely cause of broken manual bisects (crbug/723921)

Original change's description:
> Reland: zip_build.py: Exclude secondary toolchain obj and gen, as well as .ninja
> 
> Previously reverted in 1819fe2afd36da781454626345ea87dedae14643
> 
> Reason for reland:
> Hopefully fixed up logic repro'ed failure locally.
> 
> BUG= 681689 
> 
> Change-Id: I4459805add7d0f61220225f315d3224314bf6e4b
> Reviewed-on: https://chromium-review.googlesource.com/493905
> Reviewed-by: Stephen Martinis <martiniss@chromium.org>
> Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> 

TBR=machenbach@chromium.org,agrieve@chromium.org,martiniss@chromium.org,chromium-reviews@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
BUG= 681689 , 723921 

Change-Id: Ic71ae2847e3166a2bd06ddfbbdbd223adb737d90
Reviewed-on: https://chromium-review.googlesource.com/508127
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>

[modify] https://crrev.com/745e8d9fbc8344d522a84131ffa9b43472dbb06d/scripts/slave/zip_build.py

Sign in to add a comment