Other than my comment in the previous patch about (type.columns or 1) and (type.rows or 1), consider this:
Reviewed-by: Andres Gomez <[email protected]> On Fri, 2016-06-10 at 16:42 -0700, Dylan Baker wrote: > This removes a significant amount of string parsing from both the > module > and the templates, and simplifies some statements. The result is that > the generator is roughly 50% faster. > > This string parsing comes from the rows and cols functions, which are > present in both the generator and the templates. Each of these > functions > gets called roughly 40 million times. 40 million. The cols function > in > particular accounts for nearly 20 seconds of runtime (and the rows > about > 5). The replacements don't even show up in the profile. > > When you consider that the runtime is about 70 seconds before this > patch, and about 35 after, that's another big win, probably enough > that > it's not worth optimizing it further. I can't come up with any big > enhancements, maybe a few little ones that might amount to a couple > of > seconds. > > The remaining significant time sinks are the collections.deque (which > is > called from within mako and is part of the standard library, we can't > optimize it) and the regular.shader_test.mako. While I can spot some > micro optimizations that might get us a few tenths of a second here > and > there, I don't see anything that's going to win another 10 or 15 > seconds. > > Signed-off-by: Dylan Baker <[email protected]> > --- > generated_tests/gen_vs_in_fp64.py | 75 ++++++++-- > ------------ > .../gen_vs_in_fp64/columns.shader_test.mako | 40 ++++-------- > .../gen_vs_in_fp64/regular.shader_test.mako | 40 ++++-------- > 3 files changed, 48 insertions(+), 107 deletions(-) > > diff --git a/generated_tests/gen_vs_in_fp64.py > b/generated_tests/gen_vs_in_fp64.py > index fcc87f5..88c4952 100644 > --- a/generated_tests/gen_vs_in_fp64.py > +++ b/generated_tests/gen_vs_in_fp64.py > @@ -34,6 +34,7 @@ from six.moves import range > > from templates import template_dir > from modules import utils > +from modules import types as glsltypes > > TEMPLATES = > template_dir(os.path.basename(os.path.splitext(__file__)[0])) > > @@ -71,29 +72,29 @@ DOUBLE_NORMAL_VALUES = ['0xffefffffffffffff', # > Negative maximum normalized > '0x4b1e35ed24eb6496', # +7.23401345e+53 > '0x7fefffffffffffff'] # Positive maximum > normalized > > -DSCALAR_TYPES = ['double'] > +DSCALAR_TYPES = [glsltypes.DOUBLE] > > -DVEC_TYPES = ['dvec2', 'dvec3', 'dvec4'] > +DVEC_TYPES = [glsltypes.DVEC2, glsltypes.DVEC3, > glsltypes.DVEC4] > > -DMAT_TYPES = ['dmat2', 'dmat2x3', 'dmat2x4', > - 'dmat3x2', 'dmat3', 'dmat3x4', > - 'dmat4x2', 'dmat4x3', 'dmat4'] > +DMAT_TYPES = [glsltypes.DMAT2, glsltypes.DMAT2X3, > glsltypes.DMAT2X4, > + glsltypes.DMAT3X2, glsltypes.DMAT3, > glsltypes.DMAT3X4, > + glsltypes.DMAT4X2, glsltypes.DMAT4X3, > glsltypes.DMAT4] > > -FSCALAR_TYPES = ['float'] > +FSCALAR_TYPES = [glsltypes.FLOAT] > > -FVEC_TYPES = ['vec2', 'vec3', 'vec4'] > +FVEC_TYPES = [glsltypes.VEC2, glsltypes.VEC3, > glsltypes.VEC4] > > -FMAT_TYPES = ['mat2', 'mat2x3', 'mat2x4', > - 'mat3x2', 'mat3', 'mat3x4', > - 'mat4x2', 'mat4x3', 'mat4'] > +FMAT_TYPES = [glsltypes.MAT2, glsltypes.MAT2X3, > glsltypes.MAT2X4, > + glsltypes.MAT3X2, glsltypes.MAT3, > glsltypes.MAT3X4, > + glsltypes.MAT4X2, glsltypes.MAT4X3, > glsltypes.MAT4] > > -ISCALAR_TYPES = ['int'] > +ISCALAR_TYPES = [glsltypes.INT] > > -IVEC_TYPES = ['ivec2', 'ivec3', 'ivec4'] > +IVEC_TYPES = [glsltypes.IVEC2, glsltypes.IVEC3, > glsltypes.IVEC4] > > -USCALAR_TYPES = ['uint'] > +USCALAR_TYPES = [glsltypes.UINT] > > -UVEC_TYPES = ['uvec2', 'uvec3', 'uvec4'] > +UVEC_TYPES = [glsltypes.UVEC2, glsltypes.UVEC3, > glsltypes.UVEC4] > > HEX_VALUES_32BIT = ['0xc21620c5', # -3.7532 float, > -1038737211 int, 3256230085 uint > '0x75bc289b', > # 4.7703e32 float, 1975265435 int, 1975265435 uint > @@ -121,26 +122,6 @@ class TestTuple(object): > """A float64 derived and other type derived tuple to generate > the > needed conversion tests. > """ > - > - @staticmethod > - def rows(in_type): > - """Calculates the amounts of rows in a basic GLSL type.""" > - if 'vec' in in_type or 'mat' in in_type: > - return int(in_type[-1:]) > - else: > - return 1 > - > - @staticmethod > - def cols(in_type): > - """Calculates the amounts of columns in a basic GLSL > type.""" > - if 'mat' in in_type: > - if 'x' in in_type: > - return int(in_type[-3:][:1]) > - else: > - return int(in_type[-1:]) > - else: > - return 1 > - > @staticmethod > def get_dir_name(ver): > """Returns the directory name to save tests given a GLSL > version.""" > @@ -191,20 +172,17 @@ class RegularTestTuple(TestTuple): > for ver in glsl_vers: > utils.safe_makedirs(TestTuple.get_dir_name(ver)) > > - for in_types, position_order, arrays, ver in > itertools.product(in_types_array, > - > position_orders, > - > arrays_array, > - > glsl_vers): > + for in_types, position_order, arrays, ver in > itertools.product( > + in_types_array, > + position_orders, > + arrays_array, > + glsl_vers): > num_vs_in = 1 # We use an additional vec3 piglit_vertex > input > for idx, in_type in enumerate(in_types): > - if ((in_type.startswith('dvec') or > in_type.startswith('dmat')) > - and (in_type.endswith('3') or > in_type.endswith('4'))): > - multiplier = 2 > - else: > - multiplier = 1 > - num_vs_in += TestTuple.cols(in_type) * arrays[idx] * > multiplier > + num_vs_in += (in_type.columns or 1) * arrays[idx] * > \ > + (2 if in_type.type.name == 'double' and > in_type.rows in [3, 4] else 1) > # dvec* and dmat* didn't appear in GLSL until 4.20 > - if (in_type.startswith('dvec') or > in_type.startswith('dmat')) and ver == '410': > + if (in_type.type.name == 'double' and not > in_type.scalar) and ver == '410': > ver = '420' > # Skip the test if it needs too many inputs > if num_vs_in > MAX_VERTEX_ATTRIBS: > @@ -277,13 +255,12 @@ class RegularTestTuple(TestTuple): > > def generate(self): > """Generate GLSL parser tests.""" > - > filename = os.path.join(TestTuple.get_dir_name(self._ver), > 'vs-input') > for idx, in_type in enumerate(self._in_types): > if idx == self._position_order - 1: > filename += '-position' > filename += '-{}{}'.format( > - in_type, '-array{}'.format( > + in_type.name, '-array{}'.format( > self._arrays[idx]) if self._arrays[idx] - 1 else > '') > if self._position_order > len(self._in_types): > filename += '-position' > @@ -320,7 +297,7 @@ class ColumnsTestTuple(TestTuple): > utils.safe_makedirs(TestTuple.get_dir_name(ver)) > > for mat in DMAT_TYPES: > - for columns in itertools.product(range(2), > repeat=TestTuple.cols(mat)): > + for columns in itertools.product(range(2), > repeat=mat.columns): > if (0 not in columns) or (1 not in columns): > continue > for ver in glsl_vers: > @@ -337,7 +314,7 @@ class ColumnsTestTuple(TestTuple): > """Generate GLSL parser tests.""" > > filename = os.path.join(TestTuple.get_dir_name(self._ver), > - 'vs-input-columns- > {}'.format(self._mat)) > + 'vs-input-columns- > {}'.format(self._mat.name)) > for idx, column in enumerate(self._columns): > if column == 1: > filename += '-{}'.format(idx) > diff --git > a/generated_tests/templates/gen_vs_in_fp64/columns.shader_test.mako > b/generated_tests/templates/gen_vs_in_fp64/columns.shader_test.mako > index 0da2678..241c02b 100644 > --- > a/generated_tests/templates/gen_vs_in_fp64/columns.shader_test.mako > +++ > b/generated_tests/templates/gen_vs_in_fp64/columns.shader_test.mako > @@ -31,24 +31,6 @@ > glsl_version = '{}.{}'.format(glsl_version_int[0], > glsl_version_int[1:3]) > > return (glsl_version, glsl_version_int) > - > - > - def cols(in_type): > - if 'mat' in in_type: > - if 'x' in in_type: > - return int(in_type[-3:][:1]) > - else: > - return int(in_type[-1:]) > - else: > - return 1 > - > - > - def rows(in_type): > - if 'vec' in in_type or 'mat' in in_type: > - return int(in_type[-1:]) > - else: > - return 1 > - > %> > <% glsl, glsl_int = _version(ver) %> > > @@ -66,9 +48,9 @@ GLSL >= ${glsl} > #extension GL_ARB_vertex_attrib_64bit : require > % endif > > -uniform ${mat} expected; > +uniform ${mat.name} expected; > > -in ${mat} value; > +in ${mat.name} value; > in vec3 piglit_vertex; > out vec4 fs_color; > > @@ -103,16 +85,16 @@ void main() > > [vertex data] > piglit_vertex/vec3/3\ > - % for i in range(cols(mat)): > - value/${mat}/${rows(mat)}${'/{}'.format(i) if cols(mat) > 1 else > ''}\ > + % for i in range(mat.columns): > + value/${mat.name}/${mat.rows}${'/{}'.format(i) if mat.columns > 1 > else ''}\ > % endfor > > % for d in range(len(dvalues)): > % for vertex in ('-1.0 -1.0 0.0', ' 1.0 -1.0 0.0', ' > 1.0 1.0 0.0', '-1.0 1.0 0.0'): > ${vertex} \ > - % for i in range(cols(mat)): > - % for j in range(rows(mat)): > -${dvalues[(d + i * rows(mat) + j) % len(dvalues)]} \ > + % for i in range(mat.columns): > + % for j in range(mat.rows): > +${dvalues[(d + i * mat.rows + j) % len(dvalues)]} \ > % endfor > \ > % endfor > @@ -123,10 +105,10 @@ ${dvalues[(d + i * rows(mat) + j) % > len(dvalues)]} \ > [test] > % for d in range(len(dvalues)): > > - uniform ${mat} expected\ > - % for i in range(cols(mat)): > - % for j in range(rows(mat)): > - ${dvalues[(d + i * rows(mat) + j) % len(dvalues)]}\ > + uniform ${mat.name} expected\ > + % for i in range(mat.columns): > + % for j in range(mat.rows): > + ${dvalues[(d + i * mat.rows + j) % len(dvalues)]}\ > % endfor > % endfor > > diff --git > a/generated_tests/templates/gen_vs_in_fp64/regular.shader_test.mako > b/generated_tests/templates/gen_vs_in_fp64/regular.shader_test.mako > index b727b5e..1a76230 100644 > --- > a/generated_tests/templates/gen_vs_in_fp64/regular.shader_test.mako > +++ > b/generated_tests/templates/gen_vs_in_fp64/regular.shader_test.mako > @@ -31,24 +31,6 @@ > glsl_version = '{}.{}'.format(glsl_version_int[0], > glsl_version_int[1:3]) > > return (glsl_version, glsl_version_int) > - > - > - def cols(in_type): > - if 'mat' in in_type: > - if 'x' in in_type: > - return int(in_type[-3:][:1]) > - else: > - return int(in_type[-1:]) > - else: > - return 1 > - > - > - def rows(in_type): > - if 'vec' in in_type or 'mat' in in_type: > - return int(in_type[-1:]) > - else: > - return 1 > - > %> > <% glsl, glsl_int = _version(ver) %> > > @@ -68,10 +50,10 @@ GL_MAX_VERTEX_ATTRIBS >= ${num_vs_in} > % endif > > % for idx, in_type in enumerate(in_types): > - uniform ${in_type} expected${idx}${'[{}]'.format(arrays[idx]) if > arrays[idx] - 1 else ''}; > + uniform ${in_type.name} expected${idx}${'[{}]'.format(arrays[idx]) > if arrays[idx] - 1 else ''}; > % endfor > % for idx, in_type in enumerate(in_types): > - in ${in_type} value${idx}${'[{}]'.format(arrays[idx]) if > arrays[idx] - 1 else ''}; > + in ${in_type.name} value${idx}${'[{}]'.format(arrays[idx]) if > arrays[idx] - 1 else ''}; > % endfor > in vec3 piglit_vertex; > out vec4 fs_color; > @@ -109,8 +91,8 @@ void main() > piglit_vertex/vec3/3 \ > % endif > % for i in range(arrays[idx]): > - % for j in range(cols(in_type)): > - value${idx}${'[{}]'.format(i) if arrays[idx] > 1 else > ''}/${in_type}/${rows(in_type)}${'/{}'.format(j) if cols(in_type) > 1 > else ''} \ > + % for j in range(in_type.columns or 1): > + value${idx}${'[{}]'.format(i) if arrays[idx] > 1 else > ''}/${in_type.name}/${(in_type.rows or 1)}${'/{}'.format(j) if > (in_type.columns or 0) > 1 else ''} \ > % endfor > % endfor > % endfor > @@ -125,9 +107,9 @@ void main() > ${vertex} \ > % endif > % for i in range(arrays[idx]): > - % for j in range(cols(in_type)): > - % for k in range(rows(in_type)): > - ${dvalues[(d + (i * cols(in_type) + j) * rows(in_type) + > k) % len(dvalues)] if in_type.startswith('d') else hvalues[(d + (i * > cols(in_type) + j) * rows(in_type) + k) % len(hvalues)]} \ > + % for j in range(in_type.columns or 1): > + % for k in range(in_type.rows or 1): > + ${dvalues[(d + (i * (in_type.columns or 1) + j) * > (in_type.rows or 1) + k) % len(dvalues)] if in_type.type.name == > 'double' else hvalues[(d + (i * (in_type.columns or 1) + j) * > (in_type.rows or 1) + k) % len(hvalues)]} \ > % endfor > \ > % endfor > @@ -145,10 +127,10 @@ void main() > > % for idx, in_type in enumerate(in_types): > % for i in range(arrays[idx]): > - uniform ${in_type} expected${idx}${'[{}]'.format(i) if > arrays[idx] > 1 else ''}\ > - % for j in range(cols(in_type)): > - % for k in range(rows(in_type)): > - ${dvalues[(d + (i * cols(in_type) + j) * rows(in_type) + k) > % len(dvalues)] if in_type.startswith('d') else hvalues[(d + (i * > cols(in_type) + j) * rows(in_type) + k) % len(hvalues)]}\ > + uniform ${in_type.name} expected${idx}${'[{}]'.format(i) if > arrays[idx] > 1 else ''}\ > + % for j in range(in_type.columns or 1): > + % for k in range(in_type.rows or 1): > + ${dvalues[(d + (i * (in_type.columns or 1) + j) * > (in_type.rows or 1) + k) % len(dvalues)] if in_type.type.name == > 'double' else hvalues[(d + (i * (in_type.columns or 1) + j) * > (in_type.rows or 1) + k) % len(hvalues)]}\ > % endfor > % endfor > -- Br, Andres _______________________________________________ Piglit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/piglit
