1251713 - taskcluster linux64 opt builds do not run generate-build-stats step or update step

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

I am trying to verify some perfherder metrics in the logs of builds and I see them on buildbot jobs:
http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-f0a25b700170978777a0578c4d7dcf57577fa2d7/try-linux64/try-linux64-bm76-try1-build10568.txt.gz

but not on taskcluster jobs:
https://public-artifacts.taskcluster.net/YJOo7r8hRyGqMRFn85PVLQ/0/public/logs/live_backing.log

the reason why is:
19:42:13     INFO - Running post-action listener: influxdb_recording_post_action
19:42:13     INFO - #####
19:42:13     INFO - ##### Skipping package-source step.
19:42:13     INFO - #####
19:42:13     INFO - #####
19:42:13     INFO - ##### Skipping generate-source-signing-manifest step.
19:42:13     INFO - #####
19:42:13     INFO - #####
19:42:13     INFO - ##### Skipping multi-l10n step.
19:42:13     INFO - #####
19:42:13     INFO - #####
19:42:13     INFO - ##### Skipping generate-build-stats step.
19:42:13     INFO - #####
19:42:13     INFO - #####
19:42:13     INFO - ##### Skipping update step.
19:42:13     INFO - #####

in buildbot we run generate-build-stats (what I am looking for) and we also run the update step.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

:mrrrgn, is this something that you had done in:
https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/builder/build-linux.sh#128

maybe you know some history here or who might be able to help me uncover this mystery?

Comment 2

[Morgan Phillips [:mrrrgn]]

(In reply to Joel Maher (:jmaher) from comment #1)
> :mrrrgn, is this something that you had done in:
> https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/scripts/
> builder/build-linux.sh#128
> 
> maybe you know some history here or who might be able to help me uncover
> this mystery?

dustin would have the best idea. We'd disabled nearly every step initially with the idea of re-enabling as needed.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

thanks :mrrrgn!  onto :dustin for more info

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

removing " --no-action=generate-build-stats" solves one problem, let me push with also removing:
  --no-update \
  --no-upload-files \

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

that seemed to fail in upload files:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1aba4300bc7

i would like to get more history before hacking too much!

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

What Morgan said :)

I'm glad the build-stats stuff works out of the box.

For the updates, we'll need to figure out what Mozharness is trying to do in Buildbot land and then find a way to do the same thing under TaskCluster, without breaking Buildbot.

Why did you remove --no-upload-files in addition to --no-update?

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

I really don't know what those options are- the build is a mystery to me.  It sounds like this bug should be blocking making builds tier1 then- at least to determine what is missing.  count_ctors is opt/pgo only, but we do record libxul size from debug builds and that isn't showing up in tc builds.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

removing just the --no-update change works fine:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=342f96ff1d34

I was looking for the libxul.so size which is in the upload step:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/building/buildbase.py#1659


make check seems to be running, so we are just missing a few small things.

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

to clarify with opt/debug issues:
debug: libxul.so size check from the "upload-files" step
opt: num_ctors is missing from "generate-build-stats" step, in addition to libxul.so from "upload-files" step.

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Once bug 1231320 lands, we can easily add steps.  So --generate-build-stats is straightforward.

If --upload-files is checking the size of a file on disk, then I'd say that check is in the wrong step and needs to be broken out.  Is it reasonable to do that in the --generate-build-stats step?  (:wlach because hg annotate says he is the author of that section of buildbase.py)

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

chmanchester might be interested in this and have some insight.

Comment 12

[William Lachance (:wlach)]

:chmanchester is actually the right person to ask here, I'm just the perfherder guy. :)

Comment 13

[Chris Manchester (limited bugmail, email directly)]

Putting the size checks in generate_build_stats certainly sounds reasonable to me. That's after the build, so all the files should be in their final locations.

Comment 14

[Dustin J. Mitchell [:dustin] (he/him)]

The update step does nothing except on nightlies, where it submits a balrog update.  We're not doing nightlies in TC yet.  When we do, the balrog updates will be done in another, dependent task.  Nothing to do there.

So the things we know are missing are:

 - generate-build-stats action, which invokes count_ctors.py
 - libxul.so size (and probably other sizes), as they are generated in the skipped upload step; will move these to generate-build-stats (affecting both BB and TC builds)

