Skip to content

bpo-36044: Reduce number of unit tests run for PGO build. #14702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 22, 2019
Merged

bpo-36044: Reduce number of unit tests run for PGO build. #14702

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c51bc99
Reduce number of unit tests run for PGO build.
nascheme Jul 11, 2019
ae95e9f
Add NEWS.
nascheme Jul 11, 2019
055957e
Move NEWS entry under "Build"
nascheme Jul 11, 2019
6f0edcd
Move list of PGO tests from Makefile to regrtest.
nascheme Jul 15, 2019
27be2a9
Move default list of PGO tests to own module.
nascheme Jul 16, 2019
819075a
update the news wording a bit.
gpshead Jul 17, 2019
284a6db
Set PROFILE_TASK from env or configure args.
nascheme Jul 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Lib/test/libregrtest/cmdline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,9 @@ def _create_parser():
help='only write the name of test cases that will be run'
' , don\'t execute them')
group.add_argument('-P', '--pgo', dest='pgo', action='store_true',
help='enable Profile Guided Optimization training')
help='enable Profile Guided Optimization (PGO) training')
group.add_argument('--pgo-extended', action='store_true',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what "extended" means. I suggest to rename the option to "--pgo-all-tests" and rewrite the help to explain that it runs all tests by default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think list.extend(). I.e. run more tests. I didn't want to use --pgo-all because we don't actually run all the tests. Some are excluded using the support.PGO flag. However, I don't feel strongly so if you really like --pgo-all, I will change it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, --pgo-harder? 😄

help='enable extended PGO training (slower training)')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add to the description of this that "User are encouraged to benchmark the interpreter resulting from a PROFILE_TASK with this flag to decide if it is meaningfully faster than those produced using --pgo for their use cases".

I don't care what the flag name is, --pgo-extended is fine by me. as is --pgo-very-long-training-run (a flag who's length matches the build time seems... appropriate). ;)

group.add_argument('--fail-env-changed', action='store_true',
help='if a test file alters the environment, mark '
'the test as failed')
Expand Down Expand Up @@ -344,6 +346,8 @@ def _parse_args(args, **kwargs):
parser.error("-G/--failfast needs either -v or -W")
if ns.pgo and (ns.verbose or ns.verbose2 or ns.verbose3):
parser.error("--pgo/-v don't go together!")
if ns.pgo_extended:
ns.pgo = True # pgo_extended implies pgo

