Skip to content

Commit

Permalink
tbmv3 validator: Handle null values and empty histograms
Browse files Browse the repository at this point in the history
TBMv2 allows histograms to be empty and/or contain null values. We
don't support that in TBMv3. Filed crbug.com/1183797 to explore
whether we need to do that.

Change-Id: I62784241c3f66ffc800be29f44c8790bf484d9e2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2726982
Commit-Queue: Deep Roy <dproy@chromium.org>
Auto-Submit: Deep Roy <dproy@chromium.org>
Reviewed-by: Mikhail Khokhlov <khokhlov@google.com>
Cr-Commit-Position: refs/heads/master@{#859036}
  • Loading branch information
deepanjanroy authored and Chromium LUCI CQ committed Mar 2, 2021
1 parent 84ef659 commit d05f6fe
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 11 deletions.
26 changes: 18 additions & 8 deletions tools/perf/cli_tools/tbmv3/validators/simple_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
"""
Validates the rendering/frame_times metric.
Implementation for simple validators defined in simple_configs.pyl.
"""

import sys
Expand All @@ -25,13 +25,10 @@ def CheckConfig(simple_config):
(name, CONFIG_FORMAT))


def GetHistogram(histogram_set, name, metric, version):
def OptionalGetHistogram(histogram_set, name, metric, version):
hists = histogram_set.GetHistogramsNamed(name)
if len(hists) == 0:
msg = ('List of histograms produced by TBM%s metric %s:\n' %
(version, metric))
msg += '\n'.join([h.name for h in histogram_set])
raise Exception('Histogram %s not found.\n%s' % (name, msg))
return None
if len(hists) > 1:
raise Exception('Multiple histograms named %s found for TBM%s metric %s' %
(name, version, metric))
Expand Down Expand Up @@ -60,8 +57,21 @@ def CompareHistograms(test_ctx):
raise Exception('v3_histogram must be either string of v3_histogram '
' name of (v3_hist_name, precision) tuple.')

v2_hist = GetHistogram(v2_histograms, v2_hist_name, v2_metric, 'v2')
v3_hist = GetHistogram(v3_histograms, v3_hist_name, v3_metric, 'v3')
v2_hist = OptionalGetHistogram(v2_histograms, v2_hist_name, v2_metric, 'v2')
v3_hist = OptionalGetHistogram(v3_histograms, v3_hist_name, v3_metric, 'v3')

if (v2_hist is None) or (v2_hist.num_values == 0):
if (v3_hist is not None) and (v3_hist.num_values > 0):
raise Exception('v3 metric produced non-empty histogram %s, but '
'equivalent histogram %s is not present or empty '
'in v2 metric' % (v3_hist_name, v2_hist_name))
continue

if v3_hist is None:
msg = ('List of histograms produced by v3 metric %s:\n' % (v3_metric))
msg += '\n'.join([h.name for h in v3_histograms])
raise Exception('Histogram %s not produced by v3 metric\n%s' %
(v3_hist_name, msg))

try:
utils.AssertHistogramStatsAlmostEqual(test_ctx, v2_hist, v3_hist,
Expand Down
6 changes: 3 additions & 3 deletions tools/perf/cli_tools/tbmv3/validators/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.


def AssertHistogramStatsAlmostEqual(test_ctx, v2_hist, v3_hist, precision=1e-3):
"""Asserts major histogram statistics are close enough.
Expand All @@ -21,8 +20,9 @@ def AssertHistogramSamplesAlmostEqual(test_ctx,
v2_hist,
v3_hist,
precision=1e-3):
v2_samples = v2_hist.sample_values
v3_samples = v3_hist.sample_values
v2_samples = [s for s in v2_hist.sample_values if s is not None]
v3_samples = [s for s in v3_hist.sample_values if s is not None]

test_ctx.assertEqual(len(v2_samples), len(v3_samples))
v2_samples.sort()
v3_samples.sort()
Expand Down

0 comments on commit d05f6fe

Please sign in to comment.