Modified: trunk/Tools/ChangeLog (136326 => 136327)
--- trunk/Tools/ChangeLog 2012-12-02 10:55:24 UTC (rev 136326)
+++ trunk/Tools/ChangeLog 2012-12-02 11:00:13 UTC (rev 136327)
@@ -1,3 +1,35 @@
+2012-12-02 Eric Seidel <[email protected]>
+
+ run-perf-tests --profile should use iprofiler instead of instruments on Mac
+ https://bugs.webkit.org/show_bug.cgi?id=103765
+
+ Reviewed by Ryosuke Niwa.
+
+ Stephanie pointed out that iprofiler is the newer command-line interface
+ to replace /usr/bin/instruments in Instruments 4 (which according to the
+ interblags is supported on everything since 10.6.6), so I've moved --profile
+ to use iprofiler on Mac.
+
+ I also made the IProfiler class wait for iprofiler to complete so that
+ iprofiler won't continue running after run-perf-tests is complete (and
+ spam over the top of your shell output).
+
+ There are a couple of "got-chas" to this approach:
+ 1. iprofiler will ask for your password the first time you run it, which may
+ cause your first test to fail. /usr/bin/instruments doesn't seem to do that.
+ 2. iprofiler has nice (but kinda awkward for our purposes) support for separate
+ directory and basename arguments, making IProfiler a somewhat awkward
+ baseclass of SingleFileOutputProfiler
+
+ I suspect we'll iterate from here. It does seem slightly faster than
+ /usr/bin/instruments was.
+
+ * Scripts/webkitpy/common/system/profiler.py:
+ (ProfilerFactory.create_profiler):
+ (IProfiler):
+ (IProfiler.__init__):
+ (IProfiler.attach_to_pid):
+
2012-12-02 Yongjun Zhang <[email protected]>
Need a method to close all idle localstorage databases immediately.
Modified: trunk/Tools/Scripts/webkitpy/common/system/profiler.py (136326 => 136327)
--- trunk/Tools/Scripts/webkitpy/common/system/profiler.py 2012-12-02 10:55:24 UTC (rev 136326)
+++ trunk/Tools/Scripts/webkitpy/common/system/profiler.py 2012-12-02 11:00:13 UTC (rev 136327)
@@ -36,7 +36,7 @@
@classmethod
def create_profiler(cls, host, executable_path, output_dir, identifier=None):
if host.platform.is_mac():
- return Instruments(host, executable_path, output_dir, identifier)
+ return IProfiler(host, executable_path, output_dir, identifier)
return GooglePProf(host, executable_path, output_dir, identifier)
@@ -85,15 +85,22 @@
print self._first_ten_lines_of_profile(profile_text)
-# FIXME: iprofile is a newer commandline interface to replace /usr/bin/instruments.
-class Instruments(SingleFileOutputProfiler):
+class IProfiler(SingleFileOutputProfiler):
def __init__(self, host, executable_path, output_dir, identifier=None):
- super(Instruments, self).__init__(host, executable_path, output_dir, "trace", identifier)
+ super(IProfiler, self).__init__(host, executable_path, output_dir, "dtps", identifier)
+ self._profiler_process = None
- # FIXME: We may need a way to find this tracetemplate on the disk
- _time_profile = "/Applications/Xcode.app/Contents/Applications/Instruments.app/Contents/Resources/templates/Time Profiler.tracetemplate"
-
def attach_to_pid(self, pid):
- cmd = ["instruments", "-t", self._time_profile, "-D", self._output_path, "-p", pid]
+ # FIXME: iprofiler requires us to pass the directory separately
+ # from the basename of the file, with no control over the extension.
+ fs = self._host.filesystem
+ cmd = ["iprofiler", "-timeprofiler", "-a", pid,
+ "-d", fs.dirname(self._output_path), "-o", fs.splitext(fs.basename(self._output_path))[0]]
cmd = map(unicode, cmd)
- self._host.executive.popen(cmd)
+ # FIXME: Consider capturing instead of letting instruments spam to stderr directly.
+ self._profiler_process = self._host.executive.popen(cmd)
+
+ def profile_after_exit(self):
+ # It seems like a nicer user experiance to wait on the profiler to exit to prevent
+ # it from spewing to stderr at odd times.
+ self._profiler_process.wait()