Did I miss anything?

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

this seems right to me, thanks for reiterating.

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a539d02bbbe

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

17:07:04     INFO - #####
17:07:04     INFO - ##### Running generate-build-stats step.
17:07:04     INFO - #####
17:07:04     INFO - Running pre-action listener: influxdb_recording_pre_action
17:07:04     INFO - Running main action method: generate_build_stats
17:07:04     INFO - counting ctors...
17:07:04     INFO - Getting output from command: ['/home/worker/workspace/build/venv/bin/python', '/home/worker/workspace/build/src/build/util/count_ctors.py', '/home/worker/workspace/build/src/obj-firefox/dist/bin/libxul.so'] in /home/worker/workspace/build/src
17:07:04     INFO - Copy/paste: /home/worker/workspace/build/venv/bin/python /home/worker/workspace/build/src/build/util/count_ctors.py /home/worker/workspace/build/src/obj-firefox/dist/bin/libxul.so
17:07:04    DEBUG - Temporary files: tmpfile_stdout and tmpfile_stderr
17:07:04     INFO - Reading from file tmpfile_stdout
17:07:04     INFO - Output received:
17:07:04     INFO -  PERFHERDER_DATA: {"framework": {"name": "build_metrics"}, "suites": [{"subtests": [{"name": "num_constructors", "value": 91}], "name": "compiler_metrics"}]}
17:07:04    DEBUG - rmtree: tmpfile_stderr
17:07:04    DEBUG - retry: Calling remove with args: ('tmpfile_stderr',), kwargs: {}, attempt #1
17:07:04    DEBUG - rmtree: tmpfile_stdout
17:07:04    DEBUG - retry: Calling remove with args: ('tmpfile_stdout',), kwargs: {}, attempt #1
17:07:04    DEBUG - Return code: 0
17:07:04     INFO - Running post-action listener: influxdb_recording_post_action

so that's generate-build-stats.

The sizes are a little harder since TC doesn't supply the packageFilename buildbot property (from factory.py in buildbotcustom), although it looks like that would be incorrect in TaskCluster anyway, as we build an installer named target.tar.bz2.

Comment 18

