Title: [136106] trunk/Tools
Revision
136106
Author
[email protected]
Date
2012-11-29 00:03:31 -0800 (Thu, 29 Nov 2012)

Log Message

run-perf-tests does not work when the layout test directory does not exist
https://bugs.webkit.org/show_bug.cgi?id=103572

Reviewed by Ryosuke Niwa.

Make sure that Profiler() calls maybe_make_directory for the output_dir
before ever using it.  It's a little awkward to create the directory
from the constructor, but its simplest that way as the various subclasses
all use the directory at different times.

Since this required having a filesystem in Profiler (and I didn't want
to grab inside Workspace to get one), I just made Profiler expect a
SystemHost instead of a separate filesystem, executive and workspace.

* Scripts/webkitpy/common/system/profiler.py:
(ProfilerFactory.create_profiler):
(Profiler.__init__):
(SingleFileOutputProfiler.__init__):
(GooglePProf.__init__):
(GooglePProf.profile_after_exit):
(Instruments.__init__):
(Instruments.attach_to_pid):
* Scripts/webkitpy/common/system/profiler_unittest.py:
(ProfilerFactoryTest.test_basic):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (136105 => 136106)


--- trunk/Tools/ChangeLog	2012-11-29 07:46:01 UTC (rev 136105)
+++ trunk/Tools/ChangeLog	2012-11-29 08:03:31 UTC (rev 136106)
@@ -1,3 +1,30 @@
+2012-11-29  Eric Seidel  <[email protected]>
+
+        run-perf-tests does not work when the layout test directory does not exist
+        https://bugs.webkit.org/show_bug.cgi?id=103572
+
+        Reviewed by Ryosuke Niwa.
+
+        Make sure that Profiler() calls maybe_make_directory for the output_dir
+        before ever using it.  It's a little awkward to create the directory
+        from the constructor, but its simplest that way as the various subclasses
+        all use the directory at different times.
+
+        Since this required having a filesystem in Profiler (and I didn't want
+        to grab inside Workspace to get one), I just made Profiler expect a
+        SystemHost instead of a separate filesystem, executive and workspace.
+
+        * Scripts/webkitpy/common/system/profiler.py:
+        (ProfilerFactory.create_profiler):
+        (Profiler.__init__):
+        (SingleFileOutputProfiler.__init__):
+        (GooglePProf.__init__):
+        (GooglePProf.profile_after_exit):
+        (Instruments.__init__):
+        (Instruments.attach_to_pid):
+        * Scripts/webkitpy/common/system/profiler_unittest.py:
+        (ProfilerFactoryTest.test_basic):
+
 2012-11-28  Vivek Galatage  <[email protected]>
 
         Adding secondary email id

Modified: trunk/Tools/Scripts/webkitpy/common/system/profiler.py (136105 => 136106)


--- trunk/Tools/Scripts/webkitpy/common/system/profiler.py	2012-11-29 07:46:01 UTC (rev 136105)
+++ trunk/Tools/Scripts/webkitpy/common/system/profiler.py	2012-11-29 08:03:31 UTC (rev 136106)
@@ -36,17 +36,17 @@
     @classmethod
     def create_profiler(cls, host, executable_path, output_dir, identifier=None):
         if host.platform.is_mac():
-            return Instruments(host.workspace, host.executive, executable_path, output_dir, identifier)
-        return GooglePProf(host.workspace, host.executive, executable_path, output_dir, identifier)
+            return Instruments(host, executable_path, output_dir, identifier)
+        return GooglePProf(host, executable_path, output_dir, identifier)
 
 
 class Profiler(object):
-    def __init__(self, workspace, executive, executable_path, output_dir, identifier=None):
-        self._workspace = workspace
-        self._executive = executive
+    def __init__(self, host, executable_path, output_dir, identifier=None):
+        self._host = host
         self._executable_path = executable_path
         self._output_dir = output_dir
         self._identifier = "test"
+        self._host.filesystem.maybe_make_directory(self._output_dir)
 
     def adjusted_environment(self, env):
         return env
