twister: footprint: Improve command-line arguments

Improve Twister command line arguments for memory footprint:

 * group and order footprint arguments meaningfully,
 * clearer help descriptions,
 * resolve logical inconsistences for combinations of arguments,
   in particular:
   `--last-metrics` now forces `--enable-size-report`,
   `--show-footprint` now controls only detailed log output of
       footprint deltas in comparison modes.
 * align twister tests accordingly.

Signed-off-by: Dmitrii Golovanov <dmitrii.golovanov@intel.com>
This commit is contained in:
Dmitrii Golovanov 2024-04-01 22:41:13 +02:00 committed by Carles Cufí
parent 6cda8b8312
commit 2ef6b98b98
4 changed files with 112 additions and 78 deletions

View file

@ -68,8 +68,6 @@ Artificially long but functional example:
__/fifo_api/testcase.yaml
""")
compare_group_option = parser.add_mutually_exclusive_group()
platform_group_option = parser.add_mutually_exclusive_group()
run_group_option = parser.add_mutually_exclusive_group()
@ -84,6 +82,10 @@ Artificially long but functional example:
valgrind_asan_group = parser.add_mutually_exclusive_group()
footprint_group = parser.add_argument_group(
title="Memory footprint",
description="Collect and report ROM/RAM size footprint for the test instance images built.")
case_select.add_argument(
"-E",
"--save-tests",
@ -124,14 +126,6 @@ Artificially long but functional example:
case_select.add_argument("--test-tree", action="store_true",
help="""Output the test plan in a tree form""")
compare_group_option.add_argument("--compare-report",
help="Use this report file for size comparison")
compare_group_option.add_argument(
"-m", "--last-metrics", action="store_true",
help="Compare with the results of the previous twister "
"invocation")
platform_group_option.add_argument(
"-G",
"--integration",
@ -332,20 +326,10 @@ structure in the main Zephyr tree: boards/<arch>/<board_name>/""")
help="Test level to be used. By default, no levels are used for filtering"
"and do the selection based on existing filters.")
parser.add_argument(
"-D", "--all-deltas", action="store_true",
help="Show all footprint deltas, positive or negative. Implies "
"--footprint-threshold=0")
parser.add_argument(
"--device-serial-baud", action="store", default=None,
help="Serial device baud rate (default 115200)")
parser.add_argument(
"--disable-unrecognized-section-test", action="store_true",
default=False,
help="Skip the 'unrecognized section' test.")
parser.add_argument(
"--disable-suite-name-check", action="store_true", default=False,
help="Disable extended test suite name verification at the beginning "
@ -375,14 +359,6 @@ structure in the main Zephyr tree: boards/<arch>/<board_name>/""")
binaries such as those generated for the native_sim configuration.
""")
parser.add_argument("--enable-size-report", action="store_true",
help="Enable expensive computation of RAM/ROM segment sizes.")
parser.add_argument("--create-rom-ram-report", action="store_true",
help="Generate detailed ram/rom json reports for "
"each build, via cmake build calls with the "
"`--target footprint` argument")
parser.add_argument(
"--filter", choices=['buildable', 'runnable'],
default='buildable',
@ -403,12 +379,73 @@ structure in the main Zephyr tree: boards/<arch>/<board_name>/""")
help="Path to the gcov tool to use for code coverage "
"reports")
footprint_group.add_argument(
"--create-rom-ram-report",
action="store_true",
help="Generate detailed json reports with ROM/RAM symbol sizes for each test image built "
"using additional build option `--target footprint`.")
footprint_group.add_argument(
"--enable-size-report",
action="store_true",
help="Collect and report ROM/RAM section sizes for each test image built.")
parser.add_argument(
"-H", "--footprint-threshold", type=float, default=5,
help="When checking test case footprint sizes, warn the user if "
"the new app size is greater then the specified percentage "
"from the last release. Default is 5. 0 to warn on any "
"increase on app size.")
"--disable-unrecognized-section-test",
action="store_true",
default=False,
help="Don't error on unrecognized sections in the binary images.")
footprint_group.add_argument(
"--footprint-from-buildlog",
action = "store_true",
help="Take ROM/RAM sections footprint summary values from the 'build.log' "
"instead of 'objdump' results used otherwise."
"Requires --enable-size-report or one of the baseline comparison modes.")
compare_group_option = footprint_group.add_mutually_exclusive_group()
compare_group_option.add_argument(
"-m", "--last-metrics",
action="store_true",
help="Compare footprints to the previous twister invocation as a baseline "
"running in the same output directory. "
"Implies --enable-size-report option.")
compare_group_option.add_argument(
"--compare-report",
help="Use this report file as a baseline for footprint comparison. "
"The file should be of 'twister.json' schema. "
"Implies --enable-size-report option.")
footprint_group.add_argument(
"--show-footprint",
action="store_true",
help="With footprint comparison to a baseline, log ROM/RAM section deltas. ")
footprint_group.add_argument(
"-H", "--footprint-threshold",
type=float,
default=5.0,
help="With footprint comparison to a baseline, "
"warn the user for any of the footprint metric change which is greater or equal "
"to the specified percentage value. "
"Default is %(default)s for %(default)s%% delta from the new footprint value. "
"Use zero to warn on any footprint metric increase.")
footprint_group.add_argument(
"-D", "--all-deltas",
action="store_true",
help="With footprint comparison to a baseline, "
"warn on any footprint change, increase or decrease. "
"Implies --footprint-threshold=0")
footprint_group.add_argument(
"-z", "--size",
action="append",
metavar='FILENAME',
help="Ignore all other command line options and just produce a report to "
"stdout with ROM/RAM section sizes on the specified binary images.")
parser.add_argument(
"-i", "--inline-logs", action="store_true",
@ -608,13 +645,6 @@ structure in the main Zephyr tree: boards/<arch>/<board_name>/""")
"example on Windows OS. This option can be used only with "
"'--ninja' argument (to use Ninja build generator).")
parser.add_argument(
"--show-footprint",
action="store_true",
required = "--footprint-from-buildlog" in sys.argv,
help="Show footprint statistics and deltas since last release."
)
parser.add_argument(
"-t", "--tag", action="append",
help="Specify tags to restrict which tests to run by tag value. "
@ -695,18 +725,6 @@ structure in the main Zephyr tree: boards/<arch>/<board_name>/""")
directory (testplan.json).
""")
parser.add_argument(
"-z", "--size", action="append",
help="Don't run twister. Instead, produce a report to "
"stdout detailing RAM/ROM sizes on the specified filenames. "
"All other command line arguments ignored.")
parser.add_argument(
"--footprint-from-buildlog",
action = "store_true",
help="Get information about memory footprint from generated build.log. "
"Requires using --show-footprint option.")
parser.add_argument("extra_test_args", nargs=argparse.REMAINDER,
help="Additional args following a '--' are passed to the test binary")
@ -755,7 +773,7 @@ def parse_arguments(parser, args, options = None):
options.testsuite_root = [os.path.join(ZEPHYR_BASE, "tests"),
os.path.join(ZEPHYR_BASE, "samples")]
if options.show_footprint or options.compare_report:
if options.last_metrics or options.compare_report:
options.enable_size_report = True
if options.aggressive_no_clean:
@ -811,6 +829,10 @@ def parse_arguments(parser, args, options = None):
sc.size_report()
sys.exit(0)
if options.footprint_from_buildlog and not options.enable_size_report:
logger.error("--footprint-from-buildlog requires --enable-size-report")
sys.exit(1)
if len(options.extra_test_args) > 0:
# extra_test_args is a list of CLI args that Twister did not recognize
# and are intended to be passed through to the ztest executable. This

