Attachment 368380 Details for Bug 197339 – patch

[patch] patch

0001-run-benchmarks-should-have-an-intial-prep-and-restor.patch (text/plain), 9.07 KB, created by Stephanie Lewis on 2019-04-26 19:13:40 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Stephanie Lewis

Created: 2019-04-26 19:13:40 PDT

Size: 9.07 KB

patch

obsolete

>From 2dee26c74426653148db55a90acc99f19cac9a9d Mon Sep 17 00:00:00 2001
>From: Stephanie Lewis <slewis@apple.com>
>Date: Fri, 26 Apr 2019 19:12:23 -0700
>Subject: [PATCH] run-benchmarks should have an intial prep and restore env
> call for tasks that are too expensive to do for every iteration
> https://bugs.webkit.org/show_bug.cgi?id=197339
>
>Reviewed by NOBODY (OOPS!).
>
>	Add two new functions prepare_inital_env and restore_env_after_all_testing around the entire
>test run for setup steps that are too expensive to do every iteration.
>
>* Scripts/webkitpy/benchmark_runner/benchmark_runner.py:
>(BenchmarkRunner._run_benchmark):
>* Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py:
>(BrowserDriver):
>(BrowserDriver.prepare_initial_env):
>(BrowserDriver.restore_env_after_all_testing):
>* Scripts/webkitpy/benchmark_runner/browser_driver/linux_browser_driver.py:
>(LinuxBrowserDriver.prepare_initial_env):
>(LinuxBrowserDriver.restore_env_after_all_testing):
>* Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py:
>(OSXBrowserDriver.prepare_initial_env):
>(OSXBrowserDriver.restore_env_after_all_testing):
>---
> Tools/ChangeLog                               | 23 ++++++++
> .../benchmark_runner/benchmark_runner.py      | 54 ++++++++++---------
> .../browser_driver/browser_driver.py          |  8 +++
> .../browser_driver/linux_browser_driver.py    |  6 +++
> .../browser_driver/osx_browser_driver.py      |  6 +++
> 5 files changed, 72 insertions(+), 25 deletions(-)
>
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index ee1596e53ff1..1cb9b16c24b4 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,26 @@
>+2019-04-26  Stephanie Lewis  <slewis@apple.com>
>+
>+        run-benchmarks should have an intial prep and restore env call for tasks that are too expensive to do for every iteration
>+        https://bugs.webkit.org/show_bug.cgi?id=197339
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+	Add two new functions prepare_inital_env and restore_env_after_all_testing around the entire
>+        test run for setup steps that are too expensive to do every iteration.
>+
>+        * Scripts/webkitpy/benchmark_runner/benchmark_runner.py:
>+        (BenchmarkRunner._run_benchmark):
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py:
>+        (BrowserDriver):
>+        (BrowserDriver.prepare_initial_env):
>+        (BrowserDriver.restore_env_after_all_testing):
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/linux_browser_driver.py:
>+        (LinuxBrowserDriver.prepare_initial_env):
>+        (LinuxBrowserDriver.restore_env_after_all_testing):
>+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py:
>+        (OSXBrowserDriver.prepare_initial_env):
>+        (OSXBrowserDriver.restore_env_after_all_testing):
>+
> 2019-04-25  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] linkPolymorphicCall now does GC
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py b/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py
>index 757d8b8adad6..94958c8ce831 100755
>--- a/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py
>@@ -77,32 +77,36 @@ class BenchmarkRunner(object):
>     def _run_benchmark(self, count, web_root):
>         results = []
>         debug_outputs = []
>-        for iteration in xrange(1, count + 1):
>-            _log.info('Start the iteration {current_iteration} of {iterations} for current benchmark'.format(current_iteration=iteration, iterations=count))
>-            try:
>-                self._browser_driver.prepare_env(self._config)
>-
>-                if 'entry_point' in self._plan:
>-                    result = self._run_one_test(web_root, self._plan['entry_point'])
>-                    debug_outputs.append(result.pop('debugOutput', None))
>-                    assert(result)
>-                    results.append(result)
>-                elif 'test_files' in self._plan:
>-                    run_result = {}
>-                    for test in self._plan['test_files']:
>-                        result = self._run_one_test(web_root, test)
>-                        assert(result)
>-                        run_result = self._merge(run_result, result)
>+        try:
>+            self._browser_driver.prepare_initial_env(self._config)
>+            for iteration in xrange(1, count + 1):
>+                _log.info('Start the iteration {current_iteration} of {iterations} for current benchmark'.format(current_iteration=iteration, iterations=count))
>+                try:
>+                    self._browser_driver.prepare_env(self._config)
>+
>+                    if 'entry_point' in self._plan:
>+                        result = self._run_one_test(web_root, self._plan['entry_point'])
>                         debug_outputs.append(result.pop('debugOutput', None))
>-
>-                    results.append(run_result)
>-                else:
>-                    raise Exception('Plan does not contain entry_point or test_files')
>-
>-            finally:
>-                self._browser_driver.restore_env()
>-
>-            _log.info('End the iteration {current_iteration} of {iterations} for current benchmark'.format(current_iteration=iteration, iterations=count))
>+                        assert(result)
>+                        results.append(result)
>+                    elif 'test_files' in self._plan:
>+                        run_result = {}
>+                        for test in self._plan['test_files']:
>+                            result = self._run_one_test(web_root, test)
>+                            assert(result)
>+                            run_result = self._merge(run_result, result)
>+                            debug_outputs.append(result.pop('debugOutput', None))
>+
>+                        results.append(run_result)
>+                    else:
>+                        raise Exception('Plan does not contain entry_point or test_files')
>+
>+                finally:
>+                    self._browser_driver.restore_env()
>+
>+                _log.info('End the iteration {current_iteration} of {iterations} for current benchmark'.format(current_iteration=iteration, iterations=count))
>+        finally:
>+            self._browser_driver.restore_env_after_all_testing()
> 
>         results = self._wrap(results)
>         output_file = self._output_file if self._output_file else self._plan['output_file']
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py
>index f1af57e9ade2..2c1a86001749 100755
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py
>@@ -14,6 +14,10 @@ class BrowserDriver(object):
>     def prepare_env(self, config):
>         pass
> 
>+    @abstractmethod
>+    def prepare_initial_env(self, config):
>+        pass
>+
>     @abstractmethod
>     def launch_url(self, url, options, browser_build_path=None, browser_path=None):
>         pass
>@@ -34,6 +38,10 @@ class BrowserDriver(object):
>     def restore_env(self):
>         pass
> 
>+    @abstractmethod
>+    def restore_env_after_all_testing(self):
>+        pass
>+
>     @property
>     def webdriver_binary_path(self):
>         return get_driver_binary_path(self.browser_name)
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/linux_browser_driver.py b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/linux_browser_driver.py
>index 728d50b4a7b9..61abc64108ba 100644
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/linux_browser_driver.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/linux_browser_driver.py
>@@ -58,9 +58,15 @@ class LinuxBrowserDriver(BrowserDriver):
>         self._test_environ = dict(os.environ)
>         self._test_environ['HOME'] = self._temp_profiledir
> 
>+    def prepare_initial_env(self, config):
>+        pass
>+
>     def restore_env(self):
>         force_remove(self._temp_profiledir)
> 
>+    def restore_env_after_all_testing(self):
>+        pass
>+
>     def close_browsers(self):
>         if self._browser_process:
>             if self._browser_process.poll() is None:  # still running
>diff --git a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py
>index 14946207b1bf..ee2dbba4b69e 100644
>--- a/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py
>+++ b/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_browser_driver.py
>@@ -16,6 +16,9 @@ class OSXBrowserDriver(BrowserDriver):
>     platform = 'osx'
>     bundle_id = None
> 
>+    def prepare_initial_env(self, config):
>+        pass
>+
>     def prepare_env(self, config):
>         self.close_browsers()
>         from Quartz import CGWarpMouseCursorPosition
>@@ -29,6 +32,9 @@ class OSXBrowserDriver(BrowserDriver):
>             write_defaults('com.apple.dock', 'launchanim', True)
>             self._terminate_processes('Dock', 'com.apple.dock')
> 
>+    def restore_env_after_all_testing(self):
>+        pass
>+
>     def close_browsers(self):
>         self._terminate_processes(self.process_name, self.bundle_id)
> 
>-- 
>2.20.1 (Apple Git-116)
>

Flags:

dewei_zhu: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197339: 368380