Bug 135409 – [iOS] run-webkit-tests runs webkit-build-directory on every test

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Farler]

At least on the iOS simulator, run-webkit-tests calls webkit-build-directory a lot - at least once per tests with run times between 1/4 and over 2 seconds! I’ll investigate to see if this is affecting other ports and cache the result accordingly. It doesn’t need to be calling out to a Perl script for what is basically a constant.

[Csaba Osztrogonác]

I checked, this script is called only twice on EFL, so it is an iOS
specific bug.

Maybe mac_config.build_directory call in relay_path causes this problem:
http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/port/ios.py?rev=171789#L76

[David Farler]

relay_path is only called a handful of times. The culprit is in webkitpy.port.config.Config.build_directory, so unless a port overrides this somehow, it would be affecting everyone. You can check to see if it is eating up time by enabling debug logging in run-webkit-tests with --debug-rwt-logging.

[David Farler]

I stand corrected - _path_to_image_diff is called for each ref test and I’m constructing a mac Config object in there. Silly. I’ll save the path during construction of the port.

[David Farler]

Created attachment 235966 [details]
Patch

[David Farler]

Comment on attachment 235966 [details]
Patch

Wrong patch, apologies.

[David Farler]

Created attachment 236056 [details]
Patch

[Daniel Bates]

Comment on attachment 236056 [details]
Patch

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

> Tools/ChangeLog:10
> +        (IOSSimulatorPort.__init__):
> +        Cache Mac build directory once.

Nit: It seems sufficient to collapse these two line into a single line such that it reads:

(IOSSimulatorPort.__init__): Cache Mac build directory.

(Notice that I removed the word "once" since this is implied by the use of the word "cache").

> Tools/ChangeLog:12
> +        (IOSSimulatorPort.relay_path):
> +        Use cached build directory.

Nit: Similarly I would write the contents of these two lines in a single line.

> Tools/ChangeLog:14
> +        (IOSSimulatorPort._path_to_image_diff):
> +        Use cached build directory.

Ditto.

> Tools/Scripts/webkitpy/port/ios.py:67
> +        self._mac_config = port_config.Config(self._executive, self._filesystem, 'mac')

This instance variable is referenced exactly once (on the line below). Unless you plan to make use of it in subsequent patches in the near future, I suggest we either inline its value into the line below or make it a local variable.

[David Farler]

Committed r172602: <http://trac.webkit.org/changeset/172602>

[David Kilzer (:ddkilzer)]

Re-fixed in commit r180845:  <http://trac.webkit.org/changeset/180845>

[David Kilzer (:ddkilzer)]

(In reply to comment #9)
> Re-fixed in commit r180845:  <http://trac.webkit.org/changeset/180845>

This regressed with the fix for Bug 140949:

[iOS] run-webkit-tests --platform=ios* --lint-test-files does not work
https://bugs.webkit.org/show_bug.cgi?id=140949
<http://trac.webkit.org/changeset/179216>