Skip to content

Commit 83245bd

Browse files
Patrik HöglundCommit Bot
Patrik Höglund
authored and
Commit Bot
committed
Make the dashboard upload script read protos instead of JSON.
I had to pivot and make tests output protos instead of JSON. I basically move the proto -> JSON conversion into this script instead of doing it in the test binary. This is a temporary state. Later it will be enough to just read up the file and pass it straight to the Catapult implementation, once it learns to de-serialize the proto directly. Bug: chromium:1029452 Change-Id: I7ce992eeeb1a5ae0f20eed54174b08b496e74dfd Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166920 Commit-Queue: Patrik Höglund <phoglund@webrtc.org> Reviewed-by: Artem Titov <titovartem@webrtc.org> Cr-Commit-Position: refs/heads/master@{#30419}
1 parent 152b435 commit 83245bd

File tree

3 files changed

+63
-8
lines changed

3 files changed

+63
-8
lines changed

BUILD.gn

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ if (!build_with_chromium) {
8383
deps += [
8484
"audio:low_bandwidth_audio_test",
8585
"logging:rtc_event_log_rtp_dump",
86+
"tools_webrtc/perf:webrtc_dashboard_upload",
8687
]
8788
}
8889
}
@@ -336,7 +337,9 @@ config("common_config") {
336337
cflags += [ "/wd4702" ] # unreachable code
337338

338339
# MSVC 2019 warning suppressions for C++17 compiling
339-
cflags += [ "/wd5041" ] # out-of-line definition for constexpr static data member is not needed and is deprecated in C++17
340+
cflags +=
341+
[ "/wd5041" ] # out-of-line definition for constexpr static data
342+
# member is not needed and is deprecated in C++17
340343
}
341344
}
342345

tools_webrtc/perf/BUILD.gn

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# Copyright (c) 2020 The WebRTC project authors. All Rights Reserved.
2+
#
3+
# Use of this source code is governed by a BSD-style license
4+
# that can be found in the LICENSE file in the root of the source
5+
# tree. An additional intellectual property rights grant can be found
6+
# in the file PATENTS. All contributing project authors may
7+
# be found in the AUTHORS file in the root of the source tree.
8+
9+
import("../../webrtc.gni")
10+
11+
if (rtc_enable_protobuf) {
12+
group("webrtc_dashboard_upload") {
13+
data = [ "webrtc_dashboard_upload.py" ]
14+
data_deps =
15+
[ "//third_party/catapult/tracing/tracing/proto:histogram_proto" ]
16+
}
17+
}

tools_webrtc/perf/webrtc_dashboard_upload.py

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"""Adds build info to perf results and uploads them.
1111
1212
The tests don't know which bot executed the tests or at what revision, so we
13-
need to take their output and enrich it with this information. We load the JSON
13+
need to take their output and enrich it with this information. We load the proto
1414
from the tests, add the build information as shared diagnostics and then
1515
upload it to the dashboard.
1616
@@ -26,15 +26,28 @@
2626
import subprocess
2727
import zlib
2828

29+
# We just yank the python scripts we require into the PYTHONPATH. You could also
30+
# imagine a solution where we use for instance protobuf:py_proto_runtime to copy
31+
# catapult and protobuf code to out/, but this approach is allowed by
32+
# convention. Fortunately neither catapult nor protobuf require any build rules
33+
# to be executed. We can't do this for the histogram proto stub though because
34+
# it's generated; see _LoadHistogramSetFromProto.
35+
#
36+
# It would be better if there was an equivalent to py_binary in GN, but there's
37+
# not.
2938
SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__))
3039
CHECKOUT_ROOT = os.path.abspath(os.path.join(SCRIPT_DIR, os.pardir, os.pardir))
3140
sys.path.insert(0, os.path.join(CHECKOUT_ROOT, 'third_party', 'catapult',
3241
'tracing'))
42+
sys.path.insert(0, os.path.join(CHECKOUT_ROOT, 'third_party', 'protobuf',
43+
'python'))
3344

3445
from tracing.value import histogram_set
3546
from tracing.value.diagnostics import generic_set
3647
from tracing.value.diagnostics import reserved_infos
3748

49+
from google.protobuf import json_format
50+
3851

3952
def _GenerateOauthToken():
4053
args = ['luci-auth', 'token']
@@ -72,13 +85,33 @@ def _SendHistogramSet(url, histograms, oauth_token):
7285
return response, content
7386

7487

75-
def _LoadHistogramSetFromJson(options):
88+
def _LoadHistogramSetFromProto(options):
89+
# The webrtc_dashboard_upload gn rule will build the protobuf stub for python,
90+
# so put it in the path for this script before we attempt to import it.
91+
histogram_proto_path = os.path.join(options.outdir, 'pyproto', 'tracing',
92+
'tracing', 'proto')
93+
sys.path.insert(0, histogram_proto_path)
94+
95+
# TODO(https://crbug.com/1029452): Get rid of this import hack once we can
96+
# just hand the contents of input_results_file straight to the histogram set.
97+
try:
98+
import histogram_pb2
99+
except ImportError:
100+
raise ImportError('Could not find histogram_pb2. You need to build the '
101+
'webrtc_dashboard_upload target before invoking this '
102+
'script. Expected to find '
103+
'histogram_pb2 in %s.' % histogram_proto_path)
104+
76105
with options.input_results_file as f:
77-
json_data = json.load(f)
106+
histograms = histogram_pb2.HistogramSet()
107+
histograms.ParseFromString(f.read())
78108

79-
histograms = histogram_set.HistogramSet()
80-
histograms.ImportDicts(json_data)
81-
return histograms
109+
# TODO(https://crbug.com/1029452): Don't convert to JSON as a middle step once
110+
# there is a proto de-serializer ready in catapult.
111+
json_data = json.loads(json_format.MessageToJson(histograms))
112+
hs = histogram_set.HistogramSet()
113+
hs.ImportDicts(json_data)
114+
return hs
82115

83116

84117
def _AddBuildInfo(histograms, options):
@@ -127,14 +160,16 @@ def _CreateParser():
127160
help='A JSON file with output from WebRTC tests.')
128161
parser.add_argument('--output-json-file', type=argparse.FileType('w'),
129162
help='Where to write the output (for debugging).')
163+
parser.add_argument('--outdir', required=True,
164+
help='Path to the local out/ dir (usually out/Default)')
130165
return parser
131166

132167

133168
def main(args):
134169
parser = _CreateParser()
135170
options = parser.parse_args(args)
136171

137-
histograms = _LoadHistogramSetFromJson(options)
172+
histograms = _LoadHistogramSetFromProto(options)
138173
_AddBuildInfo(histograms, options)
139174

140175
if options.output_json_file:

0 commit comments

Comments
 (0)