Allow multiple performance values in _OutputPerfForStandaloneGraphing.

chrome/test/functional/perf.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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.
 
 """Basic pyauto performance tests.
 
 For tests that need to be run for multiple iterations (e.g., so that average
 and standard deviation values can be reported), the default number of iterations
 run for each of these tests is specified by |_DEFAULT_NUM_ITERATIONS|.
@@ skipping 306 matching lines @@
       os.chmod(graphs_file, 0755)
 
     # Update data file for this particular graph.
     existing_lines = []
     data_file = os.path.join(self._local_perf_dir, graph_name + '-summary.dat')
     if os.path.exists(data_file):
       with open(data_file) as f:
         existing_lines = f.readlines()
     existing_lines = map(eval, map(lambda x: x.strip(), existing_lines))
 
-    seen_key = graph_name + '|' + description
+    seen_key = graph_name
+    # We assume that the first line |existing_line[0]| is the latest.

[dennis_jeffrey, 2012/08/29 18:47:51]

nit: existing_line --> existing_lines

[Dai Mikurube (NOT FULLTIME), 2012/08/30 02:24:11]

Done.

+    # We update only the first line.

[dennis_jeffrey, 2012/08/29 18:47:51]

I recommend removing this line.  There's a case where we may not actually update
the first line, and that's when we're creating a new line for the first time
during a test run.

[Dai Mikurube (NOT FULLTIME), 2012/08/30 02:24:11]

Removed.  To write it correctly, "We insert a new line at the top, or update the
first line"?

[dennis_jeffrey, 2012/08/30 17:13:23]

Yes, I believe that would be a more accurate statement.  But I think it's also
ok to just leave out that comment.

     if units_x:
-      points = []
+      new_line = {
+        'rev': revision,
+        'traces': { description: [] }
+      }
       if seen_key in self._seen_graph_lines:
         # We've added points previously for this graph line in the current
         # test execution, so retrieve the original set of points specified in
         # the most recent revision in the data file.
-        points = existing_lines[0]['traces'][description]
+        new_line = existing_lines[0]
+        if not description in new_line['traces']:
+          new_line['traces'][description] = []
       for x_value, y_value in value:
-        points.append([str(x_value), str(y_value)])
-      new_traces = {
-        description: points
+        new_line['traces'][description].append([str(x_value), str(y_value)])
+    else:
+      new_line = {
+        'rev': revision,
+        'traces': { description: [str(value), str(0.0)] }
       }
-    else:
-      new_traces = {
-        description: [str(value), str(0.0)]
-      }
-
-    new_line = {
-      'rev': revision,
-      'traces': new_traces
-    }
 
     if seen_key in self._seen_graph_lines:

[dennis_jeffrey, 2012/08/29 18:47:51]

FYI: The reason for "seen_key" and "self._seen_graph_lines" is to allow
long-running tests (like Chrome Endure tests) to continually invoke the current
function (OutputPerfForStandaloneGraphing) during a test run, and each time the
function is invoked, *append* new results to an existing results list.

For example, suppose we're periodically measuring DOM node count during a long
test run.  The very first time we invoke OutputPerfForStandaloneGraphing, we
should create a new JSON line for the output file.  But each time we
subsequently invoke OutputPerfForStandaloneGraphing for the same metric in the
same test run, we need to know that we have to *append* to the existing JSON
line, rather than creating a new JSON line.  So that's why I associate each
metric with a unique key (called "seen_key"), and I store all the keys we've
already seen in the dictionary "self._seen_graph_lines".

[Dai Mikurube (NOT FULLTIME), 2012/08/30 02:24:11]

Thanks for the description.  Hmm, my question is: isn't it enough to compare
|revision| with the top line in -summary.dat?

I guess it would have a same effect.  If seen_key is found, it has been already
depended on the existing_lines[0].

[dennis_jeffrey, 2012/08/30 17:13:23]

I am not totally clear on exactly what you mean, but you may be correct that we
can simplify the code here.  If you'd like, feel free to make the change itself
and send me a new code review, and then I can check it out.  By looking at the
code it'll be easier for me to see exactly what you'd like to do :-)

[Dai Mikurube (NOT FULLTIME), 2012/09/04 04:32:58]

Ok, I'll try it when I have time.  Thanks. :)

       # Update results for the most recent revision.
       existing_lines[0] = new_line
     else:
       # New results for a new revision.
       existing_lines.insert(0, new_line)
       self._seen_graph_lines[seen_key] = True
 
     existing_lines = map(str, existing_lines)
     with open(data_file, 'w') as f:
       f.write('\n'.join(existing_lines))
@@ skipping 2292 matching lines @@
     """Identifies the port number to which the server is currently bound.
 
     Returns:
       The numeric port number to which the server is currently bound.
     """
     return self._server.server_address[1]
 
 
 if __name__ == '__main__':
   pyauto_functional.Main()