[CT] Add ability to run unlanded benchmarks on Chromium Perf

[CT] Add ability to run unlanded benchmarks on Chromium Perf.

Summary of changes:
* Adds a new "custom" option when selecting benchmarks. Screenshot: https://screenshot.googleplex.com/wbrvya5hBzb.png
* When the "custom" option is selected a new text field opens up. Screenshot: https://screenshot.googleplex.com/hH6tt921ZtF.png
* Adds a new section for entering a telemetry patch. Screenshot: https://screenshot.googleplex.com/TUP3KQ6ii3R.png

The purpose of the above changes is to allow users to test out an unsubmitted benchmark by using Cluster Telemetry. It will also allow users to make tweaks to existing benchmarks.

BUG=skia:4173
Committed: https://skia.googlesource.com/buildbot/+/eb3b631a2acb43eaf4738c3cb33e1fa2b6599b4e

[rmistry, 2015-10-20 13:42:03]

Description was changed from

==========
[CT] Add ability to run unlanded benchmarks on Chromium Perf

BUG=skia:4173
==========

to

==========
[CT] Add ability to run unlanded benchmarks on Chromium Perf.

Summary of changes:
* Adds a new "custom" option when selecting benchmarks.
* When the "custom" option is selected a new text field opens up.
* Adds a new section for entering a telemetry patch.

The purpose of the above changes is to allow users to test out an unsubmitted
benchmark by using Cluster Telemetry. It will also allow users to make tweaks to
existing benchmarks.

BUG=skia:4173
==========

[rmistry, 2015-10-20 13:47:19]

Description was changed from

==========
[CT] Add ability to run unlanded benchmarks on Chromium Perf.

Summary of changes:
* Adds a new "custom" option when selecting benchmarks.
* When the "custom" option is selected a new text field opens up.
* Adds a new section for entering a telemetry patch.

The purpose of the above changes is to allow users to test out an unsubmitted
benchmark by using Cluster Telemetry. It will also allow users to make tweaks to
existing benchmarks.

BUG=skia:4173
==========

to

==========
[CT] Add ability to run unlanded benchmarks on Chromium Perf.

Summary of changes:
* Adds a new "custom" option when selecting benchmarks. Screenshot:
https://screenshot.googleplex.com/wbrvya5hBzb.png
* When the "custom" option is selected a new text field opens up. Screenshot:
https://screenshot.googleplex.com/hH6tt921ZtF.png
* Adds a new section for entering a telemetry patch. Screenshot:
https://screenshot.googleplex.com/TUP3KQ6ii3R.png

The purpose of the above changes is to allow users to test out an unsubmitted
benchmark by using Cluster Telemetry. It will also allow users to make tweaks to
existing benchmarks.

BUG=skia:4173
==========

[rmistry, 2015-10-20 13:49:47]

rmistry@google.com changed reviewers:
+ benjaminwagner@google.com

[dogben, 2015-10-20 13:54:15]

On 2015/10/20 at 13:49:48, rmistry wrote:
> 

Initial question about this: How is the additional patch applied? I don't see
any change to ct/go/util/chromium_builds.go. Does it work OK to apply two
different patches to the Chromium checkout?

[rmistry, 2015-10-20 13:56:14]

On 2015/10/20 13:54:15, Ben Wagner wrote:
> On 2015/10/20 at 13:49:48, rmistry wrote:
> > 
> 
> Initial question about this: How is the additional patch applied? I don't see
> any change to ct/go/util/chromium_builds.go. Does it work OK to apply two
> different patches to the Chromium checkout?

The Chromium / Skia patches in chromium_builds.go are applied on the Chromium
build checkout on the master, after that the build is created and stored in
Google Storage for the slaves to download.
This new benchmark_patch is instead applied on the Chromium checkout on the
slaves. The slaves use that Chromium checkout to run telemetry binaries from, by
applying the patch there it enables the slaves to run benchmarks which are not
checked in yet.

[dogben, 2015-10-20 15:17:26]

https://codereview.chromium.org/1411423003/diff/180001/ct/go/db/db.go
File ct/go/db/db.go (right):

https://codereview.chromium.org/1411423003/diff/180001/ct/go/db/db.go#newcode71
ct/go/db/db.go:71: benchmark_patch        TEXT,
I don't think this is supposed to be here.

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
File ct/go/worker_scripts/run_chromium_perf/main.go (right):

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
ct/go/worker_scripts/run_chromium_perf/main.go:129: out, err :=
os.Create(benchmarkPatchLocalPath)
Seems like you could use os.WriteFile.
https://golang.org/pkg/io/ioutil/#WriteFile

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
ct/go/worker_scripts/run_chromium_perf/main.go:141: benchmarkPatchFile, _ :=
os.Open(benchmarkPatchLocalPath)
Use os.Stat. https://golang.org/pkg/os/#Stat
(Or defer skutil.Close(benchmarkPatchFile))

https://codereview.chromium.org/1411423003/diff/180001/ct/res/imp/chromium-pe...
File ct/res/imp/chromium-perf-sk.html (right):

https://codereview.chromium.org/1411423003/diff/180001/ct/res/imp/chromium-pe...
ct/res/imp/chromium-perf-sk.html:181: Documentation is <a
href="https://docs.google.com/document/d/1GhqosQcwsy6F-eBAmFn_ITDF7_Iv_rY9FhCKwAnk9qQ/edit#heading=h.i2mgtio5tori">here</a>
This link points to the wrong section.

https://codereview.chromium.org/1411423003/diff/180001/ct/res/imp/chromium-pe...
ct/res/imp/chromium-perf-sk.html:337: str += " and ";
Nit: maybe " with "? I think "with" vs "and" depends if people will be using
this to test their benchmark or if they'll make tweaks to the benchmark when
testing their main CL.