[Dustin J. Mitchell [:dustin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2674b4f5bad

Comment 19

[Dustin J. Mitchell [:dustin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cab3f5dc3b08

Comment 20

[Dustin J. Mitchell [:dustin] (he/him)]

File sizes worked for TC.. not sure about buildbot, which is still running.  Still, this is enough for review, I think.

19:35:37     INFO - #####
19:35:37     INFO - #####
19:35:37     INFO - ##### Running generate-build-stats step.
19:35:37     INFO - #####
19:35:37     INFO - Running pre-action listener: influxdb_recording_pre_action
19:35:37     INFO - Running main action method: generate_build_stats
19:35:37     INFO - counting ctors...
19:35:37     INFO - Getting output from command: ['/home/worker/workspace/build/venv/bin/python', '/home/worker/workspace/build/src/build/util/count_ctors.py', '/home/worker/workspace/build/src/obj-firefox/dist/bin/libxul.so'] in /home/worker/workspace/build/src
19:35:37     INFO - Copy/paste: /home/worker/workspace/build/venv/bin/python /home/worker/workspace/build/src/build/util/count_ctors.py /home/worker/workspace/build/src/obj-firefox/dist/bin/libxul.so
19:35:37    DEBUG - Temporary files: tmpfile_stdout and tmpfile_stderr
19:35:38     INFO - Reading from file tmpfile_stdout
19:35:38     INFO - Output received:
19:35:38     INFO -  PERFHERDER_DATA: {"framework": {"name": "build_metrics"}, "suites": [{"subtests": [{"name": "num_constructors", "value": 91}], "name": "compiler_metrics"}]}
19:35:38    DEBUG - rmtree: tmpfile_stderr
19:35:38    DEBUG - retry: Calling remove with args: ('tmpfile_stderr',), kwargs: {}, attempt #1
19:35:38    DEBUG - rmtree: tmpfile_stdout
19:35:38    DEBUG - retry: Calling remove with args: ('tmpfile_stdout',), kwargs: {}, attempt #1
19:35:38    DEBUG - Return code: 0
19:35:38     INFO - Determining filesize for /home/worker/workspace/build/src/obj-firefox/dist/target.tar.bz2
19:35:38     INFO -  54084644
19:35:38     INFO - TinderboxPrint: Size of target.tar.bz2<br/>54084644 bytes
19:35:38     INFO - Determining filesize for /home/worker/workspace/build/src/obj-firefox/dist/firefox/libxul.so
19:35:38     INFO -  102766834
19:35:38     INFO - TinderboxPrint: Size of libxul.so<br/>102766834 bytes
19:35:38     INFO - PERFHERDER_DATA: {"framework": {"name": "build_metrics"}, "suites": [{"subtests": [{"name": "libxul.so", "value": 102766834}], "name": "installer size", "value": 54084644}, {"subtests": [{"name": "pre-export", "value": 4.494840145111084}, {"name": "export", "value": 34.27146887779236}, {"name": "compile", "value": 1551.3533749580383}, {"name": "misc", "value": 6.80957293510437}, {"name": "libs", "value": 15.59085488319397}, {"name": "tools", "value": 0.28151988983154297}, {"name": "package-tests", "value": 93.1786470413208}, {"name": "buildsymbols", "value": 295.5652439594269}, {"name": "package", "value": 43.29067897796631}, {"name": "l10n-check", "value": 49.26740384101868}, {"name": "upload", "value": 9.214303970336914}], "name": "build times", "value": 2145.0504870414734}]}
19:35:38     INFO - Running post-action listener: influxdb_recording_post_action

Comment 21

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: MozReview Request: Bug 1251713: run generate-build-stats on all linux builds; r?jmaher

Review commit: https://reviewboard.mozilla.org/r/41103/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41103/

Comment 22

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: MozReview Request: Bug 1251713: include file sizes perfs in generate-build-stats; r?chmanchester

This requires determining the package filename dynamically when buildbot
properties are not available.

Review commit: https://reviewboard.mozilla.org/r/41105/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41105/

Comment 23

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8732335 [details]
MozReview Request: Bug 1251713: run generate-build-stats on all linux builds; r?jmaher

https://reviewboard.mozilla.org/r/41103/#review37711

these changes look fine!

Comment 24

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8732336 [details]
MozReview Request: Bug 1251713: include file sizes perfs in generate-build-stats; r?chmanchester

https://reviewboard.mozilla.org/r/41105/#review37717

::: testing/mozharness/mozharness/mozilla/building/buildbase.py:1885
(Diff revision 1)
> +        # if packageName is not set because we are not running in Buildbot,
> +        # then assume we are using MOZ_SIMPLE_PACKAGE_NAME, which means the
> +        # package is named one of target.{tar.bz2,zip,dmg}.
> +        if not packageName:
> +            dist_dir = os.path.join(dirs['abs_obj_dir'], 'dist')
> +            for ext in ['dmg', 'tar.bz2', 'zip']:

Should we start with 'apk' in this list, so Android doesn't fall behind. Alternately, can we pass the package name down from TC? Mostly just curious, I think this is unlikely to fail as is.

Comment 25

[Dustin J. Mitchell [:dustin] (he/him)]

https://reviewboard.mozilla.org/r/41105/#review37717

> Should we start with 'apk' in this list, so Android doesn't fall behind. Alternately, can we pass the package name down from TC? Mostly just curious, I think this is unlikely to fail as is.

Yes, total oversight on my part (I was copying the list from the extensions used to recognize an installer a few dozen lines later).  We could pass the package name from TC, but I think the better final solution will be to include it in the mozharness config.  However, we can't do that until we stop doign builds on buildbot, which doesn't use MOZ_SIMPLE_PACKAGE_NAMES and thus doesn't have stable package names.

Comment 26

[Dustin J. Mitchell [:dustin] (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a9229d5d5486d05bc5a6ad3f1a0cb33ec08701e
Bug 1251713: run generate-build-stats on all linux builds; r=jmaher

https://hg.mozilla.org/integration/mozilla-inbound/rev/a4494f43475d52282aed1532f1defb14685b6df8
Bug 1251713: include file sizes perfs in generate-build-stats; r=chmanchester

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2a9229d5d548
https://hg.mozilla.org/mozilla-central/rev/a4494f43475d