Bug 198144

Summary: webkitpy: Switch run-webkit-tests to tailspin
Product: WebKit Reporter: hysu <hysu>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, ews-watchlist, glenn, jbedard, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description hysu 2019-05-22 15:35:14 PDT
Alexey Proskuryakov:
run-webkit-tests runs sample or spindump when a test times out. This is costly and not very useful, as it collects the state after the problem happened, not during the time. Maybe we can use tailspins to make everything better.
Comment 1 hysu 2019-05-22 15:35:55 PDT
<rdar://problem/32463212>
Comment 2 hysu 2019-05-22 17:02:54 PDT
Created attachment 370468 [details]
Patch
Comment 3 Jonathan Bedard 2019-05-22 17:22:20 PDT
Comment on attachment 370468 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370468&action=review

> Tools/ChangeLog:9
> +        Changes run-webkit-tests to run tailspin on test time out, and

Instead of what?

> Tools/ChangeLog:12
> +        * Scripts/webkitpy/port/darwin.py:

Tell us what changed in these functions, unless the function name makes it obvious (like DarwinPort.tailspin_file_path)

> Tools/Scripts/webkitpy/port/darwin.py:179
> +                    DarwinPort.temp_tailspin_file_path(host, name, pid, str(tempdir)),

Will spindump let you make the input and output file the same file? That would make this code simpler (and testing easier)

> Tools/Scripts/webkitpy/port/darwin.py:183
> +                host.executive.run_command(symbolicate_command)

We probably don't need to name the list here, the only reason we did that with sudo was because we maybe had to add sudo.

> Tools/Scripts/webkitpy/port/darwin.py:214
> +    def temp_tailspin_file_path(host, name, pid, directory):

We won't need this function if we make spindump use the same file during symbolication.
Comment 4 hysu 2019-05-22 18:25:03 PDT
Created attachment 370473 [details]
Patch
Comment 5 hysu 2019-05-23 09:27:46 PDT
Comment on attachment 370468 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370468&action=review

>> Tools/Scripts/webkitpy/port/darwin.py:179
>> +                    DarwinPort.temp_tailspin_file_path(host, name, pid, str(tempdir)),
> 
> Will spindump let you make the input and output file the same file? That would make this code simpler (and testing easier)

By default, I don't think so -- if I specify the same input and output files spindump tells me "Cannot specify same output file as input file".
Comment 6 Jonathan Bedard 2019-05-23 10:49:50 PDT
Comment on attachment 370473 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370473&action=review

> Tools/Scripts/webkitpy/port/darwin.py:213
> +    def temp_tailspin_file_path(host, name, pid, directory):

The original intention of these functions was to handle the fact that we would be calling it with multiple hosts. In the temp_tailspin_file_path case, though, I think we can get away with just doing something like this:

temp_tailspin_file = host.filesystem.join(directory, "{0}-{1}-tailspin-temp.txt".format(name, pid))

in sample_process.
Comment 7 hysu 2019-05-24 00:13:28 PDT
Comment on attachment 370473 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370473&action=review

>> Tools/Scripts/webkitpy/port/darwin.py:213
>> +    def temp_tailspin_file_path(host, name, pid, directory):
> 
> The original intention of these functions was to handle the fact that we would be calling it with multiple hosts. In the temp_tailspin_file_path case, though, I think we can get away with just doing something like this:
> 
> temp_tailspin_file = host.filesystem.join(directory, "{0}-{1}-tailspin-temp.txt".format(name, pid))
> 
> in sample_process.

Good point, I'll also get rid of spindump_file_path as I don't think it's being used anywhere anymore
Comment 8 hysu 2019-05-24 00:27:02 PDT
Created attachment 370566 [details]
Patch
Comment 9 hysu 2019-05-24 09:40:52 PDT
webkitpy EWS is currently failing test_receiver_implementations as messages_unittest.py isn't working properly at the moment. See https://bugs.webkit.org/show_bug.cgi?id=198214
Comment 10 Jonathan Bedard 2019-05-28 08:29:40 PDT
Comment on attachment 370566 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370566&action=review

I think the last thing to be done here is to profile spindump vs tailspin to see which is faster. I suspect tailspin will narrowly outperform spindump, but that's going to depend on how long symbolication takes.

> Tools/Scripts/webkitpy/port/darwin.py:188
>              try:

Nit: No need for an exclamation mark

> Tools/Scripts/webkitpy/port/darwin.py:203
> +

Nit: Added newline.
Comment 11 hysu 2019-05-28 10:16:22 PDT
Created attachment 370758 [details]
Patch
Comment 12 hysu 2019-05-28 10:20:05 PDT
(In reply to Jonathan Bedard from comment #10)
> I think the last thing to be done here is to profile spindump vs tailspin to
> see which is faster. I suspect tailspin will narrowly outperform spindump,
> but that's going to depend on how long symbolication takes.
From my (rough) testing, spindump seems to be a decent bit slower than tailspin - about 30-35 seconds with spindump versus 15-20 with tailspin.
Comment 13 WebKit Commit Bot 2019-05-28 12:22:51 PDT
Comment on attachment 370758 [details]
Patch

Rejecting attachment 370758 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 370758, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=370758&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=198144&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Updating working directory
Processing patch 370758 from bug 198144.
Fetching: https://bugs.webkit.org/attachment.cgi?id=370758
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Jonathan Bedard']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 4 diffs from patch file(s).
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/Scripts/webkitpy/port/darwin.py
Hunk #1 FAILED at 160.
1 out of 3 hunks FAILED -- saving rejects to file Tools/Scripts/webkitpy/port/darwin.py.rej
patching file Tools/Scripts/webkitpy/port/darwin_testcase.py
patching file Tools/Scripts/webkitpy/port/ios_device_unittest.py

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Jonathan Bedard']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 4 diffs from patch file(s).
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/Scripts/webkitpy/port/darwin.py
Hunk #1 FAILED at 160.
1 out of 3 hunks FAILED -- saving rejects to file Tools/Scripts/webkitpy/port/darwin.py.rej
patching file Tools/Scripts/webkitpy/port/darwin_testcase.py
patching file Tools/Scripts/webkitpy/port/ios_device_unittest.py

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Jonathan Bedard']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
From https://git.webkit.org/git/WebKit
   94e868c940d..b3072db9512  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 245818 = 94e868c940d46c5745869192d07255331d00102b
r245819 = be178318cabf09424be5f56cbbfc7175532c9171
r245820 = c3621ee093f87df566a1f6df881e6f0b90b84194
r245821 = b3072db95127511e6b4bbfebfeffdf2c759cec18
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: https://webkit-queues.webkit.org/results/12310132
Comment 14 hysu 2019-05-28 12:37:00 PDT
Created attachment 370774 [details]
Patch
Comment 15 WebKit Commit Bot 2019-05-28 14:04:50 PDT
Comment on attachment 370774 [details]
Patch

Clearing flags on attachment: 370774

Committed r245824: <https://trac.webkit.org/changeset/245824>
Comment 16 WebKit Commit Bot 2019-05-28 14:04:52 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Jonathan Bedard 2019-05-30 09:05:23 PDT
Committed r245885: <https://trac.webkit.org/changeset/245885>