Skip to content

Commit

Permalink
Remove perf_expectations
Browse files Browse the repository at this point in the history
This is a modified reland of [1] with most of the changes to sizes.py reverted.
Those changes are still used by the perf dashboard so must be kept around for
the time being.

[1] https://chromium.googlesource.com/chromium/src.git/+/5f552b39961e3b36e018fdd7d3d452526bde2e4f

R=dpranke
Bug: 572393
Cq-Include-Trybots: luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng;luci.chromium.try:win_archive;luci.chromium.try:win_x64_archive;master.tryserver.chromium.android:android_archive_rel_ng

Change-Id: I459387d50f4a328192110c7b75983523a14f787d
Reviewed-on: https://chromium-review.googlesource.com/c/1255303
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597614}
  • Loading branch information
tanderson-google authored and Commit Bot committed Oct 8, 2018
1 parent dd42dc8 commit 9d5ef42
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 1,178 deletions.
2 changes: 1 addition & 1 deletion docs/static_initializers.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ http://neugierig.org/software/chromium/notes/2011/08/static-initializers.html
# How Static Initializers are Checked

* For Linux and Mac:
* The expected count is stored in [//tools/perf_expectations/perf_expectations.json](https://cs.chromium.org/chromium/src/tools/perf_expectations/perf_expectations.json)
* The expected count is stored in [//infra/scripts/legacy/scripts/slave/chromium/sizes.py](https://cs.chromium.org/chromium/src/infra/scripts/legacy/scripts/slave/chromium/sizes.py)
* For Android:
* The expected count is stored in the build target [//chrome/android:monochrome_static_initializers](https://cs.chromium.org/chromium/src/chrome/android/BUILD.gn)

Expand Down
52 changes: 40 additions & 12 deletions infra/scripts/legacy/scripts/slave/chromium/sizes.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,17 @@
SRC_DIR = os.path.abspath(
os.path.join(os.path.dirname(__file__), '..', '..', '..', '..', '..', '..'))

EXPECTED_LINUX_SI_COUNTS = {
'chrome': 8,
'nacl_helper': 6,
'nacl_helper_bootstrap': 0,
}


class ResultsCollector(object):
def __init__(self):
self.results = {}
self.failures = []

def add_result(self, name, identifier, value, units):
assert name not in self.results
Expand All @@ -45,6 +52,9 @@ def add_result(self, name, identifier, value, units):
# Legacy printing, previously used for parsing the text logs.
print 'RESULT %s: %s= %s %s' % (name, identifier, value, units)

def add_failure(self, failure):
self.failures.append(failure)


def get_size(filename):
return os.stat(filename)[stat.ST_SIZE]
Expand Down Expand Up @@ -165,6 +175,10 @@ def main_mac(options, args, results_collector):
# For Release builds only, use dump-static-initializers.py to print the
# list of static initializers.
if si_count > 0 and options.target == 'Release':
result = 125
results_collector.add_failure(
'Expected 0 static initializers in %s, but found %d' %
(chromium_framework_executable, si_count))
print '\n# Static initializers in %s:' % chromium_framework_executable

# First look for a dSYM to get information about the initializers. If
Expand Down Expand Up @@ -230,7 +244,7 @@ def main_mac(options, args, results_collector):
return 66


def check_linux_binary(target_dir, binary_name, options):
def check_linux_binary(target_dir, binary_name, options, results_collector):
"""Collect appropriate size information about the built Linux binary given.
Returns a tuple (result, sizes). result is the first non-zero exit
Expand Down Expand Up @@ -302,16 +316,23 @@ def get_elf_section_size(readelf_stdout, section_name):

# For Release builds only, use dump-static-initializers.py to print the list
# of static initializers.
if si_count > 0 and options.target == 'Release':
build_dir = os.path.dirname(target_dir)
dump_static_initializers = os.path.join(os.path.dirname(build_dir),
'tools', 'linux',
'dump-static-initializers.py')
result, stdout = run_process(result, [dump_static_initializers,
'-d', binary_file])
print '\n# Static initializers in %s:' % binary_file
print_si_fail_hint('tools/linux/dump-static-initializers.py')
print stdout
if options.target == 'Release':
if (binary_name in EXPECTED_LINUX_SI_COUNTS and
si_count > EXPECTED_LINUX_SI_COUNTS[binary_name]):
result = 125
results_collector.add_failure(
'Expected <= %d static initializers in %s, but found %d' %
(EXPECTED_LINUX_SI_COUNTS[binary_name], binary_name, si_count))
if si_count > 0:
build_dir = os.path.dirname(target_dir)
dump_static_initializers = os.path.join(os.path.dirname(build_dir),
'tools', 'linux',
'dump-static-initializers.py')
result, stdout = run_process(result, [dump_static_initializers,
'-d', binary_file])
print '\n# Static initializers in %s:' % binary_file
print_si_fail_hint('tools/linux/dump-static-initializers.py')
print stdout

# Determine if the binary has the DT_TEXTREL marker.
result, stdout = run_process(result, ['readelf', '-Wd', binary_file])
Expand Down Expand Up @@ -350,7 +371,8 @@ def main_linux(options, args, results_collector):
totals = {}

for binary in binaries:
this_result, this_sizes = check_linux_binary(target_dir, binary, options)
this_result, this_sizes = check_linux_binary(target_dir, binary, options,
results_collector)
if result == 0:
result = this_result
for name, identifier, totals_id, value, units in this_sizes:
Expand Down Expand Up @@ -534,6 +556,8 @@ def main():
help='specify platform (%s) [default: %%default]'
% ', '.join(platforms))
option_parser.add_option('--json', help='Path to JSON output file')
option_parser.add_option('--failures',
help='Path to JSON output file for failures')

options, args = option_parser.parse_args()

Expand All @@ -554,6 +578,10 @@ def main():
with open(options.json, 'w') as f:
json.dump(results_collector.results, f)

if options.failures:
with open(options.failures, 'w') as f:
json.dump(results_collector.failures, f)

return rc


Expand Down
38 changes: 3 additions & 35 deletions testing/scripts/sizes.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,51 +44,19 @@ def main_run(script_args):
os.path.join(
common.SRC_DIR, 'infra', 'scripts', 'legacy', 'scripts', 'slave',
'chromium', 'sizes.py'),
'--json', tempfile_path
'--failures', tempfile_path
]
if args.platform:
sizes_cmd.extend(['--platform', args.platform])
rc = common.run_runtest(script_args, runtest_args + sizes_cmd)
with open(tempfile_path) as f:
results = json.load(f)

with open(os.path.join(common.SRC_DIR, 'tools', 'perf_expectations',
'perf_expectations.json')) as f:
perf_expectations = json.load(f)

valid = (rc == 0)
failures = []

for name, result in results.iteritems():
fqtn = '%s/%s/%s' % (args.prefix, name, result['identifier'])
if fqtn not in perf_expectations:
continue

if perf_expectations[fqtn]['type'] != 'absolute':
print 'ERROR: perf expectation %r is not yet supported' % fqtn
valid = False
continue

actual = result['value']
expected = perf_expectations[fqtn]['regress']
better = perf_expectations[fqtn]['better']
check_result = ((actual <= expected) if better == 'lower'
else (actual >= expected))

if not check_result:
failures.append(fqtn)
print 'FAILED %s: actual %s, expected %s, better %s' % (
fqtn, actual, expected, better)
failures = json.load(f)

json.dump({
'valid': valid,
'valid': (rc == 0 or rc == 125),
'failures': failures,
}, script_args.output)

# sizes.py itself doesn't fail on regressions.
if failures and rc == 0:
rc = 1

return rc


Expand Down
11 changes: 10 additions & 1 deletion tools/linux/dump-static-initializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@
IS_GIT_WORKSPACE = (subprocess.Popen(
['git', 'rev-parse'], stderr=subprocess.PIPE).wait() == 0)


class Demangler(object):
"""A wrapper around c++filt to provide a function to demangle symbols."""

def __init__(self, toolchain):
self.cppfilt = subprocess.Popen([toolchain + 'c++filt'],
stdin=subprocess.PIPE,
Expand All @@ -50,6 +52,7 @@ def Demangle(self, sym):
self.cppfilt.stdin.write(sym + '\n')
return self.cppfilt.stdout.readline().strip()


# Matches for example: "cert_logger.pb.cc", capturing "cert_logger".
protobuf_filename_re = re.compile(r'(.*)\.pb\.cc$')
def QualifyFilenameAsProto(filename):
Expand All @@ -72,6 +75,7 @@ def QualifyFilenameAsProto(filename):
candidate = line.strip()
return candidate


# Regex matching the substring of a symbol's demangled text representation most
# likely to appear in a source file.
# Example: "v8::internal::Builtins::InitBuiltinFunctionTable()" becomes
Expand Down Expand Up @@ -99,6 +103,7 @@ def QualifyFilename(filename, symbol):
candidate = line.strip()
return candidate


# Regex matching nm output for the symbols we're interested in.
# See test_ParseNmLine for examples.
nm_re = re.compile(r'(\S+) (\S+) t (?:_ZN12)?_GLOBAL__(?:sub_)?I_(.*)')
Expand All @@ -123,6 +128,7 @@ def test_ParseNmLine():
'_GLOBAL__sub_I_extension_specifics.pb.cc')
assert parse == ('extension_specifics.pb.cc', 40607408, 36), parse


# Just always run the test; it is fast enough.
test_ParseNmLine()

Expand All @@ -136,6 +142,7 @@ def ParseNm(toolchain, binary):
if parse:
yield parse


# Regex matching objdump output for the symbols we're interested in.
# Example line:
# 12354ab: (disassembly, including <FunctionReference>)
Expand All @@ -158,13 +165,14 @@ def ExtractSymbolReferences(toolchain, binary, start, end):
if ref.startswith('.LC') or ref.startswith('_DYNAMIC'):
# Ignore these, they are uninformative.
continue
if ref.startswith('_GLOBAL__I_'):
if re.match('_GLOBAL__(?:sub_)?I_', ref):
# Probably a relative jump within this function.
continue
refs.add(ref)

return sorted(refs)


def main():
parser = optparse.OptionParser(usage='%prog [option] filename')
parser.add_option('-d', '--diffable', dest='diffable',
Expand Down Expand Up @@ -236,5 +244,6 @@ def main():

return 0


if '__main__' == __name__:
sys.exit(main())
3 changes: 0 additions & 3 deletions tools/perf_expectations/OWNERS

This file was deleted.

44 changes: 0 additions & 44 deletions tools/perf_expectations/PRESUBMIT.py

This file was deleted.

24 changes: 0 additions & 24 deletions tools/perf_expectations/README.txt

This file was deleted.

4 changes: 0 additions & 4 deletions tools/perf_expectations/chromium_perf_expectations.cfg

This file was deleted.

Loading

0 comments on commit 9d5ef42

Please sign in to comment.