Make results_dashboard send just one request per test run.

Make results_dashboard send just one request per test run.

The PerformanceLogs() method of the log processor classes returns a dictionary mapping "log file" names to lists of lines. Each one of those log files corresponds to one chart name.

Previously, we were invoking _SendResultsToDashboard for each "log file"; that is, each chart name.

With this change, we are now passing the whole dict returned by PerformanceLogs() and making just one request for all points in all charts for a given test run.

BUG=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=260698

[ghost stip (do not use), 2014-03-31 18:10:48]

generally lgtm, with a warning. I don't know if this will appear in practice or
not (if we don't have very large payloads, then no), but it's useful to have a
chunking mechanism to give a hard limit.

https://chromiumcodereview.appspot.com/217053012/diff/1/scripts/slave/results...
File scripts/slave/results_dashboard.py (right):

https://chromiumcodereview.appspot.com/217053012/diff/1/scripts/slave/results...
scripts/slave/results_dashboard.py:145: for logname, log in
logs_dict.iteritems():
we should have some kind of limit here, otherwise if a test uploads too many
datapoints we'll exceed appengine's limits and spit out 500s.

[qyearsley, 2014-03-31 20:31:33]

On 2014/03/31 18:10:48, stip wrote:
> generally lgtm, with a warning. I don't know if this will appear in practice
or
> not (if we don't have very large payloads, then no), but it's useful to have a
> chunking mechanism to give a hard limit.
> 
>
https://chromiumcodereview.appspot.com/217053012/diff/1/scripts/slave/results...
> File scripts/slave/results_dashboard.py (right):
> 
>
https://chromiumcodereview.appspot.com/217053012/diff/1/scripts/slave/results...
> scripts/slave/results_dashboard.py:145: for logname, log in
> logs_dict.iteritems():
> we should have some kind of limit here, otherwise if a test uploads too many
> datapoints we'll exceed appengine's limits and spit out 500s.

So, I just did a little experiment to see when we might exceed app engine's
limits:

Sending 16 points... status 200 in 0.52 s
Sending 32 points... status 200 in 0.79 s
Sending 64 points... status 200 in 1.1 s
Sending 128 points... status 200 in 2.01 s
Sending 256 points... status 200 in 3.47 s
Sending 512 points... status 200 in 7.4 s
Sending 1024 points... status 200 in 14.83 s
Sending 2048 points... status 200 in 42.8 s
Sending 4096 points... status 500 in 61.98 s
s~google.com:chrome-perf : uncaught application failure

(I repeated it a couple times and the results were basically the same.)

So, I think it's probably safe to send up to 1000 points. If you send between
2000 and 4000 points, it might take up to 60 seconds and time out.

What might be a good good cut-off? 256 points or 512 points?

[qyearsley, 2014-03-31 21:21:04]

On 2014/03/31 20:31:33, qyearsley wrote:
> On 2014/03/31 18:10:48, stip wrote:
> > generally lgtm, with a warning. I don't know if this will appear in practice
> or
> > not (if we don't have very large payloads, then no), but it's useful to have
a
> > chunking mechanism to give a hard limit.
> > 
> >
>
https://chromiumcodereview.appspot.com/217053012/diff/1/scripts/slave/results...
> > File scripts/slave/results_dashboard.py (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/217053012/diff/1/scripts/slave/results...
> > scripts/slave/results_dashboard.py:145: for logname, log in
> > logs_dict.iteritems():
> > we should have some kind of limit here, otherwise if a test uploads too many
> > datapoints we'll exceed appengine's limits and spit out 500s.
> 
> So, I just did a little experiment to see when we might exceed app engine's
> limits:
> 
> Sending 16 points... status 200 in 0.52 s
> Sending 32 points... status 200 in 0.79 s
> Sending 64 points... status 200 in 1.1 s
> Sending 128 points... status 200 in 2.01 s
> Sending 256 points... status 200 in 3.47 s
> Sending 512 points... status 200 in 7.4 s
> Sending 1024 points... status 200 in 14.83 s
> Sending 2048 points... status 200 in 42.8 s
> Sending 4096 points... status 500 in 61.98 s
> s~google.com:chrome-perf : uncaught application failure
> 
> (I repeated it a couple times and the results were basically the same.)
> 
> So, I think it's probably safe to send up to 1000 points. If you send between
> 2000 and 4000 points, it might take up to 60 seconds and time out.
> 
> What might be a good good cut-off? 256 points or 512 points?

Now added some grouping so that a maximum of 500 points are sent at once. Any
more suggestions?

[ghost stip (do not use), 2014-03-31 23:00:59]

nope, lgtm. nice!

[qyearsley, 2014-03-31 23:22:54]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2014-03-31 23:23:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qyearsley@chromium.org/217053012/20001

[commit-bot: I haz the power, 2014-03-31 23:24:19]

Change committed as 260698

[rlarocque, 2014-04-01 00:15:45]

A revert of this CL has been created in
https://codereview.chromium.org/219773006/ by rlarocque@chromium.org.

The reason for reverting is: I think this change is responsible for some
buildbot failures.

http://build.chromium.org/p/chromium.gpu/builders/Mac%2010.8%20Release%20%28A...

@@@STEP_TEXT@tab_capture_performance_tests@@@
@@@STEP_TEXT@<div class="BuildResultInfo">@@@
@@@STEP_TEXT@</div>@@@
@@@STEP_LOG_LINE@graphs.dat@{}@@@
@@@STEP_LOG_END_PERF@graphs.dat@tab_capture_performance@@@
Traceback (most recent call last):
  File "/b/build/scripts/slave/runtest.py", line 1743, in <module>
    sys.exit(main())
  File "/b/build/scripts/slave/runtest.py", line 1707, in main
    result = _MainMac(options, args)
  File "/b/build/scripts/slave/runtest.py", line 935, in _MainMac
    options.factory_properties.get('perf_config'))
  File "/b/build/scripts/slave/runtest.py", line 604, in _SendResultsToDashboard
    build_dir, supplemental_columns)
  File "/b/build/scripts/slave/results_dashboard.py", line 59, in SendResults
    supplemental_columns)
  File "/b/build/scripts/slave/results_dashboard.py", line 192, in
_GetResultsJson
    _PrintLinkStep(url, master, perf_id, test_name, revision)
UnboundLocalError: local variable 'revision' referenced before assignment.

[ghost stip (do not use), 2014-04-01 00:28:06]

https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
File scripts/slave/results_dashboard.py (left):

https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
scripts/slave/results_dashboard.py:189: _PrintLinkStep(url, master, bot,
test_name, revision)
as far as I can tell, this old code worked because 'revision' stuck around from
the last for loop

https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
File scripts/slave/results_dashboard.py (right):

https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
scripts/slave/results_dashboard.py:192: _PrintLinkStep(url, master, perf_id,
test_name, revision)
add a check here for `if logsdict`. my guess is things were reverted because an
empty logsdict somehow got passed?

[rlarocque, 2014-04-01 00:30:03]

On 2014/04/01 00:28:06, stip wrote:
>
https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
> File scripts/slave/results_dashboard.py (left):
> 
>
https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
> scripts/slave/results_dashboard.py:189: _PrintLinkStep(url, master, bot,
> test_name, revision)
> as far as I can tell, this old code worked because 'revision' stuck around
from
> the last for loop
> 
>
https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
> File scripts/slave/results_dashboard.py (right):
> 
>
https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
> scripts/slave/results_dashboard.py:192: _PrintLinkStep(url, master, perf_id,
> test_name, revision)
> add a check here for `if logsdict`. my guess is things were reverted because
an
> empty logsdict somehow got passed?

This failure may be related, too:
http://build.chromium.org/p/chromium/builders/Linux%20x64/builds/63191/steps/...

[qyearsley, 2014-04-01 01:14:23]

https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
File scripts/slave/results_dashboard.py (right):

https://codereview.chromium.org/217053012/diff/20001/scripts/slave/results_da...
scripts/slave/results_dashboard.py:192: _PrintLinkStep(url, master, perf_id,
test_name, revision)
On 2014/04/01 00:28:07, stip wrote:
> add a check here for `if logsdict`. my guess is things were reverted because
an
> empty logsdict somehow got passed?

Alright, so I should add a check here (and also a maybe a check for whether
supplemental_columns is valid), and make it print a warning if something isn't
right.

Then I can re-upload this CL with a different issue number, right?