View file

@ -404,7 +404,7 @@ class Reporting:
logger.debug("running footprint_reports")
deltas = self.compare_metrics(report)
warnings = 0
if deltas and show_footprint:
if deltas:
for i, metric, value, delta, lower_better in deltas:
if not all_deltas and ((delta < 0 and lower_better) or
(delta > 0 and not lower_better)):
@ -417,15 +417,19 @@ class Reporting:
if not all_deltas and (percentage < (footprint_threshold / 100.0)):
continue
logger.info("{:<25} {:<60} {}{}{}: {} {:<+4}, is now {:6} {:+.2%}".format(
i.platform.name, i.testsuite.name, Fore.YELLOW,
"INFO" if all_deltas else "WARNING", Fore.RESET,
metric, delta, value, percentage))
if show_footprint:
logger.log(
logging.INFO if all_deltas else logging.WARNING,
"{:<25} {:<60} {} {:<+4}, is now {:6} {:+.2%}".format(
i.platform.name, i.testsuite.name,
metric, delta, value, percentage))
warnings += 1
if warnings:
logger.warning("Deltas based on metrics from last %s" %
("release" if not last_metrics else "run"))
logger.warning("Found {} footprint deltas to {} as a baseline.".format(
warnings,
(report if not last_metrics else "the last twister run.")))
def synopsis(self):
cnt = 0
@ -456,13 +460,13 @@ class Reporting:
f"{example_instance.testsuite.source_dir_rel} -T {example_instance.testsuite.id}")
logger.info("-+" * 40)
def summary(self, results, unrecognized_sections, duration):
def summary(self, results, ignore_unrecognized_sections, duration):
failed = 0
run = 0
for instance in self.instances.values():
if instance.status == "failed":
failed += 1
elif instance.metrics.get("unrecognized") and not unrecognized_sections:
elif not ignore_unrecognized_sections and instance.metrics.get("unrecognized"):
logger.error("%sFAILED%s: %s has unrecognized binary sections: %s" %
(Fore.RED, Fore.RESET, instance.name,
str(instance.metrics.get("unrecognized", []))))

View file

@ -225,7 +225,7 @@ def test_parse_arguments_warnings(caplog):
TESTDATA_2 = [
(['--show-footprint']),
(['--enable-size-report']),
(['--compare-report', 'dummy']),
]

