[tools] Specify affinity and raise priority when benchmarking

tools/run_perf.py

Index: tools/run_perf.py
diff --git a/tools/run_perf.py b/tools/run_perf.py
index db4245f499fc07c3a32f435a78ec08d5f06f58a3..5e849c7172937bce2e8b613dcf41fa3e8bacbb21 100755
--- a/tools/run_perf.py
+++ b/tools/run_perf.py
@@ -612,6 +612,21 @@ class Platform(object):
 class DesktopPlatform(Platform):
   def __init__(self, options):
     super(DesktopPlatform, self).__init__(options)
+    self.command_prefix = []
+
+    if options.prioritize or options.affinitize != None:
+      self.command_prefix = ["schedtool"]
+      if options.prioritize:
+        self.command_prefix += ["-n", "-20"]
+      if options.affinitize != None:
+      # schedtool expects a bit pattern when setting affinity, where each
+      # bit set to '1' corresponds to a core where the process may run on.
+      # First bit corresponds to CPU 0. Since the 'affinitize' parameter is
+      # a core number, we need to map to said bit pattern.
+        cpu = int(options.affinitize)
+        core = 1 << cpu
+        self.command_prefix += ["-a", ("0x%x" % core)]

[Michael Achenbach, 2016/04/28 13:03:07]

Another question to this: Now we set exactly one core. Does that sacrifice
performance for v8 (as v8 is multi-threaded)? Would we not get equally stable
results (without sacrificing speed) with a pattern that sets several cores but
avoids core 0? Like 0xe

[Mircea Trofin, 2016/04/28 14:21:57]

Multi-core may introduce other sources of variance, for instance, if the cores
are not sharing caches. For my goal - stable measurements for codegen - I may
even need to avoid v8's multi-threading altogether.

+      self.command_prefix += ["-e"]
 
   def PreExecution(self):
     pass
@@ -627,15 +642,18 @@ class DesktopPlatform(Platform):
     suffix = ' - without patch' if no_patch else ''
     shell_dir = self.shell_dir_no_patch if no_patch else self.shell_dir
     title = ">>> %%s (#%d)%s:" % ((count + 1), suffix)
+    command = self.command_prefix + runnable.GetCommand(shell_dir,
+                                                        self.extra_flags)
     try:
       output = commands.Execute(
-          runnable.GetCommand(shell_dir, self.extra_flags),
-          timeout=runnable.timeout,
+        command,
+        timeout=runnable.timeout,
       )
     except OSError as e:  # pragma: no cover
       print title % "OSError"
       print e
       return ""
+
     print title % "Stdout"
     print output.stdout
     if output.stderr:  # pragma: no cover
@@ -789,6 +807,80 @@ class AndroidPlatform(Platform):  # pragma: no cover
     return stdout
 
 
+def GetASLR():
+  try:
+    f = os.open("/proc/sys/kernel/randomize_va_space", os.O_RDONLY)

[Michael Achenbach, 2016/04/28 14:47:44]

Could you use with clauses? Also below.
with open("/proc/sys/kernel/randomize_va_space") as f:
  return int(f.read().strip())

[Mircea Trofin, 2016/04/28 16:02:46]

Done.

+    val = os.read(f, 1)
+    os.close(f)
+    return int(val)
+  except Exception as e:
+    print "Failed to get current ASLR settings."
+    raise e
+
+def SetASLR(value):
+  try:
+    f = os.open("/proc/sys/kernel/randomize_va_space", os.O_WRONLY)
+    val = os.write(f, str(value))
+    os.close(f)
+
+    new_value = GetASLR()
+    if value != new_value: raise Exception("Present value is %s" % new_value)

[Michael Achenbach, 2016/04/28 14:47:44]

Maybe move this condition outside the try block to avoid catching rethrowing
this exception.

[Mircea Trofin, 2016/04/28 16:02:46]

Done.

+  except Exception as e:
+    print "Failed to update ASLR to %s." % value
+    raise e
+
+def GetCPUCoresRange():
+  try:
+    f = open("/sys/devices/system/cpu/present", "r")
+    indexes = f.readline()
+    [first, last] = map(int, indexes.split("-"))

[Michael Achenbach, 2016/04/28 14:47:44]

first, last = map(int, indexes.split("-"))
should work

[Mircea Trofin, 2016/04/28 16:02:47]

Done.

+    return range(first, last + 1)
+  except Exception as e:
+    print "Failed to retrieve CPU governor value"
+    raise e
+
+def GetCPUPathForId(cpu_index):
+  ret = "/sys/devices/system/cpu/cpu"
+  ret += str(cpu_index)
+  ret += "/cpufreq/scaling_governor"
+  return ret
+
+def GetCPUGovernor():
+  try:
+    cpu_indices = GetCPUCoresRange()
+    ret = None
+    for cpu_index in cpu_indices:

