Fix binary size analysis

Bug: skia:
Change-Id: Ic739fd2d24ee2e9ed2c8b0dfdfc7118005350400
Reviewed-on: https://skia-review.googlesource.com/113300
Commit-Queue: Eric Boren <borenet@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>

5 files changed, 64 insertions, 55 deletions

diff --git a/infra/bots/recipe_modules/core/resources/run_binary_size_analysis.py b/infra/bots/recipe_modules/core/resources/run_binary_size_analysis.py
index 99887ea9cc..822b366614 100755
--- a/infra/bots/recipe_modules/core/resources/run_binary_size_analysis.py
+++ b/infra/bots/recipe_modules/core/resources/run_binary_size_analysis.py
@@ -93,8 +93,6 @@ import urllib2
 import binary_size_utils
 import elf_symbolizer
 
-from recipe_engine.types import freeze
-
 # Node dictionary keys. These are output in json read by the webapp so
 # keep them short to save file size.
 # Note: If these change, the webapp must also change.
@@ -114,12 +112,12 @@ NAME_NO_PATH_BUCKET = '(No Path)'
 BIG_BUCKET_LIMIT = 3000
 
 # Skia addition: relative dir for libskia.so from code base.
-LIBSKIA_RELATIVE_PATH = os.path.join('out', 'Release', 'lib')
+LIBSKIA_RELATIVE_PATH = os.path.join('out', 'Release')
 
 # Skia addition: dictionary mapping symbol type code to symbol name.
 # See
 # https://code.google.com/p/chromium/codesearch#chromium/src/tools/binary_size/template/D3SymbolTreeMap.js&l=74
-SYMBOL_MAP = freeze({
+SYMBOL_MAP = {
     'A': 'global_absolute',
     'B': 'global_uninitialized_data',
     'b': 'local_uninitialized_data',
@@ -146,7 +144,7 @@ SYMBOL_MAP = freeze({
     '@': 'vtable_entry',
     '-': 'stabs_debugging',
     '?': 'unrecognized',
-})
+}
 
 
 def _MkChild(node, name):
@@ -317,44 +315,17 @@ def GetBenchDict(githash, tree_root):
   return dic
 
 
-# Skia added: constructs 'gsutil cp' subprocess command list.
-def GetGsCopyCommandList(gsutil, src, dst):
-  return [gsutil, '-h', 'Content-Type:application/json', 'cp', '-a',
-          'public-read', src, dst]
-
-
-def DumpCompactTree(symbols, symbol_path_origin_dir, ha, ts, issue, gsutil):
+def DumpCompactTree(symbols, symbol_path_origin_dir, ha, ts, issue, dest):
   tree_root = MakeCompactTree(symbols, symbol_path_origin_dir)
   json_data = {'tree_data': tree_root,
                'githash': ha,
                'commit_ts': ts,
                'key': {'source_type': 'binary_size'},
                'total_size': sum(GetTreeSizes(tree_root).values()),}
-  tmpfile = tempfile.NamedTemporaryFile(delete=False).name
-  with open(tmpfile, 'w') as out:
+  with open(dest, 'w') as out:
     # Use separators without whitespace to get a smaller file.
     json.dump(json_data, out, separators=(',', ':'))
 
-  GS_PREFIX = 'gs://skia-perf/'
-  # Writes to Google Storage for visualization.
-  subprocess.check_call(GetGsCopyCommandList(
-      gsutil, tmpfile, GS_PREFIX + 'size/' + ha + '.json'))
-  # Updates the latest data.
-  if not issue:
-    subprocess.check_call(GetGsCopyCommandList(gsutil, tmpfile,
-                                               GS_PREFIX + 'size/latest.json'))
-  # Writes an extra copy using year/month/day/hour path for easy ingestion.
-  with open(tmpfile, 'w') as out:
-    json.dump(GetBenchDict(ha, tree_root), out, separators=(',', ':'))
-  now = datetime.datetime.utcnow()
-  ingest_path = '/'.join(('nano-json-v1', str(now.year).zfill(4),
-                          str(now.month).zfill(2), str(now.day).zfill(2),
-                          str(now.hour).zfill(2)))
-  if issue:
-    ingest_path = '/'.join('trybot', ingest_path, issue)
-  subprocess.check_call(GetGsCopyCommandList(gsutil, tmpfile,
-      GS_PREFIX + ingest_path + '/binarysize_' + ha + '.json'))
-
 
 def MakeSourceMap(symbols):
   sources = {}
@@ -748,8 +719,8 @@ def main():
                     help='Timestamp for the commit. Added by Skia.')
   parser.add_option('--issue_number', default='',
                     help='The trybot issue number in string. Added by Skia.')
-  parser.add_option('--gsutil_path', default='gsutil',
-                    help='Path to gsutil binary. Added by Skia.')
+  parser.add_option('--dest', default=None,
+                    help='Destination file to write results.')
   opts, _args = parser.parse_args()
 
   if ((not opts.library) and (not opts.nm_in)) or (opts.library and opts.nm_in):
@@ -809,7 +780,7 @@ def main():
       # Just a guess. Hopefully all paths in the input file are absolute.
       symbol_path_origin_dir = os.path.abspath(os.getcwd())
     DumpCompactTree(symbols, symbol_path_origin_dir, opts.githash,
-                    opts.commit_ts, opts.issue_number, opts.gsutil_path)
+                    opts.commit_ts, opts.issue_number, opts.dest)
     print 'Report data uploaded to GS.'
 
 
