tools: WPR support for testing speculative_prefetch_predictor.

tools/resource_prefetch_predictor/prefetch_benchmark.py

Index: tools/resource_prefetch_predictor/prefetch_benchmark.py
diff --git a/tools/resource_prefetch_predictor/prefetch_benchmark.py b/tools/resource_prefetch_predictor/prefetch_benchmark.py
index 518af048f25af5731b9316fd67aa01dc56ce243f..bdde235c1e59a73c29f05a1e8f9cb82f3379aaaa 100755
--- a/tools/resource_prefetch_predictor/prefetch_benchmark.py
+++ b/tools/resource_prefetch_predictor/prefetch_benchmark.py
@@ -9,6 +9,7 @@
 import argparse
 import logging
 import os
+import random
 import sys
 import time
 
@@ -21,6 +22,7 @@ import customtabs_benchmark
 import device_setup
 
 sys.path.append(os.path.join(_SRC_PATH, 'tools', 'android', 'loading'))
+import controller
 from options import OPTIONS
 
 sys.path.append(os.path.join(_SRC_PATH, 'build', 'android'))
@@ -46,10 +48,16 @@ def _CreateArgumentParser():
                       help=('File containing the predictor database, as '
                             'obtained from generate_database.py.'))
   parser.add_argument('--url', help='URL to load.')
-  parser.add_argument('--prefetch_delay_ms',
-                      help='Prefetch delay in ms. -1 to disable prefetch.')
+  parser.add_argument('--prefetch_delays_ms',
+                      help='List of prefetch delays in ms. -1 to disable '
+                      'prefetch. Runs will randomly select one delay in the '

[pasko, 2016/12/12 18:07:10]

Are you looking for the delay that produces best results? A statistician hat
does not suit me well, but if I disregard this fact and try to fit the hat on my
head, I would seriously question the approach as leading to type 2 errors.

Can we agree upfront what 1-3 delays are considered 'typical' according to some
data from the field and then focus on benchmarking only those?

[Benoit L, 2016/12/14 17:39:39]

No, it's to disable the prefetcher, with something like 3000,-1, since -1
disables the prefetcher.

[pasko, 2016/12/14 19:38:33]

if that's the only usecase, let's have a hardcoded delay and
--disable-prefetcher. It would be easier to use and less to cover when
presenting results (like: create this file with two magic numbers, etc etc).

[Benoit L, 2016/12/16 14:27:00]

Well, I would also like to see the impact of several delays, so I would prefer
to leave that there.

[pasko, 2016/12/16 16:49:54]

This contradicts to the "No, it's to disable the prefetcher" you wrote
previously. Sad. I cannot stop you from experimenting, so this is moot.

+                      'list.')
   parser.add_argument('--output_filename',
                       help='CSV file to append the result to.')
+  parser.add_argument('--network_condition',
+                      help='Network condition for emulation.')
+  parser.add_argument('--wpr_archive', help='WPR archive path.')
+  parser.add_argument('--once', help='Only run once.', action='store_true')
   return parser
 
 
@@ -88,22 +96,40 @@ def _Setup(device, database_filename):
   device.RunShellCommand(command, as_root=True)
 
 
-def _Go(device, url, prefetch_delay_ms):
+def _Go(device, url, prefetch_delay_ms, wpr_archive, network_condition):
   disable_prefetch = prefetch_delay_ms == -1
   # Startup tracing to ease debugging.
   chrome_args = (customtabs_benchmark.CHROME_ARGS
                  + ['--trace-startup', '--trace-startup-duration=20'])
   if not disable_prefetch:
     chrome_args.append(_EXTERNAL_PREFETCH_FLAG)
-  prefetch_mode = 'disabled' if disable_prefetch else 'speculative_prefetch'
-  result = customtabs_benchmark.RunOnce(
-      device, url, warmup=True, speculation_mode=prefetch_mode,
-      delay_to_may_launch_url=2000,
-      delay_to_launch_url=prefetch_delay_ms, cold=False,
-      chrome_args=chrome_args, reset_chrome_state=False)
+
+  chrome_controller = controller.RemoteChromeController(device)
+  device.ForceStop(OPTIONS.ChromePackage().package)
+  chrome_controller.AddChromeArguments(chrome_args)
+
+  with device_setup.RemoteWprHost(
+      device, wpr_archive, record=False,
+      network_condition_name=network_condition) as wpr:
+    print wpr.chrome_args

[pasko, 2016/12/14 19:38:33]

nit: looks like debug code left here, but if you want to keep it, no objections,
logging.info may be better to avoid comments like this.

[Benoit L, 2016/12/16 14:27:00]

Done.

+    chrome_args += wpr.chrome_args
+    prefetch_mode = 'disabled' if disable_prefetch else 'speculative_prefetch'
+    result = customtabs_benchmark.RunOnce(
+        device, url, warmup=True, speculation_mode=prefetch_mode,
+        delay_to_may_launch_url=2000,
+        delay_to_launch_url=prefetch_delay_ms, cold=False,
+        chrome_args=chrome_args, reset_chrome_state=False)
   return customtabs_benchmark.ParseResult(result)
 
 
+def _RunOnce(device, database_filename, url, prefetch_delay_ms,
+             output_filename, wpr_archive, network_condition):
+  _Setup(device, database_filename)
+  result = _Go(device, url, prefetch_delay_ms, wpr_archive, network_condition)

[pasko, 2016/12/12 18:07:09]

naming: When given a "Go" and "RunOnce" and knowing that one is doing part of
the work of another, and asked which one is which, I would hesitate, and then
would say that "Go" probably does "RunOnce" several times, which is contrary to
what is really happening.

s/_Go/_RunOnceImpl/ or just inline?

[Benoit L, 2016/12/14 17:39:39]

Done.

+  with open(output_filename, 'a') as f:
+    f.write(','.join(str(x) for x in result) + '\n')

[pasko, 2016/12/12 18:07:09]

please print CSV columns as the first line of the file. Otherwise extending the
format and interpreting results from previous runs seem too error-prone.

[Benoit L, 2016/12/14 17:39:39]

Done.

+
+
 def main():
   logging.basicConfig(level=logging.INFO)
   devil_chromium.Initialize()
@@ -116,11 +142,14 @@ def main():
     logging.error('Could not find device: %s.', args.device)
     sys.exit(1)
 
-  _Setup(device, args.database)
-  result = _Go(device, args.url, int(args.prefetch_delay_ms))
-  print result
-  with open(args.output_filename, 'a') as f:
-    f.write(','.join(str(x) for x in result) + '\n')
+  delays = [int(x) for x in args.prefetch_delays_ms.split(',')]
+
+  while True:
+    delay = delays[random.randint(0, len(delays) - 1)]
+    _RunOnce(device, args.database, args.url, delay, args.output_filename,

[pasko, 2016/12/12 18:07:09]

for extra robustness against data mis-interpretation this should fail if the
file exists on start.

[Benoit L, 2016/12/14 17:39:39]

Done.

+             args.wpr_archive, args.network_condition)
+    if args.once:
+      return
 
 
 if __name__ == '__main__':