Changeset 204659 in webkit


Ignore:
Timestamp:
Aug 19, 2016, 3:13:15 PM (9 years ago)
Author:
Simon Fraser
Message:

REGRESSION (r204477): Running LayoutTests on ios-simulator became ~15 minutes slower
​https://bugs.webkit.org/show_bug.cgi?id=160985

Reviewed by Alexey Proskuryakov.

r204477 removed @memoized on a couple of ios.py functions, causing them to instantiate
a Simulator() on every call, which causes 'xcrun simctl list' to run. The functions
must not be @memoized, because their return value depends on the value of simulator_device_type().

Fix by adding some global state in simulator.py that tracks the created devices
in a worker number -> Device dictionary. Explicitly create devices in _create_simulators(),
and delete them in clean_up_test_run().

Also explicitly called 'xcrun simctl shutdown' to shut down devices, since it seems
that killing the Simulator apps isn't enough.

Simulator tracks the devices in a global dictionary, since state needs to persist
across different instances of IOSSimulatorPort.

Annoyingly, the "Command line:" dumping tried to access a device before we'd done
any setup. Rather than implicitly creating a device here (which the old code did),
override the more clearly named driver_cmd_line_for_logging() in IOSSimulatorPort
and set flag to say that device_id_for_worker_number() doesn't need to return a real
device id.

  • Scripts/webkitpy/layout_tests/views/printing.py:

(print_options):
(Printer.print_config):

  • Scripts/webkitpy/port/base.py:

(Port.driver_cmd_line_for_logging):
(Port.driver_cmd_line): Deleted.

  • Scripts/webkitpy/port/driver.py:

(IOSSimulatorDriver.cmd_line):

  • Scripts/webkitpy/port/ios.py:

(IOSSimulatorPort.init):
(IOSSimulatorPort.driver_cmd_line_for_logging):
(IOSSimulatorPort._create_simulators):
(IOSSimulatorPort.setup_test_run):
(IOSSimulatorPort.clean_up_test_run):
(IOSSimulatorPort._create_device):
(IOSSimulatorPort):
(IOSSimulatorPort._remove_device):
(IOSSimulatorPort._testing_device):
(IOSSimulatorPort.device_id_for_worker_number):
(IOSSimulatorPort._set_device_class): Deleted.
(IOSSimulatorPort.testing_device): Deleted.

  • Scripts/webkitpy/port/port_testcase.py:

(PortTestCase.test_driver_cmd_line):

  • Scripts/webkitpy/xcode/simulator.py:

(Device.shutdown):
(Device.delete):
(Device.reset):
(Simulator.create_device):
(Simulator.remove_device):
(Simulator.device_number):
(Simulator.device_state_description):
(Simulator.wait_until_device_is_in_state):

