First version of the PGO recipe.

scripts/slave/recipes/chromium_pgo.py

+# Copyright 2014 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+DEPS = [
+  'bot_update',
+  'chromium',
+  'gclient',
+  'path',
+  'platform',
+  'properties',
+  'python',
+  'step',
+  'step_history',
+]
+
+
+# List of the benchmark that we run during the profiling step.
+_BENCHMARKS_TO_RUN = {
+  'peacekeeper.dom',
+  'peacekeeper.array',
+  'peacekeeper.html5',
+  'peacekeeper.string',
+  'peacekeeper.render',
+  'dromaeo.domcoreattr',
+  'dromaeo.domcoremodify',
+  'dromaeo.domcorequery',
+  'dromaeo.domcoretraverse',
+  'dromaeo.jslibattrjquery',
+  'dromaeo.jslibattrprototype',
+  'dromaeo.jslibeventjquery',
+  'dromaeo.jslibeventprototype',
+  'dromaeo.jslibmodifyjquery',
+  'dromaeo.jslibmodifyprototype',
+  'dromaeo.jslibstylejquery',
+  'dromaeo.jslibstyleprototype',
+  'dromaeo.jslibtraversejquery',
+  'dromaeo.jslibtraverseprototype',
+  'sunspider',
+  'jsgamebench',
+  'kraken',
+}
+
+
+# Run a telemetry benchmark under the Windows PGO profiler.
+def RunTelemetryBenchmark(api, testname):
+  args = [
+    '--profiler=win_pgo_profiler',
+    '--use-live-sites',
+    testname,
+  ]
+  return api.python(
+      'Telemetry benchmark: %s' % testname,
+      api.path['checkout'].join('tools',
+                                'perf',
+                                'run_benchmark'),
+      args)

[iannucci, 2014/06/19 21:37:17]

personally, I would format this like

return api.python(
    'Telemetry benchmark: %s' % testname,
    api.path['checkout'].join('tools', 'perf', 'run_benchmark'), 
    ['--profiler=win_pgo_profiler', '--use-live-sites', testname]
)

?

[Sébastien Marchand, 2014/07/03 19:13:17]

Shorter and more readable, I like that.

+
+
+def GenSteps(api):
+  api.step.auto_resolve_conflicts = True
+  api.gclient.set_config('chromium_lkgr')
+  api.chromium.set_config('chromium', 'Release')

[iannucci, 2014/06/19 21:37:17]

flip the order of these two lines. Since 'chromium' depends on 'gclient', this
second one will end up setting the config 'chromium' on the gclient module (no,
it doesn't make a lot of sense, yes, it needs to be redesigned).

Also, you need to use kwargs for additional variables in set_config, like
BUILD_CONFIG='Release'

[Sébastien Marchand, 2014/07/03 19:13:17]

Done.

+
+  yield api.chromium.taskkill()
+  yield api.bot_update.ensure_checkout(),

[iannucci, 2014/06/19 21:37:17]

you'll want to remove these trailing commas

[Sébastien Marchand, 2014/07/03 19:13:17]

Oops, thanks.

+  if not api.step_history.last_step().json.output['did_run']:

[iannucci, 2014/06/19 21:37:17]

we can just assert that this always uses bot_update. No reason to add a new
recipe which doesn't.

+    yield api.gclient.checkout(),
+
+  # First step: compilation of the instrumented build.
+  api.chromium.apply_config('chrome_pgo_instrument')

[iannucci, 2014/06/19 21:37:17]

you should probably just set this on line 63

[Sébastien Marchand, 2014/07/03 19:13:17]

Done.

+  yield api.chromium.runhooks()
+  yield api.chromium.compile(targets=['chrome'])

[iannucci, 2014/06/19 21:37:17]

targets won't be necessary since you set it in the config already.

[Sébastien Marchand, 2014/07/03 19:13:17]

Done.

+
+  # Remove the profile files from the previous builds.
+  yield api.path.rmcontents('previous profile files',
+                            api.chromium.c.build_dir,

[iannucci, 2014/06/19 21:37:17]

I'm wondering if it wouldn't be better to just have the path be a glob-ish, e.g.
api.chromium.c.build_dir.join('*.pgc')

Then rmcontents could just append a '*' to the end of argv[1] if argv[1] doesn't
have any *'s in it, and always use glob.glob. One less code path.

[Sébastien Marchand, 2014/07/03 19:13:17]

Done.Looks like there's a rmwildcard function now !

+                            '*.pgc')
+
+  # Second step: profiling of the instrumented build.
+  for benchmark in _BENCHMARKS_TO_RUN:
+    yield RunTelemetryBenchmark(api, benchmark)
+
+  # Third step: Compilation of the optimized build, this will use the profile
+  #     data files produced by the previous step.
+  api.chromium.apply_config('chrome_pgo_optimize')

[iannucci, 2014/06/19 21:37:17]

you'll want 'set_config' here, also will need to pass in BUILD_CONFIG='Release'

[Sébastien Marchand, 2014/07/03 19:13:17]

Done.

+  yield api.chromium.runhooks()

[iannucci, 2014/06/19 21:37:17]

do we really need to run hooks again? that's sad :(

[Sébastien Marchand, 2014/07/03 19:13:17]

Yep, we have to regenerate the build (.ninja) files as the compiler options
change...

+  yield api.chromium.compile(targets=['chrome'])

[iannucci, 2014/06/19 21:37:17]

same re: targets

[Sébastien Marchand, 2014/07/03 19:13:17]

Done.

+
+
+def GenTests(api):

[iannucci, 2014/06/19 21:37:17]

it would be good to train the tests and include the expectations in the CL.

./scripts/slave/unittests/recipe_simulation_test.py train chromium_pgo

+  mastername = 'chromium.fyi'
+  buildername = 'Chromium Win PGO Builder'
+
+  def _sanitize_nonalpha(text):
+    return ''.join(c if c.isalnum() else '_' for c in text)
+
+  yield (
+    api.test('full_%s_%s' % (_sanitize_nonalpha(mastername),
+                             _sanitize_nonalpha(buildername))) +
+    api.properties.generic(mastername=mastername, buildername=buildername) +
+    api.platform('win', 32)
+  )