run-perf-tests --profile should use iprofile instead of instruments on Mac
Created attachment 177015 [details] Patch
CCing folks who likely have opinions about Mac profiling tools. I will caution all that --profile is rather new and not the perfect experience on Mac. "iprofiler" runs asynchronously from the tests, does its own printing to stderr, and doesn't provide us a way to dump any sort of perf summary from the commandline. These combine to make it slightly at-odds with the --profile design (which was originally written for linux-command-line-profilers). In any case, I think --profile will grow to be more useful, and with some help will work cleaner on Mac than it does.
Forgot to CC folks who have reviewed past --profile patches, sorry.
One final note: As I alluded to in the ChangeLog, this is not a strict improvement from /usr/bin/instruments, but I figure that since /usr/bin/instruments seems "deprecated" it's best to move to the newer tool and iterate from there.
Created attachment 177018 [details] Made IProfiler.profile_after_exit wait for iprofiler to complete before continuing
Comment on attachment 177018 [details] Made IProfiler.profile_after_exit wait for iprofiler to complete before continuing rs=me.
Comment on attachment 177018 [details] Made IProfiler.profile_after_exit wait for iprofiler to complete before continuing View in context: https://bugs.webkit.org/attachment.cgi?id=177018&action=review > Tools/Scripts/webkitpy/common/system/profiler.py:101 > + # FIXME: Consider capturing instead of letting instruments spam to stderr directly. > + self._profiler_process = self._host.executive.popen(cmd) I guess I can try it, but what does it spam to stderr? Capturing the output and post-processing it like we do for pprof sounds like it might be a good next step.
Comment on attachment 177018 [details] Made IProfiler.profile_after_exit wait for iprofiler to complete before continuing View in context: https://bugs.webkit.org/attachment.cgi?id=177018&action=review >> Tools/Scripts/webkitpy/common/system/profiler.py:101 >> + self._profiler_process = self._host.executive.popen(cmd) > > I guess I can try it, but what does it spam to stderr? Capturing the output and post-processing it like we do for pprof sounds like it might be a good next step. iprofiler: Profiling process 7271 (DumpRenderTree) for 10 seconds iprofiler: Profiling completed... iprofiler: Session saved at /Users/eseidel/Projects/WebKit/WebKitBuild/Release/layout-test-results//Users/eseidel/Projects/WebKit/WebKitBuild/Release/layout-test-results/test.dtps The output is really nice. :) But since we don't control it is makes some things more complicated (like changing the name of the to-save-test-file-after Profiler initialization -- which I want to do so I can save these files at test/file/path.trace instead of just test.trace). One complexity in trying to capture the output is that iprofiler will prompt the user for their password if necessary. We'd have to make sure to route input to the iprofiler process as well.
Comment on attachment 177018 [details] Made IProfiler.profile_after_exit wait for iprofiler to complete before continuing I now realize I think its splitting out the profile in the wrong place. Will upload a fix.
Created attachment 177022 [details] Patch for landing
Yup. Missed a fs.basename() call. Fixed.
Comment on attachment 177022 [details] Patch for landing Clearing flags on attachment: 177022 Committed r136327: <http://trac.webkit.org/changeset/136327>
All reviewed patches have been landed. Closing bug.
revision http://trac.webkit.org/changeset/136327 broke test-webkitpy (on mac-mountainlion): sn-ga-02:webkitsvn glenn$ Tools/Scripts/test-webkitpy Suppressing most webkitpy logging while running unit tests. Skipping tests in the following modules or packages because they are really, really, slow: webkitpy.common.checkout.scm.scm_unittest (https://bugs.webkit.org/show_bug.cgi?id=31818; use --all to include) Checking imports ...Failed to import webkitpy.common.system.profiler_unittest: Traceback (most recent call last): File "/Users/glenn/work/webkitsvn/Tools/Scripts/webkitpy/test/main.py", line 176, in _check_imports __import__(name) File "/Users/glenn/work/webkitsvn/Tools/Scripts/webkitpy/common/system/profiler_unittest.py", line 33, in <module> from .profiler import ProfilerFactory, Instruments, GooglePProf ImportError: cannot import name Instruments
Sorry. Easy fix. Will fix shortly.
Reopening to attach new patch.
Created attachment 177144 [details] Fix test-webkitpy
Comment on attachment 177144 [details] Fix test-webkitpy Sorry. the Commit-queue doesn't run test-webkitpy at the moment. I think Dirk said he's working on fixing that.
Comment on attachment 177144 [details] Fix test-webkitpy Rejecting attachment 177144 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 /mnt/git/webkit-commit-queue/Tools/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/15120063
Created attachment 177146 [details] Patch for landing
Comment on attachment 177144 [details] Fix test-webkitpy View in context: https://bugs.webkit.org/attachment.cgi?id=177144&action=review > Tools/ChangeLog:6 > + Unreviwed. Updating the unittests after my previous change. Unreviwed -> Unreviewed.
Comment on attachment 177146 [details] Patch for landing Clearing flags on attachment: 177146 Committed r136351: <http://trac.webkit.org/changeset/136351>
thanks, that fixed it (comment #14)