Closed Bug 1251713 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox48 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla48
Tracking Flags:
Tracking Status
firefox48 --- fixed

People

(Reporter: jmaher, Assigned: dustin)

References

Details

Attachments

(2 files)

MozReview Request: Bug 1251713: run generate-build-stats on all linux builds; r?jmaher
58 bytes, text/x-review-board-request
jmaher
: review+
Details
MozReview Request: Bug 1251713: include file sizes perfs in generate-build-stats; r?chmanchester
58 bytes, text/x-review-board-request
chmanchester
: review+
Details 
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/jmaher@mozilla.com-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.
Blocks: 1156885
: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?
Flags: needinfo?(winter2718)
(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.
Flags: needinfo?(winter2718)
thanks :mrrrgn!  onto :dustin for more info
Flags: needinfo?(dustin)
removing " --no-action=generate-build-stats" solves one problem, let me push with also removing:
  --no-update \
  --no-upload-files \
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!
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?
Flags: needinfo?(dustin)
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.
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.
Assignee: nobody → dustin
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.
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)
Flags: needinfo?(wlachance)
chmanchester might be interested in this and have some insight.
:chmanchester is actually the right person to ask here, I'm just the perfherder guy. :)
Flags: needinfo?(wlachance) → needinfo?(cmanchester)
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.
Flags: needinfo?(cmanchester)
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?
this seems right to me, thanks for reiterating.
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.
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
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/
Attachment #8732336 - Flags: review?(cmanchester)
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!
Attachment #8732335 - Flags: review?(jmaher) → review+
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.
Attachment #8732336 - Flags: review?(cmanchester) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/2a9229d5d548
https://hg.mozilla.org/mozilla-central/rev/a4494f43475d
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Depends on: 1261081
Component: Build Config → General
Product: Firefox → Firefox Build System
Target Milestone: Firefox 48 → mozilla48
You need to log in before you can comment on or make changes to this bug.