Update V8ObjectStatsMetric to work as a standalone metric

tools/perf/metrics/v8_object_stats.py

 # Copyright 2013 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.
 
 import json
 import logging
 
 from metrics import Metric
 
 _COUNTER_NAMES = [
@@ skipping 14 matching lines @@
     'V8.MemoryMapSpaceBytesCommitted',
     'V8.MemoryMapSpaceBytesUsed',
     'V8.MemoryCellSpaceBytesAvailable',
     'V8.MemoryCellSpaceBytesCommitted',
     'V8.MemoryCellSpaceBytesUsed',
     'V8.MemoryPropertyCellSpaceBytesAvailable',
     'V8.MemoryPropertyCellSpaceBytesCommitted',
     'V8.MemoryPropertyCellSpaceBytesUsed',
     'V8.MemoryLoSpaceBytesAvailable',
     'V8.MemoryLoSpaceBytesCommitted',
-    'V8.MemoryLoSpaceBytesUsed)',

[rmcilroy, 2013/09/04 18:09:21]

oops, thanks!

+    'V8.MemoryLoSpaceBytesUsed',
     'V8.SizeOf_ACCESSOR_PAIR_TYPE',
     'V8.SizeOf_ACCESS_CHECK_INFO_TYPE',
     'V8.SizeOf_ALIASED_ARGUMENTS_ENTRY_TYPE',
     'V8.SizeOf_ALLOCATION_MEMENTO_TYPE',
     'V8.SizeOf_ALLOCATION_SITE_TYPE',
     'V8.SizeOf_ASCII_INTERNALIZED_STRING_TYPE',
     'V8.SizeOf_ASCII_STRING_TYPE',
     'V8.SizeOf_BOX_TYPE',
     'V8.SizeOf_BREAK_POINT_INFO_TYPE',
     'V8.SizeOf_BYTE_ARRAY_TYPE',
@@ skipping 111 matching lines @@
   def __init__(self):
     super(V8ObjectStatsMetric, self).__init__()
     self._results = None
 
   @classmethod
   def CustomizeBrowserOptions(cls, options):
     options.AppendExtraBrowserArg('--enable-stats-table')
     options.AppendExtraBrowserArg('--enable-benchmarking')
     options.AppendExtraBrowserArg(
         '--js-flags=--track_gc_object_stats --expose_gc')
+    options.AppendExtraBrowserArg('--no-sandbox')

[tonyg, 2013/09/04 17:41:11]

I don't see how this is related. Can you explain why we need --no-sandbox?

[edmundyan, 2013/09/04 17:53:41]

I needed to add this line over in endure.py, otherwise no pages could be loaded
(I couldn't even navigate to them). I do not know why it is needed. 
'--no-sandbox' is added in the IO metric, thus this is never a problem for
page_cycler.  This is the stack trace on endure.py when I don't have sandbox:

Stack Trace:
********************************************************************************
	[7:7:0904/104928:ERROR:shared_memory_posix.cc(213)] Creating shared memory in
/dev/shm/org.chromium.Chromium.shmem.ChromiumStats2-4294967295 failed:
Permission denied
	[7:7:0904/104928:ERROR:shared_memory_posix.cc(216)] Unable to access(W_OK|X_OK)
/dev/shm: Permission denied
	[7:7:0904/104928:FATAL:shared_memory_posix.cc(218)] This is frequently caused
by incorrect permissions on /dev/shm.  Try 'sudo chmod 1777 /dev/shm' to fix.
	[8:8:0904/104930:ERROR:shared_memory_posix.cc(213)] Creating shared memory in
/dev/shm/org.chromium.Chromium.shmem.ChromiumStats2-4294967295 failed:
Permission denied
	[8:8:0904/104930:ERROR:shared_memory_posix.cc(216)] Unable to access(W_OK|X_OK)
/dev/shm: Permission denied
	[8:8:0904/104930:FATAL:shared_memory_posix.cc(218)] This is frequently caused
by incorrect permissions on /dev/shm.  Try 'sudo chmod 1777 /dev/shm' to fix.
	
********************************************************************************

[rmcilroy, 2013/09/04 18:09:21]

Ahh yes, this is required for --enable-stats-table currently (it must be set
elsewhere for the page_cycler). I have a change
(https://codereview.chromium.org/22911027/) which enables stats-table to be used
on Android, and also means that on Posix systems the --no-sandbox option is not
needed, but this hasn't landed yet.  I think this is required for now, but maybe
add a TODO(rmcilroy) to only do this on windows when the CL lands.

[edmundyan, 2013/09/04 18:54:02]

Done.
/me wonders if there are other browser options that are platform-dependent in
Telemetry.  How would we do this?

+
+  @staticmethod
+  def GetV8StatsTable(tab, counters=None):
+    if counters is None:
+      counters = _COUNTER_NAMES

[tonyg, 2013/09/04 17:41:11]

Rather than this, can't you just do counters=_COUNTER_NAMES right in the
function arguments?

[edmundyan, 2013/09/04 17:53:41]

That is what I did originally, but pylint didn't like it, saying " dangerous
default value as argument".  Technically, _COUNTER_NAMES is still a mutable
list.

[rmcilroy, 2013/09/04 18:09:21]

Maybe just: 
counters = counters or _COUNTER_NAMES

[edmundyan, 2013/09/04 18:54:02]

Done.

+
+    return tab.EvaluateJavaScript("""
+     (function(counters) {
+       var results = {};
+       if (!window.chrome || !window.chrome.benchmarking)
+        return results;
+       window.gc();  // Trigger GC to ensure stats are checkpointed.
+       for (var i = 0; i < counters.length; i++)
+         results[counters[i]] =
+             chrome.benchmarking.counterForRenderer(counters[i]);
+       return results;
+     })(%s);
+     """ % json.dumps(counters))
 
   def Start(self, page, tab):
     """Do Nothing."""
     pass
 
   def Stop(self, page, tab):
     """Get the values in the stats table after the page is loaded."""
-    self._results = tab.EvaluateJavaScript("""
- (function(counters) {
-   var results = {};
-   if (!window.chrome || !window.chrome.benchmarking)
-    return results;
-   window.gc();  // Trigger GC to ensure stats are checkpointed.
-   for (var i = 0; i < counters.length; i++)
-     results[counters[i]] = chrome.benchmarking.counterForRenderer(counters[i]);
-   return results;
- })(%s);
- """ % json.dumps(_COUNTER_NAMES))
+    self._results = V8ObjectStatsMetric.GetV8StatsTable(tab)
     if not self._results:
       logging.warning('No V8 object stats from website: ' + page.display_url)
 
   def AddResults(self, tab, results):
     """Add results for this page to the results object."""
     assert self._results != None, 'Must call Stop() first'
     for counter_name in self._results:
       results.Add(counter_name, 'kb', self._results[counter_name] / 1024.0)