[Michael Achenbach, 2016/04/28 14:47:44]

nit: Maybe inline GetCPUCoresRange()
also in method below

[Mircea Trofin, 2016/04/28 16:02:46]

I didn't understand what you meant here.

[Michael Achenbach, 2016/04/29 07:40:32]

I meant to skip the local variable. But now that the name got longer it's only
useful if you don't breach the 80 char limit:

cpu_indices = CustomMachineConfiguration.GetCPUCoresRange()
for cpu_index in cpu_indices:

into

for cpu_index in CustomMachineConfiguration.GetCPUCoresRange():

+      f = open(GetCPUPathForId(cpu_index), "r")
+      # We assume the governors of all CPUs are set to the same value
+      val = f.readline().strip()
+      f.close()
+      if ret == None:
+        ret = val
+      else:

[Michael Achenbach, 2016/04/28 14:47:44]

elif ret != val:

[Mircea Trofin, 2016/04/28 16:02:46]

Done.

+        if ret != val:
+          raise Exception("CPU cores have differing governor settings")
+    return ret
+  except Exception as e:
+    print "Failed to get the current CPU governor."
+    raise e
+
+
+def SetCPUGovernor(value):
+  try:
+    cpu_indices = GetCPUCoresRange()
+    for cpu_index in cpu_indices:
+      f = open(GetCPUPathForId(cpu_index), "w")
+      f.write(value)
+      f.close()
+
+    cur_value = GetCPUGovernor()
+    if cur_value != value:

[Michael Achenbach, 2016/04/28 14:47:44]

same as above - move condition outside try

[Mircea Trofin, 2016/04/28 16:02:46]

Done.

+      raise Exception("Could not set CPU governor. Present value is %s"
+                      % cur_value )
+  except Exception as e:
+    print "Failed to change CPU governor to %s." % value
+    raise e
+
 # TODO: Implement results_processor.
 def Main(args):
   logging.getLogger().setLevel(logging.INFO)
@@ -822,6 +914,27 @@ def Main(args):
                     help="JavaScript engine binary. By default, d8 under "
                     "architecture-specific build dir. "
                     "Not supported in conjunction with outdir-no-patch.")
+  parser.add_option("--prioritize",
+                    help="Raise the priority to nice -20 for the benchmarking"
+                    "process.Requires Linux, schedtool, and sudo privileges.",
+                    default=False, action="store_true")
+  parser.add_option("--affinitize",
+                    help="Run benchmarking process on the specified core."
+                    "For example:"
+                    "--affinitize=0 will run the benchmark process on core 0."
+                    "--affinitize=3 will run the benchmark process on core 3."
+                    "Requires Linux, schedtool, and sudo privileges.",
+                    default=None)
+  parser.add_option("--noaslr",
+                    help="Disable ASLR for the duration of the benchmarked"
+                    "process. Requires Linux and sudo privileges.",
+                    default=False, action="store_true")
+  parser.add_option("--cpu-governor",
+                    help="Set cpu governor to specified policy for the"

[Michael Achenbach, 2016/04/28 14:47:43]

nit: Space before " to not concatenate strings.

[Mircea Trofin, 2016/04/28 16:02:46]

Done.

+                    "duration of the benchmarked process. Typical options:"
+                    "'powersave' for more stable results, or 'performance'"
+                    "for shorter completion time of suite, with potentially"
+                    "more noise in results.")
 
   (options, args) = parser.parse_args(args)
 
@@ -872,6 +985,15 @@ def Main(args):
   else:
     options.shell_dir_no_patch = None
 
+  prev_aslr = None
+  prev_cpu_gov = None
+  if options.noaslr:
+    prev_aslr = GetASLR()
+    SetASLR(0)
+  if options.cpu_governor:
+    prev_cpu_gov = GetCPUGovernor()
+    SetCPUGovernor(options.cpu_governor)
+
   platform = Platform.GetPlatform(options)
 
   results = Results()
@@ -918,6 +1040,11 @@ def Main(args):
       results_no_patch += result_no_patch
     platform.PostExecution()
 

[Michael Achenbach, 2016/04/28 14:47:44]

Please refactor and put everything between setting/unsetting these things into a
separate method. Then put it in a "try" clause and put the code below into
"finally" to make sure we reset to original state also on other test runner
errors.

To make it easier, maybe move the reset code to the end of this method.

[Mircea Trofin, 2016/04/28 16:02:47]

Thanks for the suggestion - this makes sense. I moved it all under a resource
class.

[Michael Achenbach, 2016/04/29 07:40:32]

Very nice!

+  if prev_aslr != None:
+    SetASLR(prev_aslr)
+  if prev_cpu_gov != None:
+    SetCPUGovernor(prev_cpu_gov)
+
   if options.json_test_results:
     results.WriteToFile(options.json_test_results)
   else:  # pragma: no cover