https://codereview.chromium.org/1411423003/diff/180001/ct/res/imp/chromium-pe...
ct/res/imp/chromium-perf-sk.html:352: if (! this.$.desc.value) {
Maybe check here that getBenchmarkName returns non-empty?

[rmistry, 2015-10-20 17:47:48]

https://codereview.chromium.org/1411423003/diff/180001/ct/go/db/db.go
File ct/go/db/db.go (right):

https://codereview.chromium.org/1411423003/diff/180001/ct/go/db/db.go#newcode71
ct/go/db/db.go:71: benchmark_patch        TEXT,
On 2015/10/20 15:17:26, Ben Wagner wrote:
> I don't think this is supposed to be here.

Oops. Removed.

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
File ct/go/worker_scripts/run_chromium_perf/main.go (right):

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
ct/go/worker_scripts/run_chromium_perf/main.go:129: out, err :=
os.Create(benchmarkPatchLocalPath)
On 2015/10/20 15:17:26, Ben Wagner wrote:
> Seems like you could use os.WriteFile.
> https://golang.org/pkg/io/ioutil/#WriteFile

Done.

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
ct/go/worker_scripts/run_chromium_perf/main.go:141: benchmarkPatchFile, _ :=
os.Open(benchmarkPatchLocalPath)
On 2015/10/20 15:17:26, Ben Wagner wrote:
> Use os.Stat. https://golang.org/pkg/os/#Stat
> (Or defer skutil.Close(benchmarkPatchFile))

Done.

https://codereview.chromium.org/1411423003/diff/180001/ct/res/imp/chromium-pe...
File ct/res/imp/chromium-perf-sk.html (right):

https://codereview.chromium.org/1411423003/diff/180001/ct/res/imp/chromium-pe...
ct/res/imp/chromium-perf-sk.html:181: Documentation is <a
href="https://docs.google.com/document/d/1GhqosQcwsy6F-eBAmFn_ITDF7_Iv_rY9FhCKwAnk9qQ/edit#heading=h.i2mgtio5tori">here</a>
On 2015/10/20 15:17:26, Ben Wagner wrote:
> This link points to the wrong section.

Done.

https://codereview.chromium.org/1411423003/diff/180001/ct/res/imp/chromium-pe...
ct/res/imp/chromium-perf-sk.html:337: str += " and ";
On 2015/10/20 15:17:26, Ben Wagner wrote:
> Nit: maybe " with "? I think "with" vs "and" depends if people will be using
> this to test their benchmark or if they'll make tweaks to the benchmark when
> testing their main CL.

Makes sense. Done.

https://codereview.chromium.org/1411423003/diff/180001/ct/res/imp/chromium-pe...
ct/res/imp/chromium-perf-sk.html:352: if (! this.$.desc.value) {
On 2015/10/20 15:17:26, Ben Wagner wrote:
> Maybe check here that getBenchmarkName returns non-empty?

Done.

[dogben, 2015-10-20 18:14:08]

lgtm

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
File ct/go/worker_scripts/run_chromium_perf/main.go (right):

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
ct/go/worker_scripts/run_chromium_perf/main.go:141: benchmarkPatchFile, _ :=
os.Open(benchmarkPatchLocalPath)
On 2015/10/20 17:47:48, rmistry wrote:
> On 2015/10/20 15:17:26, Ben Wagner wrote:
> > Use os.Stat. https://golang.org/pkg/os/#Stat
> > (Or defer skutil.Close(benchmarkPatchFile))
> 
> Done.

Nit: You actually have the size from buf above, so Stat is not really needed.

[rmistry, 2015-10-21 12:32:47]

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
File ct/go/worker_scripts/run_chromium_perf/main.go (right):

https://codereview.chromium.org/1411423003/diff/180001/ct/go/worker_scripts/r...
ct/go/worker_scripts/run_chromium_perf/main.go:141: benchmarkPatchFile, _ :=
os.Open(benchmarkPatchLocalPath)
On 2015/10/20 18:14:08, Ben Wagner wrote:
> On 2015/10/20 17:47:48, rmistry wrote:
> > On 2015/10/20 15:17:26, Ben Wagner wrote:
> > > Use os.Stat. https://golang.org/pkg/os/#Stat
> > > (Or defer skutil.Close(benchmarkPatchFile))
> > 
> > Done.
> 
> Nit: You actually have the size from buf above, so Stat is not really needed.

Done.

[rmistry, 2015-10-21 12:33:22]

The CQ bit was checked by rmistry@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-21 12:33:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1411423003/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1411423003/220001

[commit-bot: I haz the power, 2015-10-21 12:36:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-21 12:36:23]

Dry run: Try jobs failed on following builders:
  Infra-PerCommit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/Infra-PerCommit-Trybot/b...)

[rmistry, 2015-10-21 12:40:48]

The CQ bit was checked by rmistry@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-21 12:40:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1411423003/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1411423003/240001

[commit-bot: I haz the power, 2015-10-21 12:42:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-21 12:42:24]

Dry run: This issue passed the CQ dry run.

[rmistry, 2015-10-21 12:43:18]

The CQ bit was checked by rmistry@google.com

[rmistry, 2015-10-21 12:43:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
benjaminwagner@google.com
Link to the patchset: https://codereview.chromium.org/1411423003/#ps240001
(title: "Fix errcheck")

[commit-bot: I haz the power, 2015-10-21 12:43:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1411423003/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1411423003/240001

[commit-bot: I haz the power, 2015-10-21 12:43:34]

Committed patchset #13 (id:240001) as
https://skia.googlesource.com/buildbot/+/eb3b631a2acb43eaf4738c3cb33e1fa2b659...

[dogben, 2015-10-21 13:10:57]

lgtm