This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Friday 06 December 2013 03:53:34 Siddhesh Poyarekar wrote: > I reimplemented the benchmark code generation script in python since I > figured it would be nicer to read and maintain. This has the > disadvantage of an added dependency on python - I think someone (or > maybe me?) mentioned it as a problem in the past - but it could be an > optional dependency only for those who want to run benchmarks. I was > also emboldened by Will's (really old) patch to add a python script > that generates graphs for benchmark outputs. i have no problem utilizing python in any place that does not impact: tar xf glibc.tar.bz2 && cd glibc && ./configure && make && make check for larger scripts, python is certainly easier to maintain that said, this is the first python we've imported, so i think we need to nail down some general python things before we allow it. thus proposed: - use PEP8 style. that means no tabs for indentation. - use PEP257 for docstrings. here's the preferred format: """One line description. Longer details go here on multiple lines. Args: arg: description of it Returns: Describe the return value. Raises: Any random exceptions that might be raised. """ - require python-2.7, but be compatible with python-3.2+ - use "from __future__ import print_function" - use printf strings rather than concatenation: bad: print('blah' + var + 'foo') good: print('blah%sfoo' % var) - prefer strings use single quotes rather than double quotes - use a main() func - global scope code is heavily discouraged - globals are heavily discouraged - let's add a pylintrc file and require all code to pass `pylint` with it > #!/usr/bin/env python personally, i'd be fine with /usr/bin/python. are systems so unusual that they put it elsewhere that matters ? > # Copyright (C) 2013 Free Software Foundation, Inc. > # This file is part of the GNU C Library. > > # The GNU C Library is free software; you can redistribute it and/or having one continuous comment block is easier to read -- so have a # inbetween these sections rather than just a blank line > import sys > > all_vals = {} > # Valid directives. > directives = {"name": "", > "args": [], > "includes": [], > "include-sources": [], > "ret": ""} > > # Generate the C source for the function from the values and directives. > def gen_source(func): use a docstring rather than comments above the func > # Print macros. This branches out to a separate routine if > # the function takes arguments. > if not directives["args"]: > print "#define CALL_BENCH_FUNC(v, i) %s ()" % func > print "#define NUM_VARIANTS (1)" > print "#define NUM_SAMPLES(v) (1)" > print "#define VARIANT(v) FUNCNAME \"()\"" i wonder if we should pull these out into constant strings rather than inlining them all. it's easier to reason/tweak large string blocks than multiple prints. DEFINE_SPEW = """ #define CALL_BENCH_FUNC(v, i) %(func)s () #define NUM_VARIANTS (1) #define NUM_SAMPLES(v) (1) #define VARIANT(v) FUNCNAME "()" """ ... print(DEFINE_SPEW % {'func': func}) > outargs = [] > else: > outargs = _print_arg_data (func) no space before the ( > for k in all_vals.keys(): > vals = all_vals[k] > out = map (lambda v: "{%s}," % v, vals) just use a list comprehension: out = ['{%s},' % x for x in vals] > # Members for the variants structure list that we will > # print later. > variants.append("{\"%s(%s)\", %d, in%d}," % (func, k, len(vals), n)) > > print "struct args in%d[%d] = {" % (n, len(vals)) > print '\n'.join(out) > print "};\n" > n = n + 1 use zip() & itertools: for k, n in zip(all_vals.keys(), itertools.count()): > except (IndexError, KeyError): > print >> sys.stderr, "Invalid directive:", ':'.join(d) > sys.exit(1) you use this form a couple of times. suggest adding a helper like die() to write the string to stderr and call exit. > def parse_file(func): > global all_vals > try: > file_handle = open(func + "-inputs", "r") use a with directive: with open('%s-inputs' % func) as f: for line in f: line = line.rstrip() ... > for line in file_handle: > line = line.strip() guessing you meant .rstrip() here and not .strip() ? > # Look for directives and parse it if found. > if line.startswith("##"): > _parse_directive(line[2:].split(':', 1)) > > # Skip blank lines and comments. > if not line or line[0] == '#': > continue what about inline comments ? or are those not supported ? line = line.split('#', 1)[0].rstrip() if not line: # it's a comment or blank > # Otherwise, we're an input. Add to the appropriate input set. > cur_name = directives["name"] > try: > all_vals[cur_name].append(line) > except KeyError: > all_vals[cur_name] = [] > all_vals[cur_name].append(line) this would be simpler: all_vals.setdefault(cur_name, []) all_vals[cur_name].append(line) > if __name__ == '__main__': > if not sys.argv[1:]: > print "Usage: ", sys.argv[0], " <function>" > sys.exit(1) > > parse_file(sys.argv[1]) > gen_source(sys.argv[1]) this should be in a main func. def main(args): if len(args) != 1: print('Usage: %s <function>' % sys.argv[0]) parse_file(args[0]) gen_source(args[0]) if __name__ == '__main__': sys.exit(main(sys.argv[1:])) -mike
Attachment:
signature.asc
Description: This is a digitally signed message part.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |