Reworked test/bench defines to map to global variables
Motivation: - Debuggability. Accessing the current test/bench defines from inside gdb was basically impossible for some dumb macro-debug-info reason I can't figure out. In theory, GCC provides a .debug_macro section when compiled with -g3. I can see this section with objdump --dwarf=macro, but somehow gdb can't seem to find any definitions? I'm guess the #line source remapping is causing things to break somehow... Though even if macro-debugging gets fixed, which would be valuable, accessing defines in the current test/bench runner can trigger quite a bit of hidden machinery. This risks side-effects, which is never great when debugging. All of this is quite annoying because the test/bench defines is usually the most important piece of information when debugging! This replaces the previous hidden define machinery with simple global variables, which gdb can access no problem. - Also when debugging we no longer awkwardly step into the test_define function all the time! - In theory, global variables, being a simple memory access, should be quite a bit faster than the hidden define machinery. This does matter because running tests _is_ a dev bottleneck. In practice though, any performance benefit is below the noise floor, which isn't too surprising (~630s +-~20s). - Using global variables for defines simplifies the test/bench runner quite a bit. Though some of the previous complexity was due to a whole internal define caching system, which was supposed to lazily evaluate test defines to avoid evaluating defines we don't use. This all proved to be useless because the first thing we do when running each test is evaluate all defines to generate the test id (lol). So now, instead of lazily evaluating and caching defines, we just generate global variables during compilation and evaluate all defines for each test permutation immediately before running. This relies heavily on __attribute__((weak)) symbols, and lets the linker really shine. As a funny perk this also effectively interns all test/bench defines by the address of the resulting global variable. So we don't even need to do string comparisons when mapping suite-level defines to the runner-level defines. --- Perhaps the more interesting thing to note, is the change in strategy in how we actually evaluate the test defines. This ends up being a surprisingly tricky problem, due to the potential of mutual recursion between our defines. Previously, because our define machinery was lazy, we could just evaluate each define on demand. If a define required another define, it would lazily trigger another evaluation, implicitly recursing through C's stack. If cyclic, this would eventually lead to a stack overflow, but that's ok because it's a user error to let this happen. The "correct" way, at least in terms of being computationally optimal, would be to topologically sort the defines and evaluate the resulting tree from the leaves up. But I ain't got time for that, so the solution here is equal parts hacky, simple, and effective. Basically, we just evaluate the defines repeatedly until they stop changing: - Initially, mutually recursive defines may read the uninitialized values of their dependencies, and end up with some arbitrarily wrong result. But as the defines are repeatedly evaluated, assuming no cycles, the correct results should eventually bubble up the tree until all defines converge to the correct value. - This is O(n*e) vs O(n+e), but our define graph is usually quite shallow. - To prevent non-halting, we error after an arbitrary 1000 iterations. If you hit this, it's likely because there is a cycle in the define graph. This is runtime configurable via the new --define-depth flag. - To keep things consistent and reproducible, we zero initialize all defines before the first evaluation. I don't think this is strictly necessary, but it's important for the test runner to have the exact same results on every run. No one wants a "works on my machine" situation when the tests are involved. Experimentation shows we only need an evaluation depth of 2 to successfully evaluate the current set of defines: $ ./runners/test_runner --list-defines --define-depth=2 And any performance impact is negligible (~630s +-~20s).
This commit is contained in:
Christopher Haster
+44
-62
@@ -245,7 +245,7 @@ class BenchSuite:
|
||||
file=sys.stderr)
|
||||
|
||||
def __repr__(self):
|
||||
return '<TestSuite %s>' % self.name
|
||||
return '<BenchSuite %s>' % self.name
|
||||
|
||||
def __lt__(self, other):
|
||||
# sort by name
|
||||
@@ -429,13 +429,9 @@ def compile(bench_paths, **args):
|
||||
if not args.get('source'):
|
||||
# write any suite defines
|
||||
if suite.defines:
|
||||
for i, define in enumerate(sorted(suite.defines)):
|
||||
f.writeln('#ifndef %s' % define)
|
||||
f.writeln('#define %-24s '
|
||||
'BENCH_IMPLICIT_DEFINE_COUNT+%d' % (define+'_i', i))
|
||||
f.writeln('#define %-24s '
|
||||
'BENCH_DEFINE(%s)' % (define, define+'_i'))
|
||||
f.writeln('#endif')
|
||||
for define in sorted(suite.defines):
|
||||
f.writeln('__attribute__((weak)) intmax_t %s;'
|
||||
% define)
|
||||
f.writeln()
|
||||
|
||||
# write any suite code
|
||||
@@ -477,17 +473,14 @@ def compile(bench_paths, **args):
|
||||
% (' | '.join(filter(None, [
|
||||
'BENCH_INTERNAL' if suite.internal else None]))
|
||||
or 0))
|
||||
# create suite defines
|
||||
if suite.defines:
|
||||
# create suite define names
|
||||
f.writeln(4*' '+'.define_names = (const char *const['
|
||||
'BENCH_IMPLICIT_DEFINE_COUNT+%d]){'
|
||||
% (len(suite.defines)))
|
||||
f.writeln(4*' '+'.defines = (const bench_define_t[]){')
|
||||
for k in sorted(suite.defines):
|
||||
f.writeln(8*' '+'[%-24s] = "%s",' % (k+'_i', k))
|
||||
f.writeln(8*' '+'{"%s", &%s, NULL, NULL, 0},'
|
||||
% (k, k))
|
||||
f.writeln(4*' '+'},')
|
||||
f.writeln(4*' '+'.define_count = '
|
||||
'BENCH_IMPLICIT_DEFINE_COUNT+%d,'
|
||||
% len(suite.defines))
|
||||
f.writeln(4*' '+'.define_count = %d,' % len(suite.defines))
|
||||
if suite.cases:
|
||||
f.writeln(4*' '+'.cases = (const struct bench_case[]){')
|
||||
for case in suite.cases:
|
||||
@@ -499,18 +492,20 @@ def compile(bench_paths, **args):
|
||||
% (' | '.join(filter(None, [
|
||||
'BENCH_INTERNAL' if suite.internal else None]))
|
||||
or 0))
|
||||
# create case defines
|
||||
if case.defines:
|
||||
f.writeln(12*' '+'.defines = '
|
||||
'(const bench_define_t*)(const bench_define_t[]['
|
||||
'BENCH_IMPLICIT_DEFINE_COUNT+%d]){'
|
||||
f.writeln(12*' '+'.defines'
|
||||
' = (const bench_define_t*)'
|
||||
'(const bench_define_t[][%d]){'
|
||||
% (len(suite.defines)))
|
||||
for i, permutation in enumerate(case.permutations):
|
||||
f.writeln(16*' '+'{')
|
||||
for k, vs in sorted(permutation.items()):
|
||||
f.writeln(20*' '
|
||||
+'[%-24s] = {__bench__%s__%s__%d, NULL, '
|
||||
'%d},'
|
||||
% (k+'_i', case.name, k, i,
|
||||
f.writeln(20*' '+'[%d] = {'
|
||||
'"%s", &%s, '
|
||||
'__bench__%s__%s__%d, NULL, %d},'
|
||||
% (sorted(suite.defines).index(k),
|
||||
k, k, case.name, k, i,
|
||||
sum(len(v)
|
||||
if isinstance(v, range)
|
||||
else 1
|
||||
@@ -537,30 +532,25 @@ def compile(bench_paths, **args):
|
||||
shutil.copyfileobj(sf, f)
|
||||
f.writeln()
|
||||
|
||||
# merge all defines we need, otherwise we will run into
|
||||
# redefinition errors
|
||||
defines = ({define
|
||||
for suite in suites
|
||||
if suite.isin(args['source'])
|
||||
for define in suite.defines}
|
||||
| {define
|
||||
for suite in suites
|
||||
for case in suite.cases
|
||||
if case.isin(args['source'])
|
||||
for define in case.defines})
|
||||
if defines:
|
||||
for define in sorted(defines):
|
||||
f.writeln('__attribute__((weak)) intmax_t %s;'
|
||||
% define)
|
||||
f.writeln()
|
||||
|
||||
# write any internal benches
|
||||
for suite in suites:
|
||||
if (suite.isin(args['source'])
|
||||
or any(case.isin(args['source'])
|
||||
for case in suite.cases)):
|
||||
# write defines, but note we need to undef any
|
||||
# new defines since we're in someone else's file
|
||||
if suite.defines:
|
||||
for i, define in enumerate(
|
||||
sorted(suite.defines)):
|
||||
f.writeln('#ifndef %s' % define)
|
||||
f.writeln('#define %-24s '
|
||||
'BENCH_IMPLICIT_DEFINE_COUNT+%d' % (
|
||||
define+'_i', i))
|
||||
f.writeln('#define %-24s '
|
||||
'BENCH_DEFINE(%s)' % (
|
||||
define, define+'_i'))
|
||||
f.writeln('#define '
|
||||
'__BENCH__%s__NEEDS_UNDEF' % (
|
||||
define))
|
||||
f.writeln('#endif')
|
||||
f.writeln()
|
||||
|
||||
# write any internal suite code
|
||||
if suite.isin(args['source']):
|
||||
if suite.code_lineno is not None:
|
||||
f.writeln('#line %d "%s"'
|
||||
@@ -575,19 +565,6 @@ def compile(bench_paths, **args):
|
||||
if case.isin(args['source']):
|
||||
write_case_functions(f, suite, case)
|
||||
|
||||
if (suite.isin(args['source'])
|
||||
or any(case.isin(args['source'])
|
||||
for case in suite.cases)):
|
||||
for define in sorted(suite.defines):
|
||||
f.writeln('#ifdef __BENCH__%s__NEEDS_UNDEF'
|
||||
% define)
|
||||
f.writeln('#undef __BENCH__%s__NEEDS_UNDEF'
|
||||
% define)
|
||||
f.writeln('#undef %s' % define)
|
||||
f.writeln('#undef %s' % (define+'_i'))
|
||||
f.writeln('#endif')
|
||||
f.writeln()
|
||||
|
||||
# declare our bench suites
|
||||
#
|
||||
# by declaring these as weak we can write these to every
|
||||
@@ -640,6 +617,8 @@ def find_runner(runner, id=None, **args):
|
||||
'-o%s' % args['perf']]))
|
||||
|
||||
# other context
|
||||
if args.get('define_depth'):
|
||||
cmd.append('--define-depth=%s' % args['define_depth'])
|
||||
if args.get('disk'):
|
||||
cmd.append('-d%s' % args['disk'])
|
||||
if args.get('trace'):
|
||||
@@ -662,11 +641,11 @@ def find_runner(runner, id=None, **args):
|
||||
for define in args.get('define'):
|
||||
cmd.append('-D%s' % define)
|
||||
|
||||
# test id?
|
||||
# bench id?
|
||||
#
|
||||
# note we disable defines above when id is explicit, defines override id
|
||||
# in the test runner, which is not what we want when querying an explicit
|
||||
# test id
|
||||
# in the bench runner, which is not what we want when querying an explicit
|
||||
# bench id
|
||||
if id is not None:
|
||||
cmd.append(id)
|
||||
|
||||
@@ -1511,6 +1490,9 @@ if __name__ == "__main__":
|
||||
'-D', '--define',
|
||||
action='append',
|
||||
help="Override a bench define.")
|
||||
bench_parser.add_argument(
|
||||
'--define-depth',
|
||||
help="How deep to evaluate recursive defines before erroring.")
|
||||
bench_parser.add_argument(
|
||||
'-d', '--disk',
|
||||
help="Direct block device operations to this file.")
|
||||
@@ -1568,7 +1550,7 @@ if __name__ == "__main__":
|
||||
'-F', '--failures',
|
||||
type=lambda x: int(x, 0),
|
||||
default=3,
|
||||
help="Show this many test failures. Defaults to 3.")
|
||||
help="Show this many bench failures. Defaults to 3.")
|
||||
bench_parser.add_argument(
|
||||
'-C', '--context',
|
||||
type=lambda x: int(x, 0),
|
||||
|
||||
+39
-57
@@ -434,13 +434,9 @@ def compile(test_paths, **args):
|
||||
if not args.get('source'):
|
||||
# write any suite defines
|
||||
if suite.defines:
|
||||
for i, define in enumerate(sorted(suite.defines)):
|
||||
f.writeln('#ifndef %s' % define)
|
||||
f.writeln('#define %-24s '
|
||||
'TEST_IMPLICIT_DEFINE_COUNT+%d' % (define+'_i', i))
|
||||
f.writeln('#define %-24s '
|
||||
'TEST_DEFINE(%s)' % (define, define+'_i'))
|
||||
f.writeln('#endif')
|
||||
for define in sorted(suite.defines):
|
||||
f.writeln('__attribute__((weak)) intmax_t %s;'
|
||||
% define)
|
||||
f.writeln()
|
||||
|
||||
# write any suite code
|
||||
@@ -483,17 +479,14 @@ def compile(test_paths, **args):
|
||||
'TEST_INTERNAL' if suite.internal else None,
|
||||
'TEST_REENTRANT' if suite.reentrant else None]))
|
||||
or 0))
|
||||
# create suite defines
|
||||
if suite.defines:
|
||||
# create suite define names
|
||||
f.writeln(4*' '+'.define_names = (const char *const['
|
||||
'TEST_IMPLICIT_DEFINE_COUNT+%d]){'
|
||||
% (len(suite.defines)))
|
||||
f.writeln(4*' '+'.defines = (const test_define_t[]){')
|
||||
for k in sorted(suite.defines):
|
||||
f.writeln(8*' '+'[%-24s] = "%s",' % (k+'_i', k))
|
||||
f.writeln(8*' '+'{"%s", &%s, NULL, NULL, 0},'
|
||||
% (k, k))
|
||||
f.writeln(4*' '+'},')
|
||||
f.writeln(4*' '+'.define_count = '
|
||||
'TEST_IMPLICIT_DEFINE_COUNT+%d,'
|
||||
% len(suite.defines))
|
||||
f.writeln(4*' '+'.define_count = %d,' % len(suite.defines))
|
||||
if suite.cases:
|
||||
f.writeln(4*' '+'.cases = (const struct test_case[]){')
|
||||
for case in suite.cases:
|
||||
@@ -506,18 +499,20 @@ def compile(test_paths, **args):
|
||||
'TEST_INTERNAL' if case.internal else None,
|
||||
'TEST_REENTRANT' if case.reentrant else None]))
|
||||
or 0))
|
||||
# create case defines
|
||||
if case.defines:
|
||||
f.writeln(12*' '+'.defines = '
|
||||
'(const test_define_t*)(const test_define_t[]['
|
||||
'TEST_IMPLICIT_DEFINE_COUNT+%d]){'
|
||||
f.writeln(12*' '+'.defines'
|
||||
' = (const test_define_t*)'
|
||||
'(const test_define_t[][%d]){'
|
||||
% (len(suite.defines)))
|
||||
for i, permutation in enumerate(case.permutations):
|
||||
f.writeln(16*' '+'{')
|
||||
for k, vs in sorted(permutation.items()):
|
||||
f.writeln(20*' '
|
||||
+'[%-24s] = {__test__%s__%s__%d, NULL, '
|
||||
'%d},'
|
||||
% (k+'_i', case.name, k, i,
|
||||
f.writeln(20*' '+'[%d] = {'
|
||||
'"%s", &%s, '
|
||||
'__test__%s__%s__%d, NULL, %d},'
|
||||
% (sorted(suite.defines).index(k),
|
||||
k, k, case.name, k, i,
|
||||
sum(len(v)
|
||||
if isinstance(v, range)
|
||||
else 1
|
||||
@@ -544,30 +539,25 @@ def compile(test_paths, **args):
|
||||
shutil.copyfileobj(sf, f)
|
||||
f.writeln()
|
||||
|
||||
# merge all defines we need, otherwise we will run into
|
||||
# redefinition errors
|
||||
defines = ({define
|
||||
for suite in suites
|
||||
if suite.isin(args['source'])
|
||||
for define in suite.defines}
|
||||
| {define
|
||||
for suite in suites
|
||||
for case in suite.cases
|
||||
if case.isin(args['source'])
|
||||
for define in case.defines})
|
||||
if defines:
|
||||
for define in sorted(defines):
|
||||
f.writeln('__attribute__((weak)) intmax_t %s;'
|
||||
% define)
|
||||
f.writeln()
|
||||
|
||||
# write any internal tests
|
||||
for suite in suites:
|
||||
if (suite.isin(args['source'])
|
||||
or any(case.isin(args['source'])
|
||||
for case in suite.cases)):
|
||||
# write defines, but note we need to undef any
|
||||
# new defines since we're in someone else's file
|
||||
if suite.defines:
|
||||
for i, define in enumerate(
|
||||
sorted(suite.defines)):
|
||||
f.writeln('#ifndef %s' % define)
|
||||
f.writeln('#define %-24s '
|
||||
'TEST_IMPLICIT_DEFINE_COUNT+%d' % (
|
||||
define+'_i', i))
|
||||
f.writeln('#define %-24s '
|
||||
'TEST_DEFINE(%s)' % (
|
||||
define, define+'_i'))
|
||||
f.writeln('#define '
|
||||
'__TEST__%s__NEEDS_UNDEF' % (
|
||||
define))
|
||||
f.writeln('#endif')
|
||||
f.writeln()
|
||||
|
||||
# write any internal suite code
|
||||
if suite.isin(args['source']):
|
||||
if suite.code_lineno is not None:
|
||||
f.writeln('#line %d "%s"'
|
||||
@@ -582,19 +572,6 @@ def compile(test_paths, **args):
|
||||
if case.isin(args['source']):
|
||||
write_case_functions(f, suite, case)
|
||||
|
||||
if (suite.isin(args['source'])
|
||||
or any(case.isin(args['source'])
|
||||
for case in suite.cases)):
|
||||
for define in sorted(suite.defines):
|
||||
f.writeln('#ifdef __TEST__%s__NEEDS_UNDEF'
|
||||
% define)
|
||||
f.writeln('#undef __TEST__%s__NEEDS_UNDEF'
|
||||
% define)
|
||||
f.writeln('#undef %s' % define)
|
||||
f.writeln('#undef %s' % (define+'_i'))
|
||||
f.writeln('#endif')
|
||||
f.writeln()
|
||||
|
||||
# declare our test suites
|
||||
#
|
||||
# by declaring these as weak we can write these to every
|
||||
@@ -647,6 +624,8 @@ def find_runner(runner, id=None, **args):
|
||||
'-o%s' % args['perf']]))
|
||||
|
||||
# other context
|
||||
if args.get('define_depth'):
|
||||
cmd.append('--define-depth=%s' % args['define_depth'])
|
||||
if args.get('powerloss'):
|
||||
cmd.append('-P%s' % args['powerloss'])
|
||||
if args.get('disk'):
|
||||
@@ -1525,6 +1504,9 @@ if __name__ == "__main__":
|
||||
'-D', '--define',
|
||||
action='append',
|
||||
help="Override a test define.")
|
||||
test_parser.add_argument(
|
||||
'--define-depth',
|
||||
help="How deep to evaluate recursive defines before erroring.")
|
||||
test_parser.add_argument(
|
||||
'-P', '--powerloss',
|
||||
help="Comma-separated list of power-loss scenarios to test.")
|
||||
|
||||
Reference in New Issue
Block a user