crosperf: remove idle samples from crosperf report
Right now Crosperf reports include samples from idle functions.
Filtering these samples from the reports allow users to have more
accurate evaluation of the performance impact of code changes as they
focuson the meaningful samples.
BUG=b:194737325
TEST=unit tests passed.
Change-Id: I22bc3c78999eec9a141440e92ce52457a4bfa0a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3054437
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Tested-by: Jian Cai <jiancai@google.com>
diff --git a/crosperf/results_cache.py b/crosperf/results_cache.py
index c1cc872..26da344 100644
--- a/crosperf/results_cache.py
+++ b/crosperf/results_cache.py
@@ -273,10 +273,16 @@
return keyvals_dict
def GetSamples(self):
- samples = 0
+ actual_samples = 0
for perf_data_file in self.perf_data_files:
chroot_perf_data_file = misc.GetInsideChrootPath(self.chromeos_root,
perf_data_file)
+ perf_report_file = '%s.report' % perf_data_file
+ if os.path.exists(perf_report_file):
+ raise RuntimeError('Perf report file already exists: %s' %
+ perf_report_file)
+ chroot_perf_report_file = misc.GetInsideChrootPath(
+ self.chromeos_root, perf_report_file)
perf_path = os.path.join(self.chromeos_root, 'chroot', 'usr/bin/perf')
perf_file = '/usr/sbin/perf'
if os.path.exists(perf_path):
@@ -304,17 +310,76 @@
# Each line looks like this:
# 45.42% 237210 chrome
# And we want the second number which is the sample count.
- sample = 0
+ samples = 0
try:
for line in result.split('\n'):
attr = line.split()
if len(attr) == 3 and '%' in attr[0]:
- sample += int(attr[1])
+ samples += int(attr[1])
except:
raise RuntimeError('Cannot parse perf dso result')
- samples += sample
- return [samples, u'samples']
+ actual_samples += samples
+
+ # Remove idle cycles from the accumulated sample count.
+ debug_path = self.label.debug_path
+
+ if debug_path:
+ symfs = '--symfs ' + debug_path
+ vmlinux = '--vmlinux ' + os.path.join(debug_path, 'usr', 'lib', 'debug',
+ 'boot', 'vmlinux')
+ kallsyms = ''
+ print('** WARNING **: --kallsyms option not applied, no System.map-* '
+ 'for downloaded image.')
+ else:
+ if self.label.image_type != 'local':
+ print('** WARNING **: Using local debug info in /build, this may '
+ 'not match the downloaded image.')
+ build_path = os.path.join('/build', self.board)
+ symfs = self._CheckDebugPath('symfs', build_path)
+ vmlinux_path = os.path.join(build_path, 'usr/lib/debug/boot/vmlinux')
+ vmlinux = self._CheckDebugPath('vmlinux', vmlinux_path)
+ kallsyms_path = os.path.join(build_path, 'boot')
+ kallsyms = self._CheckDebugPath('kallsyms', kallsyms_path)
+
+ command = ('%s report -n %s %s %s -i %s --stdio --quiet > %s' %
+ (perf_file, symfs, vmlinux, kallsyms, chroot_perf_data_file,
+ chroot_perf_report_file))
+ if self.log_level != 'verbose':
+ self._logger.LogOutput('Generating perf report...\nCMD: %s' % command)
+ exit_code = self.ce.ChrootRunCommand(self.chromeos_root, command)
+ if exit_code == 0:
+ if self.log_level != 'verbose':
+ self._logger.LogOutput('Perf report generated successfully.')
+ else:
+ raise RuntimeError('Perf report not generated correctly. CMD: %s' %
+ command)
+
+ idle_functions = {
+ '[kernel.kallsyms]':
+ ('intel_idle', 'arch_cpu_idle', 'intel_idle', 'cpu_startup_entry',
+ 'default_idle', 'cpu_idle_loop', 'do_idle'),
+ }
+ idle_samples = 0
+
+ with open(chroot_perf_report_file) as f:
+ try:
+ for line in f:
+ # Each line has the following fields,
+ # pylint: disable=line-too-long
+ # Overhead Samples Command Shared Object Symbol
+ # pylint: disable=line-too-long
+ # 1.48% 60 swapper [kernel.kallsyms] [k] intel_idle
+ (_, samples, _, dso, _, function) = line.strip().split()
+ if dso in idle_functions and function in idle_functions[dso]:
+ if self.log_level != 'verbose':
+ self._logger.LogOutput('Removing %s samples from %s in %s' %
+ (samples, function, dso))
+ idle_samples += int(samples)
+ except:
+ raise RuntimeError('Cannot parse perf report')
+ actual_samples -= idle_samples
+ return [actual_samples, u'samples']
def GetResultsDir(self):
if self.suite == 'tast':
diff --git a/crosperf/results_cache_unittest.py b/crosperf/results_cache_unittest.py
index 2eeea71..9d95c5d 100755
--- a/crosperf/results_cache_unittest.py
+++ b/crosperf/results_cache_unittest.py
@@ -9,6 +9,7 @@
from __future__ import print_function
+import io
import os
import shutil
import tempfile
@@ -768,15 +769,26 @@
@mock.patch.object(misc, 'GetInsideChrootPath')
@mock.patch.object(command_executer.CommandExecuter,
'ChrootRunCommandWOutput')
- def test_get_samples(self, mock_chrootruncmd, mock_getpath):
- fake_file = '/usr/chromeos/chroot/tmp/results/fake_file'
+ @mock.patch.object(command_executer.CommandExecuter, 'ChrootRunCommand')
+ def test_get_samples(self, mock_get_idle_samples, mock_get_total_samples,
+ mock_getpath):
self.result.perf_data_files = ['/tmp/results/perf.data']
self.result.board = 'samus'
- mock_getpath.return_value = fake_file
- self.result.ce.ChrootRunCommandWOutput = mock_chrootruncmd
- mock_chrootruncmd.return_value = ['', '45.42% 237210 chrome ', '']
- samples = self.result.GetSamples()
- self.assertEqual(samples, [237210, u'samples'])
+ mock_getpath.return_value = '/usr/chromeos/chroot/tmp/results/perf.data'
+ mock_get_total_samples.return_value = [
+ '', '45.42% 237210 chrome ', ''
+ ]
+ mock_get_idle_samples.return_value = 0
+
+ # mock_open does not seem to support iteration.
+ # pylint: disable=line-too-long
+ content = """1.63% 66 dav1d-tile chrome [.] decode_coefs
+ 1.48% 60 swapper [kernel.kallsyms] [k] intel_idle
+ 1.16% 47 dav1d-tile chrome [.] decode_sb"""
+
+ with mock.patch('builtins.open', return_value=io.StringIO(content)):
+ samples = self.result.GetSamples()
+ self.assertEqual(samples, [237210 - 60, u'samples'])
def test_get_results_dir(self):