@@ -59,14 +59,14 @@
 
 
 class SingleFileOutputProfiler(Profiler):
-    def __init__(self, workspace, executive, executable_path, output_dir, output_suffix, identifier=None):
-        super(SingleFileOutputProfiler, self).__init__(workspace, executive, executable_path, output_dir, identifier)
-        self._output_path = self._workspace.find_unused_filename(self._output_dir, self._identifier, output_suffix)
+    def __init__(self, host, executable_path, output_dir, output_suffix, identifier=None):
+        super(SingleFileOutputProfiler, self).__init__(host, executable_path, output_dir, identifier)
+        self._output_path = self._host.workspace.find_unused_filename(self._output_dir, self._identifier, output_suffix)
 
 
 class GooglePProf(SingleFileOutputProfiler):
-    def __init__(self, workspace, executive, executable_path, output_dir, identifier=None):
-        super(GooglePProf, self).__init__(workspace, executive, executable_path, output_dir, "pprof", identifier)
+    def __init__(self, host, executable_path, output_dir, identifier=None):
+        super(GooglePProf, self).__init__(host, executable_path, output_dir, "pprof", identifier)
 
     def adjusted_environment(self, env):
         env['CPUPROFILE'] = self._output_path
@@ -81,14 +81,14 @@
         # google-pprof installed as "pprof" on their machines for them.
         # FIXME: Similarly we should find the right perl!
         pprof_args = ['/usr/bin/perl', '/usr/bin/google-pprof', '--text', self._executable_path, self._output_path]
-        profile_text = self._executive.run_command(pprof_args)
+        profile_text = self._host.executive.run_command(pprof_args)
         print self._first_ten_lines_of_profile(profile_text)
 
 
 # FIXME: iprofile is a newer commandline interface to replace /usr/bin/instruments.
 class Instruments(SingleFileOutputProfiler):
-    def __init__(self, workspace, executive, executable_path, output_dir, identifier=None):
-        super(Instruments, self).__init__(workspace, executive, executable_path, output_dir, "trace", identifier)
+    def __init__(self, host, executable_path, output_dir, identifier=None):
+        super(Instruments, self).__init__(host, executable_path, output_dir, "trace", identifier)
 
     # 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"
@@ -96,4 +96,4 @@
     def attach_to_pid(self, pid):
         cmd = ["instruments", "-t", self._time_profile, "-D", self._output_path, "-p", pid]
         cmd = map(unicode, cmd)
-        self._executive.popen(cmd)
+        self._host.executive.popen(cmd)

Modified: trunk/Tools/Scripts/webkitpy/common/system/profiler_unittest.py (136105 => 136106)


--- trunk/Tools/Scripts/webkitpy/common/system/profiler_unittest.py	2012-11-29 07:46:01 UTC (rev 136105)
+++ trunk/Tools/Scripts/webkitpy/common/system/profiler_unittest.py	2012-11-29 08:03:31 UTC (rev 136106)
@@ -36,7 +36,9 @@
 class ProfilerFactoryTest(unittest.TestCase):
     def test_basic(self):
         host = MockSystemHost()
+        self.assertFalse(host.filesystem.exists("/tmp/output"))
         profiler = ProfilerFactory.create_profiler(host, '/bin/executable', '/tmp/output')
+        self.assertTrue(host.filesystem.exists("/tmp/output"))
         self.assertEquals(profiler._output_path, "/tmp/output/test.trace")
 
         host.platform.os_name = 'linux'
@@ -81,5 +83,5 @@
       28   0.7%  11.6%       28   0.7% WebCore::QualifiedName::localName (inline)
 """
         host = MockSystemHost()
-        profiler = GooglePProf(host.workspace, host.executive, '/bin/executable', '/tmp/output')
+        profiler = GooglePProf(host, '/bin/executable', '/tmp/output')
         self.assertEquals(profiler._first_ten_lines_of_profile(pprof_output), expected_first_ten_lines)
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to