diff --git a/infra/bots/recipes/bookmaker.py b/infra/bots/recipes/bookmaker.py
index 5071cd53b8..4b3825257d 100644
--- a/infra/bots/recipes/bookmaker.py
+++ b/infra/bots/recipes/bookmaker.py
@@ -99,9 +99,6 @@ def RunSteps(api):
         content = api.file.read_text('Read fiddleout.json',
                                      fiddlecli_output, test_data=test_data)
         out = json.loads(content)
-        # Do a dump of fiddlecli_output. Will be useful for debugging.
-        print 'Dump of %s:' % fiddlecli_output
-        print json.dumps(out, indent=4)
 
         failing_fiddles_to_errors = {}
         for fiddle_name in out:
diff --git a/infra/bots/recipes/housekeeper.expected/Housekeeper-PerCommit-Trybot.json b/infra/bots/recipes/housekeeper.expected/Housekeeper-PerCommit-Trybot.json
index d54a4a29be..7d57a29557 100644
--- a/infra/bots/recipes/housekeeper.expected/Housekeeper-PerCommit-Trybot.json
+++ b/infra/bots/recipes/housekeeper.expected/Housekeeper-PerCommit-Trybot.json
@@ -115,13 +115,29 @@
   {
     "cmd": [
       "python",
+      "-u",
+      "RECIPE_MODULE[recipe_engine::file]/resources/fileutil.py",
+      "--json-output",
+      "/path/to/tmp/json",
+      "ensure-directory",
+      "--mode",
+      "0777",
+      "[CUSTOM_[SWARM_OUT_DIR]]/perfdata/Housekeeper-PerCommit/data"
+    ],
+    "cwd": "[CUSTOM_/_B_WORK]/skia",
+    "infra_step": true,
+    "name": "makedirs perf_dir"
+  },
+  {
+    "cmd": [
+      "python",
       "RECIPE_MODULE[skia::core]/resources/run_binary_size_analysis.py",
       "--library",
-      "[START_DIR]/out/Release/lib/libskia.so",
+      "[START_DIR]/out/Release/libskia.so",
       "--githash",
       "abc123",
-      "--gsutil_path",
-      "RECIPE_PACKAGE_REPO[depot_tools]/gsutil.py",
+      "--dest",
+      "[CUSTOM_[SWARM_OUT_DIR]]/perfdata/Housekeeper-PerCommit/data/nanobench_9046e2e693bb92a76e972b694580e5d17ad10748_1337000001.json",
       "--issue_number",
       "456789"
     ],
@@ -132,7 +148,7 @@
       "PATH": "<PATH>:RECIPE_PACKAGE_REPO[depot_tools]",
       "SKIA_OUT": "[START_DIR]/out"
     },
