Skip to content

Commit

Permalink
[browserperfdash-benchmark] Allow to send benchmark data with the tim…
Browse files Browse the repository at this point in the history
…estamp of the git checkout and use it on the bots

https://bugs.webkit.org/show_bug.cgi?id=258920

Reviewed by Carlos Garcia Campos.

Now that we are running several bots in parallel for the same worker
there is no guarantee that the bots will finish in the same order
than they started. So it can happen that we send the benchmark data
in different ordering than the commits on the repository.

To avoid this (and also allow re-testing old checkouts) this implements
a way of sending to the server dashboard as timestamp of the benchmark
the same timestamp of the git commit that is checked out (HEAD).

And since the bots always check-out the same git commit than the build
they are testing, we can use this there and run everything in parallel
safely.

* Tools/CISupport/build-webkit-org/steps.py:
(RunBenchmarkTests):
* Tools/Scripts/webkitpy/browserperfdash/browserperfdash_runner.py:
(parse_args):
(BrowserPerfDashRunner.__init__):

Canonical link: https://commits.webkit.org/265798@main
  • Loading branch information
clopez committed Jul 6, 2023
1 parent 371f9f1 commit 9204150
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
3 changes: 2 additions & 1 deletion Tools/CISupport/build-webkit-org/steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,8 @@ class RunBenchmarkTests(shell.Test):
descriptionDone = ["benchmark tests"]
command = ["python3", "Tools/Scripts/browserperfdash-benchmark", "--plans-from-config",
"--config-file", "../../browserperfdash-benchmark-config.txt",
"--browser-version", WithProperties("%(archive_revision)s")]
"--browser-version", WithProperties("%(archive_revision)s"),
"--timestamp-from-repo", "."]

def start(self):
platform = self.getProperty("platform")
Expand Down
27 changes: 22 additions & 5 deletions Tools/Scripts/webkitpy/browserperfdash/browserperfdash_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import logging
import os
import re
import subprocess
import tempfile
import urllib
from datetime import datetime
Expand All @@ -54,11 +55,13 @@ def parse_args():
parser.add_argument('--local-copy', dest='localCopy', help='Path to a local copy of the benchmark (e.g. PerformanceTests/SunSpider/).')
parser.add_argument('--count', dest='countOverride', type=int, help='Number of times to run the benchmark (e.g. 5).')
parser.add_argument('--timeout', dest='timeoutOverride', type=int, help='Number of seconds to wait for the benchmark to finish (e.g. 600).')
parser.add_argument('--timestamp', dest='timestamp', type=int, help='Date of when the benchmark was run that will be sent to the performance dashboard server. Format is Unix timestamp (second since epoch). Optional. The server will use as date "now" if not specified.')
mutual_group = parser.add_mutually_exclusive_group(required=True)
mutual_group.add_argument('--plan', dest='plan', help='Benchmark plan to run. e.g. speedometer, jetstream')
mutual_group.add_argument('--plans-from-config', action='store_true', help='Run the list of plans specified in the config file.')
mutual_group.add_argument('--allplans', action='store_true', help='Run all available benchmark plans sequentially')
parser_group_timestamp = parser.add_mutually_exclusive_group(required=False)
parser_group_timestamp.add_argument('--timestamp', dest='timestamp', type=int, help='Date of when the benchmark was run that will be sent to the performance dashboard server. Format is Unix timestamp (second since epoch). Optional. The server will use as date "now" if not specified.')
parser_group_timestamp.add_argument('--timestamp-from-repo', dest='local_path_to_repo', default=None, help='Use the commit date of the checked-out git commit (HEAD) on the specified local path as the date to send to the server of when the benchmark was run. Useful for running bots in parallel (or even re-testing old checkouts) and ensuring that the timestamps of the benchmark data on the server gets the same value than the commit date of the checked-out commit on the given repository.')
parser_group_plan_selection = parser.add_mutually_exclusive_group(required=True)
parser_group_plan_selection.add_argument('--plan', dest='plan', help='Benchmark plan to run. e.g. speedometer, jetstream')
parser_group_plan_selection.add_argument('--plans-from-config', action='store_true', help='Run the list of plans specified in the config file.')
parser_group_plan_selection.add_argument('--allplans', action='store_true', help='Run all available benchmark plans sequentially')

parser.add_argument('browser_args', nargs='*', help='Additional arguments to pass to the browser process. These are positional arguments and must follow all other arguments. If the pass through arguments begin with a dash, use `--` before the argument list begins.')
args = parser.parse_args()
Expand Down Expand Up @@ -91,6 +94,20 @@ def __init__(self, args):
self._result_data['timestamp'] = args.timestamp
date_str = datetime.fromtimestamp(self._result_data['timestamp']).isoformat()
_log.info('Will send the benchmark data as if it was generated on date: {date}'.format(date=date_str))
elif args.local_path_to_repo is not None:
if not os.path.isdir(args.local_path_to_repo):
raise ValueError('The value "{path}" to the local repository is not a valid directory'.format(path=args.local_path_to_repo))
_log.info('Checking the timestamp of the git commit checked-out (HEAD) on the repository at path "{repo_path}"'.format(repo_path=args.local_path_to_repo))
timestamp = subprocess.check_output(['git', 'log', '-1', '--pretty=%ct', 'HEAD'], cwd=args.local_path_to_repo)
timestamp = timestamp.decode('utf-8').strip()
if not timestamp.isnumeric():
raise ValueError('The git command to find the timestamp on the HEAD git commit returned an unexpected string: {timestamp}'.format(timestamp=timestamp))
timestamp = int(timestamp)
if timestamp < 0:
raise ValueError('The git command to find the timestamp on the HEAD git commit returned a negative timestamp: {timestamp}'.format(timestamp=timestamp))
date_str = datetime.fromtimestamp(timestamp).isoformat()
_log.info('Will send the benchmark data as if it was generated on date: {date}'.format(date=date_str))
self._result_data['timestamp'] = timestamp

# The precedence order for setting the value for the arguments is:
# 1 - What the user defines via command line switches (current_value is not None).
Expand Down

0 comments on commit 9204150

Please sign in to comment.