Location:
trunk/Tools
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r204656 r204659  
     12016-08-18  Simon Fraser  <simon.fraser@apple.com>
     2
     3        REGRESSION (r204477): Running LayoutTests on ios-simulator became ~15 minutes slower
     4        https://bugs.webkit.org/show_bug.cgi?id=160985
     5
     6        Reviewed by Alexey Proskuryakov.
     7       
     8        r204477 removed @memoized on a couple of ios.py functions, causing them to instantiate
     9        a Simulator() on every call, which causes 'xcrun simctl list' to run. The functions
     10        must not be @memoized, because their return value depends on the value of simulator_device_type().
     11       
     12        Fix by adding some global state in simulator.py that tracks the created devices
     13        in a worker number -> Device dictionary. Explicitly create devices in _create_simulators(),
     14        and delete them in clean_up_test_run().
     15       
     16        Also explicitly called 'xcrun simctl shutdown' to shut down devices, since it seems
     17        that killing the Simulator apps isn't enough.
     18       
     19        Simulator tracks the devices in a global dictionary, since state needs to persist
     20        across different instances of IOSSimulatorPort.
     21       
     22        Annoyingly, the "Command line:" dumping tried to access a device before we'd done
     23        any setup. Rather than implicitly creating a device here (which the old code did),
     24        override the more clearly named driver_cmd_line_for_logging() in IOSSimulatorPort
     25        and set flag to say that device_id_for_worker_number() doesn't need to return a real
     26        device id.
     27
     28        * Scripts/webkitpy/layout_tests/views/printing.py:
     29        (print_options):
     30        (Printer.print_config):
     31        * Scripts/webkitpy/port/base.py:
     32        (Port.driver_cmd_line_for_logging):
     33        (Port.driver_cmd_line): Deleted.
     34        * Scripts/webkitpy/port/driver.py:
     35        (IOSSimulatorDriver.cmd_line):
     36        * Scripts/webkitpy/port/ios.py:
     37        (IOSSimulatorPort.__init__):
     38        (IOSSimulatorPort.driver_cmd_line_for_logging):
     39        (IOSSimulatorPort._create_simulators):
     40        (IOSSimulatorPort.setup_test_run):
     41        (IOSSimulatorPort.clean_up_test_run):
     42        (IOSSimulatorPort._create_device):
     43        (IOSSimulatorPort):
     44        (IOSSimulatorPort._remove_device):
     45        (IOSSimulatorPort._testing_device):
     46        (IOSSimulatorPort.device_id_for_worker_number):
     47        (IOSSimulatorPort._set_device_class): Deleted.
     48        (IOSSimulatorPort.testing_device): Deleted.
     49        * Scripts/webkitpy/port/port_testcase.py:
     50        (PortTestCase.test_driver_cmd_line):
     51        * Scripts/webkitpy/xcode/simulator.py:
     52        (Device.shutdown):
     53        (Device.delete):
     54        (Device.reset):
     55        (Simulator.create_device):
     56        (Simulator.remove_device):
     57        (Simulator.device_number):
     58        (Simulator.device_state_description):
     59        (Simulator.wait_until_device_is_in_state):
     60
    1612016-08-19  Alexey Proskuryakov  <ap@apple.com>
    262

  • trunk/Tools/Scripts/webkitpy/layout_tests/views/printing.py

    r204477 r204659  
    9595                  (self._options.time_out_ms, self._options.slow_time_out_ms))
    9696
    97         self._print_default('Command line: ' + ' '.join(self._port.driver_cmd_line()))
     97        self._print_default('Command line: ' + ' '.join(self._port.driver_cmd_line_for_logging()))
    9898        self._print_default('')
    9999

  • trunk/Tools/Scripts/webkitpy/port/base.py

    r204477 r204659  
    659659        return test_name
    660660
    661     def driver_cmd_line(self):
     661    def driver_cmd_line_for_logging(self):
    662662        """Prints the DRT command line that will be used."""
    663663        driver = self.create_driver(0)

  • trunk/Tools/Scripts/webkitpy/port/driver.py

    r204656 r204659  
    601601        relay_args = [
    602602            '-developerDir', self._port.developer_dir,
    603             '-udid', self._port.testing_device(self._worker_number).udid,
     603            '-udid', self._port.device_id_for_worker_number(self._worker_number),
    604604            '-productDir', product_dir,
    605605            '-app', dump_tool,

  • trunk/Tools/Scripts/webkitpy/port/ios.py

    r204477 r204659  
    9999
    100100        optional_device_class = self.get_option('device_class')
     101        self._printing_cmd_line = False
    101102        self._device_class = optional_device_class if optional_device_class else self.DEFAULT_DEVICE_CLASS
    102103        _log.debug('IOSSimulatorPort _device_class is %s', self._device_class)
    … …  
    108109            return 'WebKitTestRunnerApp.app'
    109110        return 'DumpRenderTree.app'
     111
     112    def driver_cmd_line_for_logging(self):
     113        # Avoid spinning up devices just for logging the commandline.
     114        self._printing_cmd_line = True
     115        result = super(IOSSimulatorPort, self).driver_cmd_line_for_logging()
     116        self._printing_cmd_line = False
     117        return result
    110118
    111119    @property
    … …  
    225233
    226234    def _set_device_class(self, device_class):
    227         # Ideally we'd ensure that no simulators are running when this is called.
    228235        self._device_class = device_class if device_class else self.DEFAULT_DEVICE_CLASS
    229236
    … …  
    237244
    238245        for i in xrange(self.child_processes()):
    239             Simulator.wait_until_device_is_in_state(self.testing_device(i).udid, Simulator.DeviceState.SHUTDOWN)
    240             Simulator.reset_device(self.testing_device(i).udid)
     246            self._create_device(i)
     247
     248        for i in xrange(self.child_processes()):
     249            device_udid = self._testing_device(i).udid
     250            Simulator.wait_until_device_is_in_state(device_udid, Simulator.DeviceState.SHUTDOWN)
     251            Simulator.reset_device(device_udid)
    241252
    242253    def setup_test_run(self, device_class=None):
    … …  
    251262
    252263        for i in xrange(self.child_processes()):
    253             device_udid = self.testing_device(i).udid
     264            device_udid = self._testing_device(i).udid
    254265            _log.debug('testing device %s has udid %s', i, device_udid)
    255266
    … …  
    264275        _log.info('Waiting for all iOS Simulators to finish booting.')
    265276        for i in xrange(self.child_processes()):
    266             Simulator.wait_until_device_is_booted(self.testing_device(i).udid)
     277            Simulator.wait_until_device_is_booted(self._testing_device(i).udid)
    267278
    268279    def _quit_ios_simulator(self):
    269         _log.debug("_quit_ios_simulator")
     280        _log.debug("_quit_ios_simulator killing all Simulator processes")
    270281        # FIXME: We should kill only the Simulators we started.
    271282        subprocess.call(["killall", "-9", "-m", "Simulator"])
    … …  
    285296        for i in xrange(self.child_processes()):
    286297            simulator_path = self.get_simulator_path(i)
    287             device_udid = self.testing_device(i).udid
     298            device_udid = self._testing_device(i).udid
     299            self._remove_device(i)
     300
    288301            if not os.path.exists(simulator_path):
    289302                continue
    … …  
    302315                self._filesystem.rmtree(saved_state_path)
    303316
    304                 Simulator().delete_device(device_udid)
    305317            except:
    306318                _log.warning('Unable to remove Simulator' + str(i))
    … …  
    370382        return stderr, crash_log
    371383
    372     def testing_device(self, number):
    373         # FIXME: rather than calling lookup_or_create_device every time, we should just store a mapping of
    374         # number to device_udid.
    375         device_type = self.simulator_device_type()
    376         _log.debug(' testing_device %s using device_type %s', number, device_type)
    377         return Simulator().lookup_or_create_device(device_type.name + ' WebKit Tester' + str(number), device_type, self.simulator_runtime)
     384    def _create_device(self, number):
     385        return Simulator.create_device(number, self.simulator_device_type(), self.simulator_runtime)
     386
     387    def _remove_device(self, number):
     388        Simulator.remove_device(number)
     389
     390    def _testing_device(self, number):
     391        return Simulator.device_number(number)
     392
     393    # This is only exposed so that IOSSimulatorDriver can use it.
     394    def device_id_for_worker_number(self, number):
     395        if self._printing_cmd_line:
     396            return '<dummy id>'
     397        return self._testing_device(number).udid
    378398
    379399    def get_simulator_path(self, suffix=""):

  • trunk/Tools/Scripts/webkitpy/port/port_testcase.py

    r192944 r204659  
    101101    def test_driver_cmd_line(self):
    102102        port = self.make_port()
    103         self.assertTrue(len(port.driver_cmd_line()))
     103        self.assertTrue(len(port.driver_cmd_line_for_logging()))
    104104
    105105        options = MockOptions(additional_drt_flag=['--foo=bar', '--foo=baz'])
    106106        port = self.make_port(options=options)
    107         cmd_line = port.driver_cmd_line()
     107        cmd_line = port.driver_cmd_line_for_logging()
    108108        self.assertTrue('--foo=bar' in cmd_line)
    109109        self.assertTrue('--foo=baz' in cmd_line)

  • trunk/Tools/Scripts/webkitpy/xcode/simulator.py

    r204341 r204659  
    219219
    220220    @classmethod
     221    def shutdown(cls, udid):
     222        """
     223        Shut down the given CoreSimulator device.
     224        :param udid: The udid of the device.
     225        :type udid: str
     226        """
     227        device_state = Simulator.device_state(udid)
     228        if device_state == Simulator.DeviceState.BOOTING or device_state == Simulator.DeviceState.BOOTED:
     229            try:
     230                _log.debug('xcrun simctl shutdown %s', udid)
     231                subprocess.check_call(['xcrun', 'simctl', 'shutdown', udid])
     232            except subprocess.CalledProcessError:
     233                raise RuntimeError('"xcrun simctl shutdown" failed')
     234
     235        Simulator.wait_until_device_is_in_state(udid, Simulator.DeviceState.SHUTDOWN)
     236
     237    @classmethod
    221238    def delete(cls, udid):
    222239        """
    … …  
    225242        :type udid: str
    226243        """
    227         _log.debug('deleting device %s', udid)
    228         Simulator.wait_until_device_is_in_state(udid, Simulator.DeviceState.SHUTDOWN)
     244        Device.shutdown(udid)
    229245        try:
    230246            _log.debug('xcrun simctl delete %s', udid)
    … …  
    240256        :type udid: str
    241257        """
    242         _log.debug('resetting device %s', udid)
    243         Simulator.wait_until_device_is_in_state(udid, Simulator.DeviceState.SHUTDOWN)
     258        Device.shutdown(udid)
    244259        try:
    245260            _log.debug('xcrun simctl erase %s', udid)
    … …  
    282297        '\s*(?P<name>[^(]+ )\((?P<udid>[^)]+)\) \((?P<state>[^)]+)\)( \((?P<availability>[^)]+)\))?')
    283298
     299    _managed_devices = {}
     300
    284301    def __init__(self, host=None):
    285302        self._host = host or Host()
    … …  
    297314        SHUTTING_DOWN = 4
    298315
     316    NAME_FOR_STATE = [
     317        'CREATING',
     318        'SHUTDOWN',
     319        'BOOTING',
     320        'BOOTED',
     321        'SHUTTING_DOWN'
     322    ]
     323
     324    @staticmethod
     325    def create_device(number, device_type, runtime):
     326        device = Simulator().lookup_or_create_device(device_type.name + ' WebKit Tester' + str(number), device_type, runtime)
     327        _log.debug('created device {} {}'.format(number, device))
     328        assert(len(Simulator._managed_devices) == number)
     329        Simulator._managed_devices[number] = device
     330
     331    @staticmethod
     332    def remove_device(number):
     333        if not Simulator._managed_devices[number]:
     334            return
     335        device_udid = Simulator._managed_devices[number].udid
     336        _log.debug('removing device {} {}'.format(number, device_udid))
     337        del Simulator._managed_devices[number]
     338        Simulator.delete_device(device_udid)
     339
     340    @staticmethod
     341    def device_number(number):
     342        return Simulator._managed_devices[number]
     343
     344    @staticmethod
     345    def device_state_description(state):
     346        if (state == Simulator.DeviceState.DOES_NOT_EXIST):
     347            return 'DOES_NOT_EXIST'
     348        return Simulator.NAME_FOR_STATE[state]
     349
    299350    @staticmethod
    300351    def wait_until_device_is_booted(udid, timeout_seconds=60 * 5):
    … …  
    316367    @staticmethod
    317368    def wait_until_device_is_in_state(udid, wait_until_state, timeout_seconds=60 * 5):
    318         _log.debug('waiting for device %s to enter state %s with timeout %s', udid, wait_until_state, timeout_seconds)
     369        _log.debug('waiting for device %s to enter state %s with timeout %s', udid, Simulator.device_state_description(wait_until_state), timeout_seconds)
    319370        with timeout(seconds=timeout_seconds):
    320371            device_state = Simulator.device_state(udid)
    321372            while (device_state != wait_until_state):
    322373                device_state = Simulator.device_state(udid)
    323                 _log.debug(' device state %s', device_state)
     374                _log.debug(' device state %s', Simulator.device_state_description(device_state))
    324375                time.sleep(0.5)
    325376
    326377        end_state = Simulator.device_state(udid)
    327378        if (end_state != wait_until_state):
    328             raise RuntimeError('Timed out waiting for simulator device to enter state {0}; current state is {1}'.format(wait_until_state, end_state))
     379            raise RuntimeError('Timed out waiting for simulator device to enter state {0}; current state is {1}'.format(Simulator.device_state_description(wait_until_state), Simulator.device_state_description(end_state)))
    329380
    330381    @staticmethod
Note: See TracChangeset for help on using the changeset viewer.