if ns.nowindows:
print("Warning: the --nowindows (-n) option is deprecated. "
Expand Down
56 changes: 56 additions & 0 deletions Lib/test/libregrtest/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,55 @@
from test import support


# Set of tests run by default if --pgo is specified. The tests below were
# chosen based on the following criteria: either they exercise a commonly used
# C extension module or type, or they run some relatively typical Python code.
# Long running tests should be avoided because the PGO instrumented executable
# runs slowly.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep main.py small. Would you mind to create libregrtest/pgo.py for this list please?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

_PGO_TESTS = [
'test_array',
'test_base64',
'test_binascii',
'test_binop',
'test_bisect',
'test_bytes',
'test_cmath',
'test_codecs',
'test_collections',
'test_complex',
'test_dataclasses',
'test_datetime',
'test_decimal',
'test_difflib',
'test_embed',
'test_float',
'test_fstring',
'test_functools',
'test_generators',
'test_hashlib',
'test_heapq',
'test_int',
'test_itertools',
'test_json',
'test_long',
'test_math',
'test_memoryview',
'test_operator',
'test_ordered_dict',
'test_pickle',
'test_pprint',
'test_re',
'test_set',
'test_statistics',
'test_struct',
'test_tabnanny',
'test_time',
'test_unicode',
'test_xml_etree',
'test_xml_etree_c',
]


class Regrtest:
"""Execute a test suite.

Expand Down Expand Up @@ -214,6 +263,13 @@ def find_tests(self, tests):

removepy(self.tests)

# Add default PGO tests if no tests are specified
if self.ns.pgo and not self.ns.args:
if self.ns.pgo_extended:
pass # will run all tests not marked excluded for PGO
else:
self.ns.args = _PGO_TESTS[:] # run smaller set of tests

stdtests = STDTESTS[:]
nottests = NOTTESTS.copy()
if self.ns.exclude:
Expand Down
7 changes: 4 additions & 3 deletions Makefile.pre.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,10 @@ TCLTK_INCLUDES= @TCLTK_INCLUDES@
TCLTK_LIBS= @TCLTK_LIBS@

# The task to run while instrumented when building the profile-opt target.
# We exclude unittests with -x that take a rediculious amount of time to
# run in the instrumented training build or do not provide much value.
PROFILE_TASK=-m test.regrtest --pgo
# To speed up profile generation, we don't run the full unit test suite
# by default. The default is "-m test --pgo". To run more tests, use
# PROFILE_TASK="-m test --pgo-extended"
PROFILE_TASK= @PROFILE_TASK@

# report files for gcov / lcov coverage report
COVERAGE_INFO= $(abs_builddir)/coverage.info
Expand Down
6 changes: 6 additions & 0 deletions Misc/NEWS.d/next/Build/2019-07-11-01-28-24.bpo-36044.gIgfiJ.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Reduce the number of unit tests run for the PGO generation task. This
speeds up the task by a factor of about 15X. Running the full unit test
suite is slow. This change may result in a slightly less optimized build
since not as many code branches will be executed. If you are willing to
wait for the much slower build, the old behavior can be restored using
'./configure [..] --with-profile-task="-m test --pgo-extended"'.
20 changes: 20 additions & 0 deletions configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@ target_vendor
target_cpu
target
LLVM_AR
PROFILE_TASK
DEF_MAKE_RULE
DEF_MAKE_ALL_RULE
ABIFLAGS
Expand Down Expand Up @@ -819,6 +820,7 @@ with_pydebug
with_trace_refs
with_assertions
enable_optimizations
with_profile_task
with_lto
with_hash_algorithm
with_address_sanitizer
Expand Down Expand Up @@ -1505,6 +1507,8 @@ Optional Packages:
--with-pydebug build with Py_DEBUG defined
--with-trace-refs enable tracing references for debugging purpose
--with-assertions build with C assertions enabled
--with-profile-task=ARGS
Python args for PGO generation task
--with-lto Enable Link Time Optimization in any build. Disabled
by default.
--with-hash-algorithm=[fnv|siphash24]
Expand Down Expand Up @@ -6426,6 +6430,22 @@ else
DEF_MAKE_RULE="all"
fi

# PGO generation task

PROFILE_TASK='-m test --pgo'
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for --with-profile-task=STRING" >&5
$as_echo_n "checking for --with-profile-task=STRING... " >&6; }

# Check whether --with-profile-task was given.
if test "${with_profile_task+set}" = set; then :
withval=$with_profile_task;
PROFILE_TASK="$withval"

fi

{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $PROFILE_TASK" >&5
$as_echo "$PROFILE_TASK" >&6; }

# Make llvm-relatec checks work on systems where llvm tools are not installed with their
# normal names in the default $PATH (ie: Ubuntu). They exist under the
# non-suffixed name in their versioned llvm directory.
Expand Down
12 changes: 12 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1293,6 +1293,18 @@ else
DEF_MAKE_RULE="all"
fi

# PGO generation task
AC_SUBST(PROFILE_TASK)
PROFILE_TASK='-m test --pgo'
AC_MSG_CHECKING(for --with-profile-task=STRING)
AC_ARG_WITH(profile-task,
AS_HELP_STRING([--with-profile-task=ARGS],
[Python args for PGO generation task]),
[
PROFILE_TASK="$withval"
])
AC_MSG_RESULT($PROFILE_TASK)

# Make llvm-relatec checks work on systems where llvm tools are not installed with their
# normal names in the default $PATH (ie: Ubuntu). They exist under the
# non-suffixed name in their versioned llvm directory.
Expand Down