-    "name": "generate and upload binary size data"
+    "name": "generate binary size data"
   },
   {
     "name": "$result",
diff --git a/infra/bots/recipes/housekeeper.expected/Housekeeper-PerCommit.json b/infra/bots/recipes/housekeeper.expected/Housekeeper-PerCommit.json
index 9ee17b1897..f9b13d8a7b 100644
--- a/infra/bots/recipes/housekeeper.expected/Housekeeper-PerCommit.json
+++ b/infra/bots/recipes/housekeeper.expected/Housekeeper-PerCommit.json
@@ -125,13 +125,29 @@
   {
     "cmd": [
       "python",
+      "-u",
+      "RECIPE_MODULE[recipe_engine::file]/resources/fileutil.py",
+      "--json-output",
+      "/path/to/tmp/json",
+      "ensure-directory",
+      "--mode",
+      "0777",
+      "[CUSTOM_[SWARM_OUT_DIR]]/perfdata/Housekeeper-PerCommit/data"
+    ],
+    "cwd": "[CUSTOM_/_B_WORK]/skia",
+    "infra_step": true,
+    "name": "makedirs perf_dir"
+  },
+  {
+    "cmd": [
+      "python",
       "RECIPE_MODULE[skia::core]/resources/run_binary_size_analysis.py",
       "--library",
-      "[START_DIR]/out/Release/lib/libskia.so",
+      "[START_DIR]/out/Release/libskia.so",
       "--githash",
       "abc123",
-      "--gsutil_path",
-      "RECIPE_PACKAGE_REPO[depot_tools]/gsutil.py"
+      "--dest",
+      "[CUSTOM_[SWARM_OUT_DIR]]/perfdata/Housekeeper-PerCommit/data/nanobench_9046e2e693bb92a76e972b694580e5d17ad10748_1337000001.json"
     ],
     "cwd": "[CUSTOM_/_B_WORK]/skia",
     "env": {
@@ -140,7 +156,7 @@
       "PATH": "<PATH>:RECIPE_PACKAGE_REPO[depot_tools]",
       "SKIA_OUT": "[START_DIR]/out"
     },
-    "name": "generate and upload binary size data"
+    "name": "generate binary size data"
   },
   {
     "name": "$result",
diff --git a/infra/bots/recipes/housekeeper.py b/infra/bots/recipes/housekeeper.py
index 5d6a5f9119..f4c206a488 100644
--- a/infra/bots/recipes/housekeeper.py
+++ b/infra/bots/recipes/housekeeper.py
@@ -5,13 +5,19 @@
 
 # Recipe for the Skia PerCommit Housekeeper.
 
+
+import calendar
+
+
 DEPS = [
   'depot_tools/bot_update',
   'recipe_engine/context',
+  'recipe_engine/file',
   'recipe_engine/path',
   'recipe_engine/properties',
   'recipe_engine/python',
   'recipe_engine/step',
+  'recipe_engine/time',
   'core',
   'run',
   'vars',
@@ -27,7 +33,6 @@ def RunSteps(api):
   # TODO(borenet): Detect static initializers?
 
   with api.context(cwd=cwd):
-    gsutil_path = api.bot_update._module.PACKAGE_REPO_ROOT.join('gsutil.py')
     if not api.vars.is_trybot:
       api.run(
         api.step,
@@ -35,18 +40,22 @@ def RunSteps(api):
         cmd=['python', api.core.resource('generate_and_upload_doxygen.py')],
         abort_on_failure=False)
 
+    now = api.time.utcnow()
+    ts = int(calendar.timegm(now.utctimetuple()))
+    filename = 'nanobench_%s_%d.json' % (api.vars.got_revision, ts)
+    dest_dir = api.vars.perf_data_dir
+    dest_file = dest_dir + '/' + filename
+    api.file.ensure_directory('makedirs perf_dir', dest_dir)
     cmd = ['python', api.core.resource('run_binary_size_analysis.py'),
-           '--library', api.vars.skia_out.join(
-               'Release', 'lib', 'libskia.so'),
+           '--library', api.vars.skia_out.join('Release', 'libskia.so'),
            '--githash', api.properties['revision'],
-           '--gsutil_path', gsutil_path]
+           '--dest', dest_file]
     if api.vars.is_trybot:
       cmd.extend(['--issue_number', str(api.properties['patch_issue'])])
     api.run(
       api.step,
-      'generate and upload binary size data',
-      cmd=cmd,
-      abort_on_failure=False)
+      'generate binary size data',
+      cmd=cmd)
 
 
 def GenTests(api):