View file

@ -12,6 +12,7 @@ import mock
import os
import pytest
import sys
import re
from conftest import ZEPHYR_BASE, TEST_DATA, testsuite_filename_mock, clear_log_in_test
from twisterlib.testplan import TestPlan
@ -24,11 +25,14 @@ class TestFootprint:
# These warnings notify us that deltas were shown in log.
# Coupled with the code under test.
DELTA_WARNING_RELEASE = 'Deltas based on metrics from last release'
DELTA_WARNING_RUN = 'Deltas based on metrics from last run'
DELTA_WARNING_COMPARE = re.compile(
r'Found [1-9]+[0-9]* footprint deltas to .*blackbox-out\.[0-9]+/twister.json as a baseline'
)
DELTA_WARNING_RUN = re.compile(r'Found [1-9]+[0-9]* footprint deltas to the last twister run')
# Size report key we modify to control for deltas
RAM_KEY = 'used_ram'
DELTA_DETAIL = re.compile(RAM_KEY + r' \+[0-9]+, is now +[0-9]+ \+[0-9.]+%')
@classmethod
def setup_class(cls):
@ -103,11 +107,11 @@ class TestFootprint:
if expect_delta_log:
assert self.RAM_KEY in caplog.text
assert self.DELTA_WARNING_RELEASE in caplog.text, \
assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \
'Expected footprint deltas not logged.'
else:
assert self.RAM_KEY not in caplog.text
assert self.DELTA_WARNING_RELEASE not in caplog.text, \
assert not re.search(self.DELTA_WARNING_COMPARE, caplog.text), \
'Unexpected footprint deltas logged.'
def test_footprint_from_buildlog(self, out_path):
@ -116,7 +120,7 @@ class TestFootprint:
path = os.path.join(TEST_DATA, 'tests', 'dummy', 'device', 'group')
args = ['-i', '--outdir', out_path, '-T', path] + \
[] + \
['--show-footprint'] + \
['--enable-size-report'] + \
[val for pair in zip(
['-p'] * len(test_platforms), test_platforms
) for val in pair]
@ -141,7 +145,7 @@ class TestFootprint:
path = os.path.join(TEST_DATA, 'tests', 'dummy', 'device', 'group')
args = ['-i', '--outdir', out_path, '-T', path] + \
['--footprint-from-buildlog'] + \
['--show-footprint'] + \
['--enable-size-report'] + \
[val for pair in zip(
['-p'] * len(test_platforms), test_platforms
) for val in pair]
@ -229,11 +233,11 @@ class TestFootprint:
if expect_delta_log:
assert self.RAM_KEY in caplog.text
assert self.DELTA_WARNING_RELEASE in caplog.text, \
assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \
'Expected footprint deltas not logged.'
else:
assert self.RAM_KEY not in caplog.text
assert self.DELTA_WARNING_RELEASE not in caplog.text, \
assert not re.search(self.DELTA_WARNING_COMPARE, caplog.text), \
'Unexpected footprint deltas logged.'
@pytest.mark.parametrize(
@ -298,12 +302,16 @@ class TestFootprint:
if expect_delta_log:
assert self.RAM_KEY in caplog.text
assert self.DELTA_WARNING_RELEASE in caplog.text, \
assert re.search(self.DELTA_DETAIL, caplog.text), \
'Expected footprint delta not logged.'
assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \
'Expected footprint deltas not logged.'
else:
assert self.RAM_KEY not in caplog.text
assert self.DELTA_WARNING_RELEASE not in caplog.text, \
'Unexpected footprint deltas logged.'
assert not re.search(self.DELTA_DETAIL, caplog.text), \
'Expected footprint delta not logged.'
assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \
'Expected footprint deltas logged.'
@pytest.mark.parametrize(
'old_ram_multiplier, expect_delta_log',
@ -367,11 +375,11 @@ class TestFootprint:
if expect_delta_log:
assert self.RAM_KEY in caplog.text
assert self.DELTA_WARNING_RUN in caplog.text, \
assert re.search(self.DELTA_WARNING_RUN, caplog.text), \
'Expected footprint deltas not logged.'
else:
assert self.RAM_KEY not in caplog.text
assert self.DELTA_WARNING_RUN not in caplog.text, \
assert not re.search(self.DELTA_WARNING_RUN, caplog.text), \
'Unexpected footprint deltas logged.'
second_logs = caplog.records
@ -464,9 +472,9 @@ class TestFootprint:
if expect_delta_log:
assert self.RAM_KEY in caplog.text
assert self.DELTA_WARNING_RELEASE in caplog.text, \
assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \
'Expected footprint deltas not logged.'
else:
assert self.RAM_KEY not in caplog.text
assert self.DELTA_WARNING_RELEASE not in caplog.text, \
assert not re.search(self.DELTA_WARNING_COMPARE, caplog.text), \
'Unexpected footprint deltas logged.'