From 1f7ffa8ddc89da4d04ddbab1bb4204a336ecbaad Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Mon, 9 May 2022 17:38:20 -0700 Subject: [PATCH 01/16] Initial implementation of code-caching --- Lib/dataclasses.py | 204 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 161 insertions(+), 43 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 4645ebfa71e71c7..c65a5b81b5e4d69 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -9,7 +9,7 @@ import itertools import abc import _thread -from types import FunctionType, GenericAlias +from types import FunctionType, GenericAlias, CellType __all__ = ['dataclass', @@ -279,6 +279,15 @@ def __init__(self, default, default_factory, init, repr, hash, compare, types.MappingProxyType(metadata)) self.kw_only = kw_only self._field_type = None + + def _getcompare(self): + return ( + self.default is MISSING, + self.default_factory is MISSING, + self.init, + self.kw_only, + self._field_type, + ) def __repr__(self): return ('Field(' @@ -386,7 +395,7 @@ def _tuple_str(obj_name, fields): if not fields: return '()' # Note the trailing comma, needed if this turns out to be a 1-tuple. - return f'({",".join([f"{obj_name}.{f.name}" for f in fields])},)' + return f'({",".join([f"{obj_name}.__dataclass_field_name_{i}__" for i in range(len(fields))])},)' # This function's logic is copied from "recursive_repr" function in @@ -433,34 +442,114 @@ def _create_fn(name, args, body, *, globals=None, locals=None, txt = f"def __create_fn__({local_vars}):\n{txt}\n return {name}" ns = {} exec(txt, globals, ns) - return ns['__create_fn__'](**locals) + return ns['__create_fn__'](**locals), txt + +_CODE_CACHE = {} +_CODE_CACHE_TOTAL = 0 +_CODE_CACHE_MISS = 0 + +def _cached_create_fn( + name, + args, + body, + fields, + globals=None, + locals=None, + return_type=MISSING, + salt=None, +): + global _CODE_CACHE_TOTAL, _CODE_CACHE_MISS + key = (name, len(fields), locals and tuple(sorted(locals.keys())), salt) + x = _CODE_CACHE.get(key) + if x is None: + _CODE_CACHE_MISS += 1 + func, txt = _create_fn( + name, args, body, globals=globals, locals=locals + ) + cached, txt = _CODE_CACHE[key] = func.__code__, txt + else: + cached, txt = x + # t = _create_fn( + # name, args, body, globals=globals, locals=locals + # )[1] + # if t != txt: + # raise RuntimeError(f'\n\n{t}\n\n{txt}\n\ncode cache is out of sync') + _CODE_CACHE_TOTAL += 1 + print(f'{_CODE_CACHE_TOTAL} total, {_CODE_CACHE_MISS} miss ({1 - _CODE_CACHE_MISS/_CODE_CACHE_TOTAL:.0%} hit rate)') + if locals is not None: + closure = tuple(CellType(locals[name]) for name in cached.co_freevars) + else: + closure = None + f = FunctionType(cached, globals or {}, closure=closure) + varnames = list(cached.co_varnames) + for i, varname in enumerate(cached.co_varnames): + if varname.startswith('__dataclass_field_name_'): + j = int(varname.removeprefix('__dataclass_field_name_').removesuffix('__')) + varnames[i] = fields[j].name + varnames = tuple(varnames) + names = list(cached.co_names) + for i, name in enumerate(cached.co_names): + if name.startswith('__dataclass_field_name_'): + j = int(name.removeprefix('__dataclass_field_name_').removesuffix('__')) + names[i] = fields[j].name + names = tuple(names) + consts = list(cached.co_consts) + for i, const in enumerate(cached.co_consts): + if isinstance(const, str) and '__dataclass_field_xxx_' in const: + split = const.split('__dataclass_field_xxx_') + for j, s in enumerate(split[1:], 1): + stop = s.index('__') + k = int(s[:stop]) + split[j] = f"{fields[k].name}{s[stop + 2:]}" + consts[i] = ''.join(split) + elif isinstance(const, tuple): + consts_i = list(consts[i]) + for j, const_j in enumerate(consts_i): + if isinstance(const_j, str) and '__dataclass_field_xxx_' in const_j: + split = const_j.split('__dataclass_field_xxx_') + for k, s in enumerate(split[1:], 1): + stop = s.index('__') + l = int(s[:stop]) + split[k] = f"{fields[l].name}{s[stop + 2:]}" + consts_i[j] = ''.join(split) + consts[i] = tuple(consts_i) + consts = tuple(consts) + + f.__code__ = cached.replace( + co_varnames=varnames, + co_names=names, + co_consts=consts, + ) + if return_type is not MISSING: + f.__annotations__['return'] = return_type + return f -def _field_assign(frozen, name, value, self_name): +def _field_assign(frozen, name, value, self_name, i): # If we're a frozen class, then assign to our fields in __init__ # via object.__setattr__. Otherwise, just use a simple # assignment. # # self_name is what "self" is called in this function: don't # hard-code "self", since that might be a field name. - if frozen: - return f'BUILTINS.object.__setattr__({self_name},{name!r},{value})' - return f'{self_name}.{name}={value}' + if frozen: # XXX + return f'BUILTINS.object.__setattr__({self_name},"__dataclass_field_xxx_{i}__",{value})' + return f'{self_name}.__dataclass_field_name_{i}__={value}' -def _field_init(f, frozen, globals, self_name, slots): +def _field_init(f, frozen, globals, self_name, slots, i): # Return the text of the line in the body of __init__ that will # initialize this field. - default_name = f'_dflt_{f.name}' + default_name = f'__dataclass_field_default_{i}__' if f.default_factory is not MISSING: if f.init: # This field has a default factory. If a parameter is # given, use it. If not, call the factory. globals[default_name] = f.default_factory value = (f'{default_name}() ' - f'if {f.name} is _HAS_DEFAULT_FACTORY ' - f'else {f.name}') + f'if __dataclass_field_name_{i}__ is _HAS_DEFAULT_FACTORY ' + f'else __dataclass_field_name_{i}__') else: # This is a field that's not in the __init__ params, but # has a default factory function. It needs to be @@ -483,10 +572,10 @@ def _field_init(f, frozen, globals, self_name, slots): if f.init: if f.default is MISSING: # There's no default, just do an assignment. - value = f.name + value = f"__dataclass_field_name_{i}__" elif f.default is not MISSING: globals[default_name] = f.default - value = f.name + value = f"__dataclass_field_name_{i}__" else: # If the class has slots, then initialize this field. if slots and f.default is not MISSING: @@ -505,10 +594,10 @@ def _field_init(f, frozen, globals, self_name, slots): return None # Now, actually generate the field assignment. - return _field_assign(frozen, f.name, value, self_name) + return _field_assign(frozen, f.name, value, self_name, i) -def _init_param(f): +def _init_param(f, i): # Return the __init__ parameter string for this field. For # example, the equivalent of 'x:int=3' (except instead of 'int', # reference a variable set to int, and instead of '3', reference a @@ -520,11 +609,11 @@ def _init_param(f): elif f.default is not MISSING: # There's a default, this will be the name that's used to look # it up. - default = f'=_dflt_{f.name}' + default = f'=__dataclass_field_default_{i}__' elif f.default_factory is not MISSING: # There's a factory function. Set a marker. default = '=_HAS_DEFAULT_FACTORY' - return f'{f.name}:_type_{f.name}{default}' + return f'__dataclass_field_name_{i}__{default}' def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, @@ -547,15 +636,16 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, raise TypeError(f'non-default argument {f.name!r} ' 'follows default argument') - locals = {f'_type_{f.name}': f.type for f in fields} + locals = {} locals.update({ 'MISSING': MISSING, '_HAS_DEFAULT_FACTORY': _HAS_DEFAULT_FACTORY, + 'BUILTINS': builtins, }) body_lines = [] - for f in fields: - line = _field_init(f, frozen, locals, self_name, slots) + for i, f in enumerate(fields): + line = _field_init(f, frozen, locals, self_name, slots, i) # line is None means that this field doesn't require # initialization (it's a pseudo-field). Just skip it. if line: @@ -563,7 +653,7 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, # Does this class have a post-init function? if has_post_init: - params_str = ','.join(f.name for f in fields + params_str = ','.join(f"__dataclass_field_name_{i}__" for i, f in enumerate(fields) if f._field_type is _FIELD_INITVAR) body_lines.append(f'{self_name}.{_POST_INIT_NAME}({params_str})') @@ -571,77 +661,105 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, if not body_lines: body_lines = ['pass'] - _init_params = [_init_param(f) for f in std_fields] + _init_params = [_init_param(f, fields.index(f)) for f in std_fields] if kw_only_fields: # Add the keyword-only args. Because the * can only be added if # there's at least one keyword-only arg, there needs to be a test here # (instead of just concatenting the lists together). _init_params += ['*'] - _init_params += [_init_param(f) for f in kw_only_fields] - return _create_fn('__init__', + _init_params += [_init_param(f, fields.index(f)) for f in kw_only_fields] + f = _cached_create_fn('__init__', [self_name] + _init_params, body_lines, locals=locals, globals=globals, - return_type=None) + return_type=None, + fields=fields, salt=(frozen, tuple(f._getcompare() for f in fields), has_post_init, self_name)) + defaults = [] + kwdefaults = {} + for field in std_fields: + f.__annotations__[field.name] = field.type + if field.default is MISSING and field.default_factory is MISSING: + # There's no default, and no default_factory, just output the + # variable name and type. + pass + elif field.default is not MISSING: + # There's a default, this will be the name that's used to look + # it up. + defaults.append(field.default) + elif field.default_factory is not MISSING: + # There's a factory function. Set a marker. + defaults.append(_HAS_DEFAULT_FACTORY) + for field in kw_only_fields: + f.__annotations__[field.name] = field.type + if field.default is not MISSING: + kwdefaults[field.name] = field.default + f.__kwdefaults__ = kwdefaults or None + f.__defaults__ = tuple(defaults) or None + return f def _repr_fn(fields, globals): - fn = _create_fn('__repr__', + fn = _cached_create_fn('__repr__', ('self',), - ['return self.__class__.__qualname__ + f"(' + - ', '.join([f"{f.name}={{self.{f.name}!r}}" - for f in fields]) + + ['return self.__class__.__qualname__ + f"(' + # XXX + ', '.join([f"__dataclass_field_xxx_{i}__={{self.__dataclass_field_name_{i}__!r}}" + for i in range(len(fields))]) + ')"'], - globals=globals) + globals=globals, + fields=fields) return _recursive_repr(fn) def _frozen_get_del_attr(cls, fields, globals): locals = {'cls': cls, 'FrozenInstanceError': FrozenInstanceError} - if fields: - fields_str = '(' + ','.join(repr(f.name) for f in fields) + ',)' + if fields: # XXX + fields_str = '(' + ','.join(repr(f"__dataclass_field_xxx_{i}__") for i in range(len(fields))) + ',)' else: # Special case for the zero-length tuple. fields_str = '()' - return (_create_fn('__setattr__', + return (_cached_create_fn('__setattr__', ('self', 'name', 'value'), (f'if type(self) is cls or name in {fields_str}:', ' raise FrozenInstanceError(f"cannot assign to field {name!r}")', f'super(cls, self).__setattr__(name, value)'), locals=locals, - globals=globals), - _create_fn('__delattr__', + globals=globals, + fields=fields), + _cached_create_fn('__delattr__', ('self', 'name'), (f'if type(self) is cls or name in {fields_str}:', ' raise FrozenInstanceError(f"cannot delete field {name!r}")', f'super(cls, self).__delattr__(name)'), locals=locals, - globals=globals), + globals=globals, + fields=fields), ) -def _cmp_fn(name, op, self_tuple, other_tuple, globals): +def _cmp_fn(name, op, self_tuple, other_tuple, globals, fields): # Create a comparison function. If the fields in the object are # named 'x' and 'y', then self_tuple is the string # '(self.x,self.y)' and other_tuple is the string # '(other.x,other.y)'. - return _create_fn(name, + return _cached_create_fn(name, ('self', 'other'), [ 'if other.__class__ is self.__class__:', f' return {self_tuple}{op}{other_tuple}', 'return NotImplemented'], - globals=globals) + globals=globals, + fields=fields) def _hash_fn(fields, globals): self_tuple = _tuple_str('self', fields) - return _create_fn('__hash__', + return _cached_create_fn('__hash__', ('self',), [f'return hash({self_tuple})'], - globals=globals) + globals=globals, + fields=fields) def _is_classvar(a_type, typing): @@ -1056,7 +1174,7 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, _set_new_attribute(cls, '__eq__', _cmp_fn('__eq__', '==', self_tuple, other_tuple, - globals=globals)) + globals=globals, fields=flds)) if order: # Create and set the ordering methods. @@ -1070,7 +1188,7 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, ]: if _set_new_attribute(cls, name, _cmp_fn(name, op, self_tuple, other_tuple, - globals=globals)): + globals=globals, fields=flds)): raise TypeError(f'Cannot overwrite attribute {name} ' f'in class {cls.__name__}. Consider using ' 'functools.total_ordering') From 17dd0d06e339d7a1d89499ed80b03eaaadb1f9fa Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 May 2022 11:31:06 -0700 Subject: [PATCH 02/16] More cleanup --- Lib/dataclasses.py | 209 ++++++++++++++++++--------------------------- 1 file changed, 85 insertions(+), 124 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index c65a5b81b5e4d69..7a84f0947d0bf84 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -9,7 +9,7 @@ import itertools import abc import _thread -from types import FunctionType, GenericAlias, CellType +from types import CellType, FunctionType, GenericAlias __all__ = ['dataclass', @@ -395,7 +395,7 @@ def _tuple_str(obj_name, fields): if not fields: return '()' # Note the trailing comma, needed if this turns out to be a 1-tuple. - return f'({",".join([f"{obj_name}.__dataclass_field_name_{i}__" for i in range(len(fields))])},)' + return f'({",".join([f"{obj_name}.__field_{i}__" for i in range(len(fields))])},)' # This function's logic is copied from "recursive_repr" function in @@ -419,8 +419,7 @@ def wrapper(self): return wrapper -def _create_fn(name, args, body, *, globals=None, locals=None, - return_type=MISSING): +def _create_code(name, args, body, *, globals=None, locals=None): # Note that we mutate locals when exec() is called. Caller # beware! The only callers are internal to this module, so no # worries about external callers. @@ -428,101 +427,64 @@ def _create_fn(name, args, body, *, globals=None, locals=None, locals = {} if 'BUILTINS' not in locals: locals['BUILTINS'] = builtins - return_annotation = '' - if return_type is not MISSING: - locals['_return_type'] = return_type - return_annotation = '->_return_type' args = ','.join(args) body = '\n'.join(f' {b}' for b in body) # Compute the text of the entire function. - txt = f' def {name}({args}){return_annotation}:\n{body}' + txt = f' def {name}({args}):\n{body}' local_vars = ', '.join(locals.keys()) txt = f"def __create_fn__({local_vars}):\n{txt}\n return {name}" ns = {} exec(txt, globals, ns) - return ns['__create_fn__'](**locals), txt - -_CODE_CACHE = {} -_CODE_CACHE_TOTAL = 0 -_CODE_CACHE_MISS = 0 - -def _cached_create_fn( - name, - args, - body, - fields, - globals=None, - locals=None, - return_type=MISSING, - salt=None, + return ns['__create_fn__'](**locals).__code__ + + +_code_cache = {} +_code_cache_total = 0 +_code_cache_miss = 0 + + +def _create_fn( + name, args, body, fields, globals=None, locals=None, salt=None, ): - global _CODE_CACHE_TOTAL, _CODE_CACHE_MISS - key = (name, len(fields), locals and tuple(sorted(locals.keys())), salt) - x = _CODE_CACHE.get(key) - if x is None: - _CODE_CACHE_MISS += 1 - func, txt = _create_fn( - name, args, body, globals=globals, locals=locals + key = (name, len(fields), locals and frozenset(locals), salt) + code = _code_cache.get(key) + global _code_cache_total, _code_cache_miss + _code_cache_total += 1 + if code is None: + _code_cache_miss += 1 + code = _code_cache[key] = _create_code( + name, args, body, globals=globals, locals=locals ) - cached, txt = _CODE_CACHE[key] = func.__code__, txt - else: - cached, txt = x - # t = _create_fn( - # name, args, body, globals=globals, locals=locals - # )[1] - # if t != txt: - # raise RuntimeError(f'\n\n{t}\n\n{txt}\n\ncode cache is out of sync') - _CODE_CACHE_TOTAL += 1 - print(f'{_CODE_CACHE_TOTAL} total, {_CODE_CACHE_MISS} miss ({1 - _CODE_CACHE_MISS/_CODE_CACHE_TOTAL:.0%} hit rate)') - if locals is not None: - closure = tuple(CellType(locals[name]) for name in cached.co_freevars) - else: - closure = None - f = FunctionType(cached, globals or {}, closure=closure) - varnames = list(cached.co_varnames) - for i, varname in enumerate(cached.co_varnames): - if varname.startswith('__dataclass_field_name_'): - j = int(varname.removeprefix('__dataclass_field_name_').removesuffix('__')) - varnames[i] = fields[j].name - varnames = tuple(varnames) - names = list(cached.co_names) - for i, name in enumerate(cached.co_names): - if name.startswith('__dataclass_field_name_'): - j = int(name.removeprefix('__dataclass_field_name_').removesuffix('__')) - names[i] = fields[j].name - names = tuple(names) - consts = list(cached.co_consts) - for i, const in enumerate(cached.co_consts): - if isinstance(const, str) and '__dataclass_field_xxx_' in const: - split = const.split('__dataclass_field_xxx_') - for j, s in enumerate(split[1:], 1): - stop = s.index('__') - k = int(s[:stop]) - split[j] = f"{fields[k].name}{s[stop + 2:]}" - consts[i] = ''.join(split) + patched = {f"__field_{i}__": f.name for i, f in enumerate(fields)} + varnames = tuple(patched.get(name, name) for name in code.co_varnames) + names = tuple(patched.get(name, name) for name in code.co_names) + consts = list(code.co_consts) + for i, const in enumerate(code.co_consts): + if isinstance(const, str): + for pair in patched.items(): + const = const.replace(*pair) + consts[i] = const elif isinstance(const, tuple): - consts_i = list(consts[i]) - for j, const_j in enumerate(consts_i): - if isinstance(const_j, str) and '__dataclass_field_xxx_' in const_j: - split = const_j.split('__dataclass_field_xxx_') - for k, s in enumerate(split[1:], 1): - stop = s.index('__') - l = int(s[:stop]) - split[k] = f"{fields[l].name}{s[stop + 2:]}" - consts_i[j] = ''.join(split) + consts_i = list(const) + for j, const in enumerate(consts_i): + if isinstance(const, str): + for pair in patched.items(): + const = const.replace(*pair) + consts_i[j] = const consts[i] = tuple(consts_i) consts = tuple(consts) - - f.__code__ = cached.replace( - co_varnames=varnames, - co_names=names, - co_consts=consts, + closure = tuple(CellType(locals[freevar]) for freevar in code.co_freevars) + return FunctionType( + code=code.replace( + co_varnames=varnames, + co_names=names, + co_consts=consts, + ), + globals=globals or {}, + closure=closure, ) - if return_type is not MISSING: - f.__annotations__['return'] = return_type - return f def _field_assign(frozen, name, value, self_name, i): @@ -532,24 +494,24 @@ def _field_assign(frozen, name, value, self_name, i): # # self_name is what "self" is called in this function: don't # hard-code "self", since that might be a field name. - if frozen: # XXX - return f'BUILTINS.object.__setattr__({self_name},"__dataclass_field_xxx_{i}__",{value})' - return f'{self_name}.__dataclass_field_name_{i}__={value}' + if frozen: + return f'BUILTINS.object.__setattr__({self_name},"__field_{i}__",{value})' + return f'{self_name}.__field_{i}__={value}' def _field_init(f, frozen, globals, self_name, slots, i): # Return the text of the line in the body of __init__ that will # initialize this field. - default_name = f'__dataclass_field_default_{i}__' + default_name = f'__field_default_{i}__' if f.default_factory is not MISSING: if f.init: # This field has a default factory. If a parameter is # given, use it. If not, call the factory. globals[default_name] = f.default_factory value = (f'{default_name}() ' - f'if __dataclass_field_name_{i}__ is _HAS_DEFAULT_FACTORY ' - f'else __dataclass_field_name_{i}__') + f'if __field_{i}__ is _HAS_DEFAULT_FACTORY ' + f'else __field_{i}__') else: # This is a field that's not in the __init__ params, but # has a default factory function. It needs to be @@ -572,10 +534,10 @@ def _field_init(f, frozen, globals, self_name, slots, i): if f.init: if f.default is MISSING: # There's no default, just do an assignment. - value = f"__dataclass_field_name_{i}__" + value = f"__field_{i}__" elif f.default is not MISSING: globals[default_name] = f.default - value = f"__dataclass_field_name_{i}__" + value = f"__field_{i}__" else: # If the class has slots, then initialize this field. if slots and f.default is not MISSING: @@ -609,11 +571,11 @@ def _init_param(f, i): elif f.default is not MISSING: # There's a default, this will be the name that's used to look # it up. - default = f'=__dataclass_field_default_{i}__' + default = f'=__field_default_{i}__' elif f.default_factory is not MISSING: # There's a factory function. Set a marker. default = '=_HAS_DEFAULT_FACTORY' - return f'__dataclass_field_name_{i}__{default}' + return f'__field_{i}__{default}' def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, @@ -636,12 +598,11 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, raise TypeError(f'non-default argument {f.name!r} ' 'follows default argument') - locals = {} - locals.update({ + locals = { 'MISSING': MISSING, '_HAS_DEFAULT_FACTORY': _HAS_DEFAULT_FACTORY, 'BUILTINS': builtins, - }) + } body_lines = [] for i, f in enumerate(fields): @@ -653,7 +614,7 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, # Does this class have a post-init function? if has_post_init: - params_str = ','.join(f"__dataclass_field_name_{i}__" for i, f in enumerate(fields) + params_str = ','.join(f"__field_{i}__" for i, f in enumerate(fields) if f._field_type is _FIELD_INITVAR) body_lines.append(f'{self_name}.{_POST_INIT_NAME}({params_str})') @@ -668,42 +629,42 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, # (instead of just concatenting the lists together). _init_params += ['*'] _init_params += [_init_param(f, fields.index(f)) for f in kw_only_fields] - f = _cached_create_fn('__init__', - [self_name] + _init_params, - body_lines, - locals=locals, - globals=globals, - return_type=None, - fields=fields, salt=(frozen, tuple(f._getcompare() for f in fields), has_post_init, self_name)) + salt = (frozen, tuple(f._getcompare() for f in fields), has_post_init, self_name) + f = _create_fn( + '__init__', + [self_name] + _init_params, + body_lines, + locals=locals, + globals=globals, + fields=fields, + salt=salt, + ) + annotations = {"return": None} defaults = [] kwdefaults = {} for field in std_fields: - f.__annotations__[field.name] = field.type - if field.default is MISSING and field.default_factory is MISSING: - # There's no default, and no default_factory, just output the - # variable name and type. - pass - elif field.default is not MISSING: - # There's a default, this will be the name that's used to look - # it up. + annotations[field.name] = field.type + if field.default is not MISSING: defaults.append(field.default) elif field.default_factory is not MISSING: - # There's a factory function. Set a marker. defaults.append(_HAS_DEFAULT_FACTORY) for field in kw_only_fields: - f.__annotations__[field.name] = field.type + annotations[field.name] = field.type if field.default is not MISSING: kwdefaults[field.name] = field.default - f.__kwdefaults__ = kwdefaults or None + elif field.default_factory is not MISSING: + defaults.append(_HAS_DEFAULT_FACTORY) + f.__annotations__ = annotations f.__defaults__ = tuple(defaults) or None + f.__kwdefaults__ = kwdefaults or None return f def _repr_fn(fields, globals): - fn = _cached_create_fn('__repr__', + fn = _create_fn('__repr__', ('self',), ['return self.__class__.__qualname__ + f"(' + # XXX - ', '.join([f"__dataclass_field_xxx_{i}__={{self.__dataclass_field_name_{i}__!r}}" + ', '.join([f"__field_{i}__={{self.__field_{i}__!r}}" for i in range(len(fields))]) + ')"'], globals=globals, @@ -715,11 +676,11 @@ def _frozen_get_del_attr(cls, fields, globals): locals = {'cls': cls, 'FrozenInstanceError': FrozenInstanceError} if fields: # XXX - fields_str = '(' + ','.join(repr(f"__dataclass_field_xxx_{i}__") for i in range(len(fields))) + ',)' + fields_str = '(' + ','.join(repr(f"__field_{i}__") for i in range(len(fields))) + ',)' else: # Special case for the zero-length tuple. fields_str = '()' - return (_cached_create_fn('__setattr__', + return (_create_fn('__setattr__', ('self', 'name', 'value'), (f'if type(self) is cls or name in {fields_str}:', ' raise FrozenInstanceError(f"cannot assign to field {name!r}")', @@ -727,7 +688,7 @@ def _frozen_get_del_attr(cls, fields, globals): locals=locals, globals=globals, fields=fields), - _cached_create_fn('__delattr__', + _create_fn('__delattr__', ('self', 'name'), (f'if type(self) is cls or name in {fields_str}:', ' raise FrozenInstanceError(f"cannot delete field {name!r}")', @@ -744,7 +705,7 @@ def _cmp_fn(name, op, self_tuple, other_tuple, globals, fields): # '(self.x,self.y)' and other_tuple is the string # '(other.x,other.y)'. - return _cached_create_fn(name, + return _create_fn(name, ('self', 'other'), [ 'if other.__class__ is self.__class__:', f' return {self_tuple}{op}{other_tuple}', @@ -755,7 +716,7 @@ def _cmp_fn(name, op, self_tuple, other_tuple, globals, fields): def _hash_fn(fields, globals): self_tuple = _tuple_str('self', fields) - return _cached_create_fn('__hash__', + return _create_fn('__hash__', ('self',), [f'return hash({self_tuple})'], globals=globals, @@ -937,7 +898,7 @@ def _get_field(cls, a_name, a_type, default_kw_only): return f def _set_qualname(cls, value): - # Ensure that the functions returned from _create_fn uses the proper + # Ensure that the functions returned from _create_code uses the proper # __qualname__ (the class they belong to). if isinstance(value, FunctionType): value.__qualname__ = f"{cls.__qualname__}.{value.__name__}" From 7096466c1b5240f46141775682c838b321b58225 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 May 2022 12:51:35 -0700 Subject: [PATCH 03/16] More more cleanup --- Lib/dataclasses.py | 92 ++++++++++++++++++---------------------------- 1 file changed, 36 insertions(+), 56 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 7a84f0947d0bf84..d8c5a368c8f30ea 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -280,7 +280,7 @@ def __init__(self, default, default_factory, init, repr, hash, compare, self.kw_only = kw_only self._field_type = None - def _getcompare(self): + def _extra_keys(self): return ( self.default is MISSING, self.default_factory is MISSING, @@ -377,6 +377,9 @@ def field(*, default=MISSING, default_factory=MISSING, init=True, repr=True, metadata, kw_only) +symbol = "__field_{}__".format + + def _fields_in_init_order(fields): # Returns the fields as __init__ will output them. It returns 2 tuples: # the first for normal args, and the second for keyword args. @@ -395,7 +398,7 @@ def _tuple_str(obj_name, fields): if not fields: return '()' # Note the trailing comma, needed if this turns out to be a 1-tuple. - return f'({",".join([f"{obj_name}.__field_{i}__" for i in range(len(fields))])},)' + return f'({"".join([f"{obj_name}.{symbol(i)}," for i in range(len(fields))])})' # This function's logic is copied from "recursive_repr" function in @@ -446,18 +449,18 @@ def _create_code(name, args, body, *, globals=None, locals=None): def _create_fn( - name, args, body, fields, globals=None, locals=None, salt=None, + name, args, body, fields, extra_keys=(), globals=None, locals=None ): - key = (name, len(fields), locals and frozenset(locals), salt) - code = _code_cache.get(key) global _code_cache_total, _code_cache_miss + key = (name, len(fields), locals and frozenset(locals), *extra_keys) + code = _code_cache.get(key) _code_cache_total += 1 if code is None: _code_cache_miss += 1 code = _code_cache[key] = _create_code( name, args, body, globals=globals, locals=locals ) - patched = {f"__field_{i}__": f.name for i, f in enumerate(fields)} + patched = {symbol(i): f.name for i, f in enumerate(fields)} varnames = tuple(patched.get(name, name) for name in code.co_varnames) names = tuple(patched.get(name, name) for name in code.co_names) consts = list(code.co_consts) @@ -487,7 +490,7 @@ def _create_fn( ) -def _field_assign(frozen, name, value, self_name, i): +def _field_assign(frozen, name, value, self_name): # If we're a frozen class, then assign to our fields in __init__ # via object.__setattr__. Otherwise, just use a simple # assignment. @@ -495,23 +498,24 @@ def _field_assign(frozen, name, value, self_name, i): # self_name is what "self" is called in this function: don't # hard-code "self", since that might be a field name. if frozen: - return f'BUILTINS.object.__setattr__({self_name},"__field_{i}__",{value})' - return f'{self_name}.__field_{i}__={value}' + return f'BUILTINS.object.__setattr__({self_name},{name!r},{value})' + return f'{self_name}.{name}={value}' def _field_init(f, frozen, globals, self_name, slots, i): # Return the text of the line in the body of __init__ that will # initialize this field. - default_name = f'__field_default_{i}__' + field_name = symbol(i) + default_name = f'__default_{i}__' if f.default_factory is not MISSING: if f.init: # This field has a default factory. If a parameter is # given, use it. If not, call the factory. globals[default_name] = f.default_factory value = (f'{default_name}() ' - f'if __field_{i}__ is _HAS_DEFAULT_FACTORY ' - f'else __field_{i}__') + f'if {field_name} is _HAS_DEFAULT_FACTORY ' + f'else {field_name}') else: # This is a field that's not in the __init__ params, but # has a default factory function. It needs to be @@ -534,10 +538,10 @@ def _field_init(f, frozen, globals, self_name, slots, i): if f.init: if f.default is MISSING: # There's no default, just do an assignment. - value = f"__field_{i}__" + value = field_name elif f.default is not MISSING: globals[default_name] = f.default - value = f"__field_{i}__" + value = field_name else: # If the class has slots, then initialize this field. if slots and f.default is not MISSING: @@ -556,27 +560,7 @@ def _field_init(f, frozen, globals, self_name, slots, i): return None # Now, actually generate the field assignment. - return _field_assign(frozen, f.name, value, self_name, i) - - -def _init_param(f, i): - # Return the __init__ parameter string for this field. For - # example, the equivalent of 'x:int=3' (except instead of 'int', - # reference a variable set to int, and instead of '3', reference a - # variable set to 3). - if f.default is MISSING and f.default_factory is MISSING: - # There's no default, and no default_factory, just output the - # variable name and type. - default = '' - elif f.default is not MISSING: - # There's a default, this will be the name that's used to look - # it up. - default = f'=__field_default_{i}__' - elif f.default_factory is not MISSING: - # There's a factory function. Set a marker. - default = '=_HAS_DEFAULT_FACTORY' - return f'__field_{i}__{default}' - + return _field_assign(frozen, field_name, value, self_name) def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, self_name, globals, slots): @@ -614,7 +598,7 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, # Does this class have a post-init function? if has_post_init: - params_str = ','.join(f"__field_{i}__" for i, f in enumerate(fields) + params_str = ','.join(symbol(i) for i, f in enumerate(fields) if f._field_type is _FIELD_INITVAR) body_lines.append(f'{self_name}.{_POST_INIT_NAME}({params_str})') @@ -622,23 +606,23 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, if not body_lines: body_lines = ['pass'] - _init_params = [_init_param(f, fields.index(f)) for f in std_fields] + _init_params = [symbol(fields.index(f)) for f in std_fields] if kw_only_fields: # Add the keyword-only args. Because the * can only be added if # there's at least one keyword-only arg, there needs to be a test here # (instead of just concatenting the lists together). _init_params += ['*'] - _init_params += [_init_param(f, fields.index(f)) for f in kw_only_fields] - salt = (frozen, tuple(f._getcompare() for f in fields), has_post_init, self_name) - f = _create_fn( - '__init__', - [self_name] + _init_params, - body_lines, - locals=locals, - globals=globals, - fields=fields, - salt=salt, + _init_params += [symbol(fields.index(f)) for f in kw_only_fields] + extra_keys = ( + frozen, *(f._extra_keys() for f in fields), has_post_init, self_name ) + f = _create_fn('__init__', + [self_name] + _init_params, + body_lines, + locals=locals, + globals=globals, + fields=fields, + extra_keys=extra_keys) annotations = {"return": None} defaults = [] kwdefaults = {} @@ -653,7 +637,7 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, if field.default is not MISSING: kwdefaults[field.name] = field.default elif field.default_factory is not MISSING: - defaults.append(_HAS_DEFAULT_FACTORY) + kwdefaults[field.name] = _HAS_DEFAULT_FACTORY f.__annotations__ = annotations f.__defaults__ = tuple(defaults) or None f.__kwdefaults__ = kwdefaults or None @@ -663,8 +647,8 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, def _repr_fn(fields, globals): fn = _create_fn('__repr__', ('self',), - ['return self.__class__.__qualname__ + f"(' + # XXX - ', '.join([f"__field_{i}__={{self.__field_{i}__!r}}" + ['return self.__class__.__qualname__ + f"(' + + ', '.join([f"{symbol(i)}={{self.{symbol(i)}!r}}" for i in range(len(fields))]) + ')"'], globals=globals, @@ -675,11 +659,7 @@ def _repr_fn(fields, globals): def _frozen_get_del_attr(cls, fields, globals): locals = {'cls': cls, 'FrozenInstanceError': FrozenInstanceError} - if fields: # XXX - fields_str = '(' + ','.join(repr(f"__field_{i}__") for i in range(len(fields))) + ',)' - else: - # Special case for the zero-length tuple. - fields_str = '()' + fields_str = '(' + ''.join(f"{symbol(i)!r}," for i in range(len(fields))) + ')' return (_create_fn('__setattr__', ('self', 'name', 'value'), (f'if type(self) is cls or name in {fields_str}:', @@ -898,7 +878,7 @@ def _get_field(cls, a_name, a_type, default_kw_only): return f def _set_qualname(cls, value): - # Ensure that the functions returned from _create_code uses the proper + # Ensure that the functions returned from _create_fn uses the proper # __qualname__ (the class they belong to). if isinstance(value, FunctionType): value.__qualname__ = f"{cls.__qualname__}.{value.__name__}" From 52113c5b57dc403ab423605848061e3a6357b1e4 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 May 2022 12:57:06 -0700 Subject: [PATCH 04/16] Smaller diff --- Lib/dataclasses.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index d8c5a368c8f30ea..78c73dd30d1a7cd 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -398,7 +398,7 @@ def _tuple_str(obj_name, fields): if not fields: return '()' # Note the trailing comma, needed if this turns out to be a 1-tuple. - return f'({"".join([f"{obj_name}.{symbol(i)}," for i in range(len(fields))])})' + return f'({",".join([f"{obj_name}.{symbol(i)}" for i in range(len(fields))])},)' # This function's logic is copied from "recursive_repr" function in @@ -619,10 +619,10 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, f = _create_fn('__init__', [self_name] + _init_params, body_lines, - locals=locals, - globals=globals, fields=fields, - extra_keys=extra_keys) + extra_keys=extra_keys, + locals=locals, + globals=globals) annotations = {"return": None} defaults = [] kwdefaults = {} @@ -651,8 +651,8 @@ def _repr_fn(fields, globals): ', '.join([f"{symbol(i)}={{self.{symbol(i)}!r}}" for i in range(len(fields))]) + ')"'], - globals=globals, - fields=fields) + fields=fields, + globals=globals) return _recursive_repr(fn) @@ -665,17 +665,17 @@ def _frozen_get_del_attr(cls, fields, globals): (f'if type(self) is cls or name in {fields_str}:', ' raise FrozenInstanceError(f"cannot assign to field {name!r}")', f'super(cls, self).__setattr__(name, value)'), + fields=fields, locals=locals, - globals=globals, - fields=fields), + globals=globals), _create_fn('__delattr__', ('self', 'name'), (f'if type(self) is cls or name in {fields_str}:', ' raise FrozenInstanceError(f"cannot delete field {name!r}")', f'super(cls, self).__delattr__(name)'), + fields=fields, locals=locals, - globals=globals, - fields=fields), + globals=globals), ) @@ -690,8 +690,8 @@ def _cmp_fn(name, op, self_tuple, other_tuple, globals, fields): [ 'if other.__class__ is self.__class__:', f' return {self_tuple}{op}{other_tuple}', 'return NotImplemented'], - globals=globals, - fields=fields) + fields=fields, + globals=globals) def _hash_fn(fields, globals): @@ -699,8 +699,8 @@ def _hash_fn(fields, globals): return _create_fn('__hash__', ('self',), [f'return hash({self_tuple})'], - globals=globals, - fields=fields) + fields=fields, + globals=globals) def _is_classvar(a_type, typing): @@ -1115,7 +1115,7 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, _set_new_attribute(cls, '__eq__', _cmp_fn('__eq__', '==', self_tuple, other_tuple, - globals=globals, fields=flds)) + fields=flds, globals=globals)) if order: # Create and set the ordering methods. @@ -1129,7 +1129,7 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, ]: if _set_new_attribute(cls, name, _cmp_fn(name, op, self_tuple, other_tuple, - globals=globals, fields=flds)): + fields=flds, globals=globals)): raise TypeError(f'Cannot overwrite attribute {name} ' f'in class {cls.__name__}. Consider using ' 'functools.total_ordering') From 62d0c876daf957dd0db189491c458960db2736d9 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 May 2022 13:50:16 -0700 Subject: [PATCH 05/16] More diff cleanup --- Lib/dataclasses.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 78c73dd30d1a7cd..bf92e8117a3de73 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -377,7 +377,8 @@ def field(*, default=MISSING, default_factory=MISSING, init=True, repr=True, metadata, kw_only) -symbol = "__field_{}__".format +def _symbol(i): + return f"__field_{i}__" def _fields_in_init_order(fields): @@ -398,7 +399,7 @@ def _tuple_str(obj_name, fields): if not fields: return '()' # Note the trailing comma, needed if this turns out to be a 1-tuple. - return f'({",".join([f"{obj_name}.{symbol(i)}" for i in range(len(fields))])},)' + return f'({",".join([f"{obj_name}.{_symbol(i)}" for i in range(len(fields))])},)' # This function's logic is copied from "recursive_repr" function in @@ -460,7 +461,7 @@ def _create_fn( code = _code_cache[key] = _create_code( name, args, body, globals=globals, locals=locals ) - patched = {symbol(i): f.name for i, f in enumerate(fields)} + patched = {_symbol(i): f.name for i, f in enumerate(fields)} varnames = tuple(patched.get(name, name) for name in code.co_varnames) names = tuple(patched.get(name, name) for name in code.co_names) consts = list(code.co_consts) @@ -506,7 +507,7 @@ def _field_init(f, frozen, globals, self_name, slots, i): # Return the text of the line in the body of __init__ that will # initialize this field. - field_name = symbol(i) + field_name = _symbol(i) default_name = f'__default_{i}__' if f.default_factory is not MISSING: if f.init: @@ -598,7 +599,7 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, # Does this class have a post-init function? if has_post_init: - params_str = ','.join(symbol(i) for i, f in enumerate(fields) + params_str = ','.join(_symbol(i) for i, f in enumerate(fields) if f._field_type is _FIELD_INITVAR) body_lines.append(f'{self_name}.{_POST_INIT_NAME}({params_str})') @@ -606,13 +607,13 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, if not body_lines: body_lines = ['pass'] - _init_params = [symbol(fields.index(f)) for f in std_fields] + _init_params = [_symbol(fields.index(f)) for f in std_fields] if kw_only_fields: # Add the keyword-only args. Because the * can only be added if # there's at least one keyword-only arg, there needs to be a test here # (instead of just concatenting the lists together). _init_params += ['*'] - _init_params += [symbol(fields.index(f)) for f in kw_only_fields] + _init_params += [_symbol(fields.index(f)) for f in kw_only_fields] extra_keys = ( frozen, *(f._extra_keys() for f in fields), has_post_init, self_name ) @@ -648,7 +649,7 @@ def _repr_fn(fields, globals): fn = _create_fn('__repr__', ('self',), ['return self.__class__.__qualname__ + f"(' + - ', '.join([f"{symbol(i)}={{self.{symbol(i)}!r}}" + ', '.join([f"{_symbol(i)}={{self.{_symbol(i)}!r}}" for i in range(len(fields))]) + ')"'], fields=fields, @@ -659,7 +660,11 @@ def _repr_fn(fields, globals): def _frozen_get_del_attr(cls, fields, globals): locals = {'cls': cls, 'FrozenInstanceError': FrozenInstanceError} - fields_str = '(' + ''.join(f"{symbol(i)!r}," for i in range(len(fields))) + ')' + if fields: + fields_str = '(' + ','.join(repr(_symbol(i)) for i in range(len(fields))) + ',)' + else: + # Special case for the zero-length tuple. + fields_str = '()' return (_create_fn('__setattr__', ('self', 'name', 'value'), (f'if type(self) is cls or name in {fields_str}:', From af64d69721a3a5c6d137b5e4b7292d5726bf49b6 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 May 2022 14:14:26 -0700 Subject: [PATCH 06/16] Revert unnecessary changes --- Lib/dataclasses.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index bf92e8117a3de73..70dd222a91e1398 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -450,7 +450,7 @@ def _create_code(name, args, body, *, globals=None, locals=None): def _create_fn( - name, args, body, fields, extra_keys=(), globals=None, locals=None + name, args, body, fields, *, extra_keys=(), globals=None, locals=None ): global _code_cache_total, _code_cache_miss key = (name, len(fields), locals and frozenset(locals), *extra_keys) @@ -508,7 +508,7 @@ def _field_init(f, frozen, globals, self_name, slots, i): # initialize this field. field_name = _symbol(i) - default_name = f'__default_{i}__' + default_name = f'_dflt_{i}' if f.default_factory is not MISSING: if f.init: # This field has a default factory. If a parameter is From e3773931f130fc8590076664c908b0b0cc947cbb Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 May 2022 14:22:42 -0700 Subject: [PATCH 07/16] Remove unnecessary code --- Lib/dataclasses.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 70dd222a91e1398..b47d379a7ba8b37 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -393,7 +393,7 @@ def _fields_in_init_order(fields): def _tuple_str(obj_name, fields): # Return a string representing each field of obj_name as a tuple # member. So, if fields is ['x', 'y'] and obj_name is "self", - # return "(self.x,self.y)". + # return "(self.__fields_0__,self.__fields_1__)". # Special case for the 0-tuple. if not fields: @@ -429,8 +429,6 @@ def _create_code(name, args, body, *, globals=None, locals=None): # worries about external callers. if locals is None: locals = {} - if 'BUILTINS' not in locals: - locals['BUILTINS'] = builtins args = ','.join(args) body = '\n'.join(f' {b}' for b in body) From 9d01e5c7c1aab4a7f11fee7d8197f95962eb50db Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 May 2022 14:35:26 -0700 Subject: [PATCH 08/16] Simplify tuple patching --- Lib/dataclasses.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index b47d379a7ba8b37..5610af4ecf23eeb 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -471,10 +471,7 @@ def _create_fn( elif isinstance(const, tuple): consts_i = list(const) for j, const in enumerate(consts_i): - if isinstance(const, str): - for pair in patched.items(): - const = const.replace(*pair) - consts_i[j] = const + consts_i[j] = patched.get(const, const) consts[i] = tuple(consts_i) consts = tuple(consts) closure = tuple(CellType(locals[freevar]) for freevar in code.co_freevars) From c9acd33f937f506910fd9ef838e34bc9fe6ce9a1 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 May 2022 17:23:39 -0700 Subject: [PATCH 09/16] Further improve tuple patching --- Lib/dataclasses.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 5610af4ecf23eeb..d4ee32894fd9e5d 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -469,10 +469,7 @@ def _create_fn( const = const.replace(*pair) consts[i] = const elif isinstance(const, tuple): - consts_i = list(const) - for j, const in enumerate(consts_i): - consts_i[j] = patched.get(const, const) - consts[i] = tuple(consts_i) + consts[i] = tuple(patched.get(c, c) for c in const) consts = tuple(consts) closure = tuple(CellType(locals[freevar]) for freevar in code.co_freevars) return FunctionType( From 82f1c75a0ed4b6e260bc66ee0f1f1227d04256d5 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 11 May 2022 09:38:26 -0700 Subject: [PATCH 10/16] Simplify cache keying --- Lib/dataclasses.py | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index d4ee32894fd9e5d..a95b27bb81c09a3 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -279,15 +279,6 @@ def __init__(self, default, default_factory, init, repr, hash, compare, types.MappingProxyType(metadata)) self.kw_only = kw_only self._field_type = None - - def _extra_keys(self): - return ( - self.default is MISSING, - self.default_factory is MISSING, - self.init, - self.kw_only, - self._field_type, - ) def __repr__(self): return ('Field(' @@ -447,11 +438,9 @@ def _create_code(name, args, body, *, globals=None, locals=None): _code_cache_miss = 0 -def _create_fn( - name, args, body, fields, *, extra_keys=(), globals=None, locals=None -): +def _create_fn(name, args, body, fields, *, globals=None, locals=None): global _code_cache_total, _code_cache_miss - key = (name, len(fields), locals and frozenset(locals), *extra_keys) + key = (name, args, body) code = _code_cache.get(key) _code_cache_total += 1 if code is None: @@ -606,14 +595,10 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, # (instead of just concatenting the lists together). _init_params += ['*'] _init_params += [_symbol(fields.index(f)) for f in kw_only_fields] - extra_keys = ( - frozen, *(f._extra_keys() for f in fields), has_post_init, self_name - ) f = _create_fn('__init__', - [self_name] + _init_params, - body_lines, + (self_name, *_init_params), + tuple(body_lines), fields=fields, - extra_keys=extra_keys, locals=locals, globals=globals) annotations = {"return": None} @@ -640,10 +625,10 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, def _repr_fn(fields, globals): fn = _create_fn('__repr__', ('self',), - ['return self.__class__.__qualname__ + f"(' + + ('return self.__class__.__qualname__ + f"(' + ', '.join([f"{_symbol(i)}={{self.{_symbol(i)}!r}}" for i in range(len(fields))]) + - ')"'], + ')"',), fields=fields, globals=globals) return _recursive_repr(fn) @@ -684,9 +669,9 @@ def _cmp_fn(name, op, self_tuple, other_tuple, globals, fields): return _create_fn(name, ('self', 'other'), - [ 'if other.__class__ is self.__class__:', + ( 'if other.__class__ is self.__class__:', f' return {self_tuple}{op}{other_tuple}', - 'return NotImplemented'], + 'return NotImplemented'), fields=fields, globals=globals) @@ -695,7 +680,7 @@ def _hash_fn(fields, globals): self_tuple = _tuple_str('self', fields) return _create_fn('__hash__', ('self',), - [f'return hash({self_tuple})'], + (f'return hash({self_tuple})',), fields=fields, globals=globals) From 42e1df8f6e02d9b4f3ae902d0394702c9e489483 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 29 Dec 2022 01:46:33 -0800 Subject: [PATCH 11/16] Report code cache stats after testing --- Lib/test/test_dataclasses.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index a09f36c3dac1ce1..ae35b9ddb285fbf 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -4312,6 +4312,14 @@ def test_make_dataclass(self): self.assertTrue(fields(B)[0].kw_only) self.assertFalse(fields(B)[1].kw_only) +def report_code_cache_stats(): + misses = len(dataclasses._code_cache) + hits = dataclasses._code_cache_hits + success = hits / (hits + misses) + print(f"Code cache: {hits:,} hits, {misses:,} misses, {success:.0%} success rate.") if __name__ == '__main__': - unittest.main() + try: + unittest.main() + finally: + report_code_cache_stats() From 69fa6d51c834fa098a2d4fcfe4369d4ff5f10b31 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 29 Dec 2022 01:51:20 -0800 Subject: [PATCH 12/16] Use named symbols and number them later --- Lib/dataclasses.py | 116 +++++++++++++++++++-------------------------- 1 file changed, 50 insertions(+), 66 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 13b5617dc7cce13..eb2aef31a6f8856 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -382,10 +382,6 @@ def field(*, default=MISSING, default_factory=MISSING, init=True, repr=True, metadata, kw_only) -def _symbol(i): - return f"__field_{i}__" - - def _fields_in_init_order(fields): # Returns the fields as __init__ will output them. It returns 2 tuples: # the first for normal args, and the second for keyword args. @@ -394,17 +390,20 @@ def _fields_in_init_order(fields): tuple(f for f in fields if f.init and f.kw_only) ) +def _symbol(x): + return f"__field_{x}__" + def _tuple_str(obj_name, fields): # Return a string representing each field of obj_name as a tuple # member. So, if fields is ['x', 'y'] and obj_name is "self", - # return "(self.__fields_0__,self.__fields_1__)". + # return "(self.__field_0__,self.__field_1__)". # Special case for the 0-tuple. if not fields: return '()' # Note the trailing comma, needed if this turns out to be a 1-tuple. - return f'({",".join([f"{obj_name}.{_symbol(i)}" for i in range(len(fields))])},)' + return f'({",".join([f"{obj_name}.{_symbol(f.name)}" for f in fields])},)' # This function's logic is copied from "recursive_repr" function in @@ -428,62 +427,47 @@ def wrapper(self): return wrapper -def _create_code(name, args, body, *, globals=None, locals=None): - # Note that we may mutate locals. Callers beware! - # The only callers are internal to this module, so no +_code_cache = {} +_code_cache_hits = 0 + +def _create_fn(name, args, body, fields, *, globals=None, locals=None): + # Note that we mutate locals when exec() is called. Caller + # beware! The only callers are internal to this module, so no # worries about external callers. if locals is None: locals = {} args = ','.join(args) body = '\n'.join(f' {b}' for b in body) - - # Compute the text of the entire function. - txt = f' def {name}({args}):\n{body}' - - # Free variables in exec are resolved in the global namespace. - # The global namespace we have is user-provided, so we can't modify it for - # our purposes. So we put the things we need into locals and introduce a - # scope to allow the function we're creating to close over them. - local_vars = ', '.join(locals.keys()) - txt = f"def __create_fn__({local_vars}):\n{txt}\n return {name}" - ns = {} - exec(txt, globals, ns) - return ns['__create_fn__'](**locals).__code__ - - -_code_cache = {} -_code_cache_total = 0 -_code_cache_miss = 0 - - -def _create_fn(name, args, body, fields, *, globals=None, locals=None): - global _code_cache_total, _code_cache_miss - key = (name, args, body) - code = _code_cache.get(key) - _code_cache_total += 1 + source = f'def {name}({args}):\n{body}' + patches = {} + for i, f in enumerate(fields): + named_symbol = _symbol(f.name) + numbered_symbol = _symbol(i) + source = source.replace(named_symbol, numbered_symbol) + patches[numbered_symbol] = f.name + code = _code_cache.get(source) if code is None: - _code_cache_miss += 1 - code = _code_cache[key] = _create_code( - name, args, body, globals=globals, locals=locals - ) - patched = {_symbol(i): f.name for i, f in enumerate(fields)} - varnames = tuple(patched.get(name, name) for name in code.co_varnames) - names = tuple(patched.get(name, name) for name in code.co_names) - consts = list(code.co_consts) - for i, const in enumerate(code.co_consts): - if isinstance(const, str): - for pair in patched.items(): - const = const.replace(*pair) - consts[i] = const - elif isinstance(const, tuple): - consts[i] = tuple(patched.get(c, c) for c in const) - consts = tuple(consts) + # Free variables in exec are resolved in the global namespace. + # The global namespace we have is user-provided, so we can't modify it for + # our purposes. So we put the things we need into locals and introduce a + # scope to allow the function we're creating to close over them. + local_vars = ', '.join(locals.keys()) + t = f"def __create_fn__({local_vars}):\n {source}\n return {name}" + ns = {} + exec(t, globals, ns) + code = _code_cache[source] = ns['__create_fn__'](**locals).__code__ + else: + global _code_cache_hits + _code_cache_hits += 1 + consts = tuple(patches.get(name, name) for name in code.co_consts) + names = tuple(patches.get(name, name) for name in code.co_names) + varnames = tuple(patches.get(name, name) for name in code.co_varnames) closure = tuple(CellType(locals[freevar]) for freevar in code.co_freevars) return FunctionType( code=code.replace( - co_varnames=varnames, - co_names=names, co_consts=consts, + co_names=names, + co_varnames=varnames, ), globals=globals or {}, closure=closure, @@ -506,7 +490,6 @@ def _field_init(f, frozen, globals, self_name, slots, i): # Return the text of the line in the body of __init__ that will # initialize this field. - field_name = _symbol(i) default_name = f'_dflt_{i}' if f.default_factory is not MISSING: if f.init: @@ -514,8 +497,8 @@ def _field_init(f, frozen, globals, self_name, slots, i): # given, use it. If not, call the factory. globals[default_name] = f.default_factory value = (f'{default_name}() ' - f'if {field_name} is _HAS_DEFAULT_FACTORY ' - f'else {field_name}') + f'if {_symbol(f.name)} is _HAS_DEFAULT_FACTORY ' + f'else {_symbol(f.name)}') else: # This is a field that's not in the __init__ params, but # has a default factory function. It needs to be @@ -538,10 +521,10 @@ def _field_init(f, frozen, globals, self_name, slots, i): if f.init: if f.default is MISSING: # There's no default, just do an assignment. - value = field_name + value = _symbol(f.name) elif f.default is not MISSING: globals[default_name] = f.default - value = field_name + value = _symbol(f.name) else: # If the class has slots, then initialize this field. if slots and f.default is not MISSING: @@ -560,7 +543,7 @@ def _field_init(f, frozen, globals, self_name, slots, i): return None # Now, actually generate the field assignment. - return _field_assign(frozen, field_name, value, self_name) + return _field_assign(frozen, _symbol(f.name), value, self_name) def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, self_name, globals, slots): @@ -598,7 +581,7 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, # Does this class have a post-init function? if has_post_init: - params_str = ','.join(_symbol(i) for i, f in enumerate(fields) + params_str = ','.join(_symbol(f.name) for f in fields if f._field_type is _FIELD_INITVAR) body_lines.append(f'{self_name}.{_POST_INIT_NAME}({params_str})') @@ -606,13 +589,13 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, if not body_lines: body_lines = ['pass'] - _init_params = [_symbol(fields.index(f)) for f in std_fields] + _init_params = [_symbol(f.name) for f in std_fields] if kw_only_fields: # Add the keyword-only args. Because the * can only be added if # there's at least one keyword-only arg, there needs to be a test here # (instead of just concatenting the lists together). _init_params += ['*'] - _init_params += [_symbol(fields.index(f)) for f in kw_only_fields] + _init_params += [_symbol(f.name) for f in kw_only_fields] f = _create_fn('__init__', (self_name, *_init_params), tuple(body_lines), @@ -641,11 +624,12 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, def _repr_fn(fields, globals): + # "{'__field_foo__'!s}={self.__field_foo__!r}" + attributes = [f"{{{_symbol(f.name)!r}!s}}={{self.{_symbol(f.name)}!r}}" for f in fields] fn = _create_fn('__repr__', ('self',), ('return self.__class__.__qualname__ + f"(' + - ', '.join([f"{_symbol(i)}={{self.{_symbol(i)}!r}}" - for i in range(len(fields))]) + + ', '.join(attributes) + ')"',), fields=fields, globals=globals) @@ -656,13 +640,13 @@ def _frozen_get_del_attr(cls, fields, globals): locals = {'cls': cls, 'FrozenInstanceError': FrozenInstanceError} if fields: - fields_str = '(' + ','.join(repr(_symbol(i)) for i in range(len(fields))) + ',)' + fields_str = '[' + ','.join(repr(_symbol(f.name)) for f in fields) + ']' else: # Special case for the zero-length tuple. fields_str = '()' return (_create_fn('__setattr__', ('self', 'name', 'value'), - (f'if type(self) is cls or name in {fields_str}:', + (f'if type(self) is cls or name in tuple({fields_str}):', ' raise FrozenInstanceError(f"cannot assign to field {name!r}")', f'super(cls, self).__setattr__(name, value)'), fields=fields, @@ -670,7 +654,7 @@ def _frozen_get_del_attr(cls, fields, globals): globals=globals), _create_fn('__delattr__', ('self', 'name'), - (f'if type(self) is cls or name in {fields_str}:', + (f'if type(self) is cls or name in tuple({fields_str}):', ' raise FrozenInstanceError(f"cannot delete field {name!r}")', f'super(cls, self).__delattr__(name)'), fields=fields, From 66070d2a54c5229c688ecd3e894dbe792fd50a95 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 29 Dec 2022 10:30:27 -0800 Subject: [PATCH 13/16] Discover fields automatically in _create_fn --- Lib/dataclasses.py | 114 ++++++++++++++++++++++++--------------------- 1 file changed, 60 insertions(+), 54 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index eb2aef31a6f8856..f42da9baa8ed44b 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -262,6 +262,7 @@ class Field: 'metadata', 'kw_only', '_field_type', # Private: not to be used by user code. + '_placeholder', ) def __init__(self, default, default_factory, init, repr, hash, compare, @@ -279,6 +280,7 @@ def __init__(self, default, default_factory, init, repr, hash, compare, types.MappingProxyType(metadata)) self.kw_only = kw_only self._field_type = None + self._placeholder = None def __repr__(self): return ('Field(' @@ -390,20 +392,16 @@ def _fields_in_init_order(fields): tuple(f for f in fields if f.init and f.kw_only) ) -def _symbol(x): - return f"__field_{x}__" - - def _tuple_str(obj_name, fields): # Return a string representing each field of obj_name as a tuple # member. So, if fields is ['x', 'y'] and obj_name is "self", - # return "(self.__field_0__,self.__field_1__)". + # return "(self.$x$,self.$y$)". # Special case for the 0-tuple. if not fields: return '()' # Note the trailing comma, needed if this turns out to be a 1-tuple. - return f'({",".join([f"{obj_name}.{_symbol(f.name)}" for f in fields])},)' + return f'({",".join([f"{obj_name}.{f._placeholder}" for f in fields])},)' # This function's logic is copied from "recursive_repr" function in @@ -430,36 +428,53 @@ def wrapper(self): _code_cache = {} _code_cache_hits = 0 -def _create_fn(name, args, body, fields, *, globals=None, locals=None): - # Note that we mutate locals when exec() is called. Caller - # beware! The only callers are internal to this module, so no +def _create_fn(name, args, body, *, globals=None, locals=None): + # Note that we may mutate locals. Callers beware! + # The only callers are internal to this module, so no # worries about external callers. if locals is None: locals = {} args = ','.join(args) body = '\n'.join(f' {b}' for b in body) - source = f'def {name}({args}):\n{body}' + txt = f'def {name}({args}):\n{body}' + # Turn every illegal named placeholder ("$foo$", "$bar$", ...) in the + # source into a legal numbered identifier ("__field_0__", "__field_1__", + # ...). At the same time, build a mapping of these new identifiers to their + # final field names ({"__field_0__": "foo", "__field_1__": "bar", ...}): + # patches = {} + # for i, f in enumerate(fields): + # named_placeholder = f._placeholder + # numbered_identifier = f"__field_{i}__" + # txt = txt.replace(named_placeholder, numbered_identifier) + # patches[numbered_identifier] = f.name patches = {} - for i, f in enumerate(fields): - named_symbol = _symbol(f.name) - numbered_symbol = _symbol(i) - source = source.replace(named_symbol, numbered_symbol) - patches[numbered_symbol] = f.name - code = _code_cache.get(source) + for named_placeholder in dict.fromkeys(re.findall(r"\$\w+\$", txt)): + numbered_identifier = f"__field_{len(patches)}__" + field_name = named_placeholder[1:-1] + txt = txt.replace(named_placeholder, numbered_identifier) + patches[numbered_identifier] = field_name + key = txt + code = _code_cache.get(key) if code is None: # Free variables in exec are resolved in the global namespace. # The global namespace we have is user-provided, so we can't modify it for # our purposes. So we put the things we need into locals and introduce a # scope to allow the function we're creating to close over them. local_vars = ', '.join(locals.keys()) - t = f"def __create_fn__({local_vars}):\n {source}\n return {name}" + txt = f"def __create_fn__({local_vars}):\n {txt}\n return {name}" ns = {} - exec(t, globals, ns) - code = _code_cache[source] = ns['__create_fn__'](**locals).__code__ + exec(txt, globals, ns) + code = _code_cache[key] = ns['__create_fn__'](**locals).__code__ else: global _code_cache_hits _code_cache_hits += 1 - consts = tuple(patches.get(name, name) for name in code.co_consts) + consts = [] + for const in code.co_consts: + if isinstance(const, str): + for numbered_symbol, field_name in patches.items(): + const = const.replace(numbered_symbol, field_name) + consts.append(const) + consts = tuple(consts) names = tuple(patches.get(name, name) for name in code.co_names) varnames = tuple(patches.get(name, name) for name in code.co_varnames) closure = tuple(CellType(locals[freevar]) for freevar in code.co_freevars) @@ -497,8 +512,8 @@ def _field_init(f, frozen, globals, self_name, slots, i): # given, use it. If not, call the factory. globals[default_name] = f.default_factory value = (f'{default_name}() ' - f'if {_symbol(f.name)} is _HAS_DEFAULT_FACTORY ' - f'else {_symbol(f.name)}') + f'if {f._placeholder} is _HAS_DEFAULT_FACTORY ' + f'else {f._placeholder}') else: # This is a field that's not in the __init__ params, but # has a default factory function. It needs to be @@ -521,10 +536,10 @@ def _field_init(f, frozen, globals, self_name, slots, i): if f.init: if f.default is MISSING: # There's no default, just do an assignment. - value = _symbol(f.name) + value = f._placeholder elif f.default is not MISSING: globals[default_name] = f.default - value = _symbol(f.name) + value = f._placeholder else: # If the class has slots, then initialize this field. if slots and f.default is not MISSING: @@ -543,7 +558,7 @@ def _field_init(f, frozen, globals, self_name, slots, i): return None # Now, actually generate the field assignment. - return _field_assign(frozen, _symbol(f.name), value, self_name) + return _field_assign(frozen, f._placeholder, value, self_name) def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, self_name, globals, slots): @@ -581,7 +596,7 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, # Does this class have a post-init function? if has_post_init: - params_str = ','.join(_symbol(f.name) for f in fields + params_str = ','.join(f._placeholder for f in fields if f._field_type is _FIELD_INITVAR) body_lines.append(f'{self_name}.{_POST_INIT_NAME}({params_str})') @@ -589,17 +604,16 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, if not body_lines: body_lines = ['pass'] - _init_params = [_symbol(f.name) for f in std_fields] + _init_params = [f._placeholder for f in std_fields] if kw_only_fields: # Add the keyword-only args. Because the * can only be added if # there's at least one keyword-only arg, there needs to be a test here # (instead of just concatenting the lists together). _init_params += ['*'] - _init_params += [_symbol(f.name) for f in kw_only_fields] + _init_params += [f._placeholder for f in kw_only_fields] f = _create_fn('__init__', (self_name, *_init_params), tuple(body_lines), - fields=fields, locals=locals, globals=globals) annotations = {"return": None} @@ -624,14 +638,12 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, def _repr_fn(fields, globals): - # "{'__field_foo__'!s}={self.__field_foo__!r}" - attributes = [f"{{{_symbol(f.name)!r}!s}}={{self.{_symbol(f.name)}!r}}" for f in fields] fn = _create_fn('__repr__', ('self',), - ('return self.__class__.__qualname__ + f"(' + - ', '.join(attributes) + - ')"',), - fields=fields, + ['return self.__class__.__qualname__ + f"(' + + ', '.join([f"{f._placeholder}={{self.{f._placeholder}!r}}" + for f in fields]) + + ')"'], globals=globals) return _recursive_repr(fn) @@ -639,42 +651,35 @@ def _repr_fn(fields, globals): def _frozen_get_del_attr(cls, fields, globals): locals = {'cls': cls, 'FrozenInstanceError': FrozenInstanceError} - if fields: - fields_str = '[' + ','.join(repr(_symbol(f.name)) for f in fields) + ']' - else: - # Special case for the zero-length tuple. - fields_str = '()' + fields_str = ' '.join(f._placeholder for f in fields) return (_create_fn('__setattr__', ('self', 'name', 'value'), - (f'if type(self) is cls or name in tuple({fields_str}):', + (f'if type(self) is cls or name in {fields_str!r}:', ' raise FrozenInstanceError(f"cannot assign to field {name!r}")', f'super(cls, self).__setattr__(name, value)'), - fields=fields, locals=locals, globals=globals), _create_fn('__delattr__', ('self', 'name'), - (f'if type(self) is cls or name in tuple({fields_str}):', + (f'if type(self) is cls or name in {fields_str!r}:', ' raise FrozenInstanceError(f"cannot delete field {name!r}")', f'super(cls, self).__delattr__(name)'), - fields=fields, locals=locals, globals=globals), ) -def _cmp_fn(name, op, self_tuple, other_tuple, globals, fields): +def _cmp_fn(name, op, self_tuple, other_tuple, globals): # Create a comparison function. If the fields in the object are # named 'x' and 'y', then self_tuple is the string - # '(self.x,self.y)' and other_tuple is the string - # '(other.x,other.y)'. + # '(self.$x$,self.$y$)' and other_tuple is the string + # '(other.$x$,other.$y$)'. return _create_fn(name, ('self', 'other'), - ( 'if other.__class__ is self.__class__:', + [ 'if other.__class__ is self.__class__:', f' return {self_tuple}{op}{other_tuple}', - 'return NotImplemented'), - fields=fields, + 'return NotImplemented'], globals=globals) @@ -682,8 +687,7 @@ def _hash_fn(fields, globals): self_tuple = _tuple_str('self', fields) return _create_fn('__hash__', ('self',), - (f'return hash({self_tuple})',), - fields=fields, + [f'return hash({self_tuple})',], globals=globals) @@ -785,6 +789,8 @@ def _get_field(cls, a_name, a_type, default_kw_only): f.name = a_name f.type = a_type + f._placeholder = f"${a_name}$" + # Assume it's a normal field until proven otherwise. We're next # going to decide if it's a ClassVar or InitVar, everything else # is just a normal field. @@ -1098,7 +1104,7 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, _set_new_attribute(cls, '__eq__', _cmp_fn('__eq__', '==', self_tuple, other_tuple, - fields=flds, globals=globals)) + globals=globals)) if order: # Create and set the ordering methods. @@ -1112,7 +1118,7 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, ]: if _set_new_attribute(cls, name, _cmp_fn(name, op, self_tuple, other_tuple, - fields=flds, globals=globals)): + globals=globals)): raise TypeError(f'Cannot overwrite attribute {name} ' f'in class {cls.__name__}. Consider using ' 'functools.total_ordering') From b0f795f0453b9300f25072317b712a123a8ea775 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 Jan 2023 06:38:00 -0800 Subject: [PATCH 14/16] Refactor how code objects are constructed --- Lib/dataclasses.py | 195 +++++++++++++++++++++++++---------- Lib/test/test_dataclasses.py | 9 +- 2 files changed, 146 insertions(+), 58 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index b960fc81dfde0dd..a31b557781989da 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -223,6 +223,9 @@ def __repr__(self): # https://bugs.python.org/issue33453 for details. _MODULE_IDENTIFIER_RE = re.compile(r'^(?:\s*(\w+)\s*\.)?\s*(\w+)') +_PLACEHOLDER_PREFIX = "_field_" +_PLACEHOLDER_RE = re.compile(rf"({_PLACEHOLDER_PREFIX}\w+)") + # This function's logic is copied from "recursive_repr" function in # reprlib module to avoid dependency. def _recursive_repr(user_function): @@ -416,7 +419,7 @@ def _fields_in_init_order(fields): def _tuple_str(obj_name, fields): # Return a string representing each field of obj_name as a tuple # member. So, if fields is ['x', 'y'] and obj_name is "self", - # return "(self.$x$,self.$y$)". + # return "(self._field_x,self._field_y)". # Special case for the 0-tuple. if not fields: @@ -436,59 +439,124 @@ def _create_fn(name, args, body, *, globals=None, locals=None): locals = {} args = ','.join(args) body = '\n'.join(f' {b}' for b in body) - txt = f'def {name}({args}):\n{body}' - # Turn every illegal named placeholder ("$foo$", "$bar$", ...) in the - # source into a legal numbered identifier ("__field_0__", "__field_1__", - # ...). At the same time, build a mapping of these new identifiers to their - # final field names ({"__field_0__": "foo", "__field_1__": "bar", ...}): - # patches = {} - # for i, f in enumerate(fields): - # named_placeholder = f._placeholder - # numbered_identifier = f"__field_{i}__" - # txt = txt.replace(named_placeholder, numbered_identifier) - # patches[numbered_identifier] = f.name - patches = {} - for named_placeholder in dict.fromkeys(re.findall(r"\$\w+\$", txt)): - numbered_identifier = f"__field_{len(patches)}__" - field_name = named_placeholder[1:-1] - txt = txt.replace(named_placeholder, numbered_identifier) - patches[numbered_identifier] = field_name - key = txt - code = _code_cache.get(key) - if code is None: - # Free variables in exec are resolved in the global namespace. - # The global namespace we have is user-provided, so we can't modify it for - # our purposes. So we put the things we need into locals and introduce a - # scope to allow the function we're creating to close over them. - local_vars = ', '.join(locals.keys()) - txt = f"def __create_fn__({local_vars}):\n {txt}\n return {name}" - ns = {} - exec(txt, globals, ns) - code = _code_cache[key] = ns['__create_fn__'](**locals).__code__ - else: + + # Compute the text of the entire function. + txt = f' def {name}({args}):\n{body}' + + # exec is *really* slow, so we want to avoid using it whenever possible. + # Turns out, if we already have a code object that is the exact same as the + # one we want to create (except for the actual names of the fields), it's + # 2-3x faster to just copy the other code object, fix up the names, and + # create a function using the types.FunctionType constructor. + # + # In order for this to work, we need to be able to detect the names of the + # fields from the source. For this reason, it's *crucial* that anything + # building source strings uses field._placeholder, *not* field.name! + # + # This trick requires two passes: + # - Strip out the field names from the source text, and replace them with + # something else (like numbers). This is used to search for (or create) + # cached code with the same structure. This happens in _extract_fields. + # - Make a copy of the cached code object, and fix up all of the stripped + # names in the consts, names, and varnames. This happens in _patch_fields. + # + # This is surprisingly effective, since we generate lots of functions that + # differ *only* in the names of their fields (we avoid something like 95% of + # the exec calls in test_dataclasses.py). Credit for this neat idea goes to + # https://github.com/dabeaz/dataklasses. + + patches, key = _extract_fields(txt) + if key in _code_cache: global _code_cache_hits _code_cache_hits += 1 + code = _code_cache[key] + else: + code = _code_cache[key] = _create_code(name, key, globals, locals) + # Build the closure: + closure = [] + for freevar in code.co_freevars: + closure.append(CellType(locals[freevar])) + # Build the function: + return FunctionType( + code=_patch_fields(patches, code), + globals=globals or {}, + closure=tuple(closure), + ) + + +def _create_code(name, txt, globals, locals): + # Free variables in exec are resolved in the global namespace. + # The global namespace we have is user-provided, so we can't modify it for + # our purposes. So we put the things we need into locals and introduce a + # scope to allow the function we're creating to close over them. + local_vars = ', '.join(locals.keys()) + txt = f"def __create_fn__({local_vars}):\n{txt}\n return {name}" + ns = {} + exec(txt, globals, ns) + return ns['__create_fn__'](**locals).__code__ + + +def _extract_fields(source): + """ + Replace every named placeholder (like "_field_spam") in the source with a + numbered placeholder (like "_field_1"). At the same time, build a mapping + of these new numbered placeholders to their actual field names. + + Return the mapping and the patched source. + """ + named_to_numbered = {} # {"_field_spam": "_field_1", ...} + numbered_to_field = {} # {"_field_1": "spam", ...} + parts = _PLACEHOLDER_RE.split(source) + # Every other part is a named placeholder. Those are all that we care about: + for i in range(1, len(parts), 2): + named_placeholder = parts[i] + if named_placeholder in named_to_numbered: + parts[i] = named_to_numbered[named_placeholder] + else: + # Using i doesn't give us sequential field numbers, but that's okay: + parts[i] = numbered_placeholder = f"{_PLACEHOLDER_PREFIX}{i}" + field_name = named_placeholder.removeprefix(_PLACEHOLDER_PREFIX) + named_to_numbered[named_placeholder] = numbered_placeholder + numbered_to_field[numbered_placeholder] = field_name + return numbered_to_field, "".join(parts) + + +def _patch_fields(patches, code): + """Copy the given code with its consts, names, and varnames patched.""" consts = [] for const in code.co_consts: - if isinstance(const, str): - for numbered_symbol, field_name in patches.items(): - const = const.replace(numbered_symbol, field_name) + match code.co_name, const: + case "__init__", str() if const in patches: + # const is a field name: + const = patches[const] + case "__repr__", str(): + # const may be a string *containing* field names: + unpatched = _PLACEHOLDER_RE.split(const) + patched = _apply_patches(patches, unpatched) + const = "".join(patched) + case "__delattr__" | "__setattr__", tuple(): + # const may be a tuple *containing* field names: + patched = _apply_patches(patches, const) + const = tuple(patched) consts.append(const) - consts = tuple(consts) - names = tuple(patches.get(name, name) for name in code.co_names) - varnames = tuple(patches.get(name, name) for name in code.co_varnames) - closure = tuple(CellType(locals[freevar]) for freevar in code.co_freevars) - return FunctionType( - code=code.replace( - co_consts=consts, - co_names=names, - co_varnames=varnames, - ), - globals=globals or {}, - closure=closure, + names = _apply_patches(patches, code.co_names) + varnames = _apply_patches(patches, code.co_varnames) + return code.replace( + co_consts=tuple(consts), + co_names=tuple(names), + co_varnames=tuple(varnames), ) +def _apply_patches(patches, unpatched): + patched = [] + for item in unpatched: + if item in patches: + item = patches[item] + patched.append(item) + return patched + + def _field_assign(frozen, name, value, self_name): # If we're a frozen class, then assign to our fields in __init__ # via object.__setattr__. Otherwise, just use a simple @@ -560,6 +628,7 @@ def _field_init(f, frozen, globals, self_name, slots, i): # Now, actually generate the field assignment. return _field_assign(frozen, f._placeholder, value, self_name) + def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, self_name, globals, slots): # fields contains both real fields and InitVar pseudo-fields. @@ -616,6 +685,15 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, tuple(body_lines), locals=locals, globals=globals) + _add_init_dunders(f, std_fields, kw_only_fields) + return f + + +def _add_init_dunders(init, std_fields, kw_only_fields): + """ + Add __annotations__, __defaults__, and __kwdefaults__ to an __init__ + function. + """ annotations = {"return": None} defaults = [] kwdefaults = {} @@ -631,10 +709,9 @@ def _init_fn(fields, std_fields, kw_only_fields, frozen, has_post_init, kwdefaults[field.name] = field.default elif field.default_factory is not MISSING: kwdefaults[field.name] = _HAS_DEFAULT_FACTORY - f.__annotations__ = annotations - f.__defaults__ = tuple(defaults) or None - f.__kwdefaults__ = kwdefaults or None - return f + init.__annotations__ = annotations + init.__defaults__ = tuple(defaults) or None + init.__kwdefaults__ = kwdefaults or None def _repr_fn(fields, globals): @@ -651,17 +728,21 @@ def _repr_fn(fields, globals): def _frozen_get_del_attr(cls, fields, globals): locals = {'cls': cls, 'FrozenInstanceError': FrozenInstanceError} - fields_str = ' '.join(f._placeholder for f in fields) + if fields: + fields_str = '(' + ','.join(repr(f._placeholder) for f in fields) + ',)' + else: + # Special case for the zero-length tuple. + fields_str = '()' return (_create_fn('__setattr__', ('self', 'name', 'value'), - (f'if type(self) is cls or name in {fields_str!r}:', + (f'if type(self) is cls or name in {fields_str}:', ' raise FrozenInstanceError(f"cannot assign to field {name!r}")', f'super(cls, self).__setattr__(name, value)'), locals=locals, globals=globals), _create_fn('__delattr__', ('self', 'name'), - (f'if type(self) is cls or name in {fields_str!r}:', + (f'if type(self) is cls or name in {fields_str}:', ' raise FrozenInstanceError(f"cannot delete field {name!r}")', f'super(cls, self).__delattr__(name)'), locals=locals, @@ -672,8 +753,8 @@ def _frozen_get_del_attr(cls, fields, globals): def _cmp_fn(name, op, self_tuple, other_tuple, globals): # Create a comparison function. If the fields in the object are # named 'x' and 'y', then self_tuple is the string - # '(self.$x$,self.$y$)' and other_tuple is the string - # '(other.$x$,other.$y$)'. + # '(self._field_x,self._field_y)' and other_tuple is the string + # '(other._field_x,other._field_y)'. return _create_fn(name, ('self', 'other'), @@ -687,7 +768,7 @@ def _hash_fn(fields, globals): self_tuple = _tuple_str('self', fields) return _create_fn('__hash__', ('self',), - [f'return hash({self_tuple})',], + [f'return hash({self_tuple})'], globals=globals) @@ -789,7 +870,7 @@ def _get_field(cls, a_name, a_type, default_kw_only): f.name = a_name f.type = a_type - f._placeholder = f"${a_name}$" + f._placeholder = f"{_PLACEHOLDER_PREFIX}{a_name}" # Assume it's a normal field until proven otherwise. We're next # going to decide if it's a ClassVar or InitVar, everything else diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index 73ea44a5e35e1b3..d82b89f01382f48 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -4330,11 +4330,18 @@ def test_make_dataclass(self): self.assertTrue(fields(B)[0].kw_only) self.assertFalse(fields(B)[1].kw_only) + def report_code_cache_stats(): misses = len(dataclasses._code_cache) hits = dataclasses._code_cache_hits success = hits / (hits + misses) - print(f"Code cache: {hits:,} hits, {misses:,} misses, {success:.0%} success rate.") + print( + f"Code cache:", + f"{hits:,} hits,", + f"{misses:,} misses,", + f"{success:.0%} success rate.", + ) + if __name__ == '__main__': try: From 41dacab2004ca80c2ee6255c9bf4183bcb9dc90e Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 Jan 2023 07:43:39 -0800 Subject: [PATCH 15/16] fixup --- Lib/dataclasses.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index a31b557781989da..dcb3ee1b6079ece 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -446,20 +446,22 @@ def _create_fn(name, args, body, *, globals=None, locals=None): # exec is *really* slow, so we want to avoid using it whenever possible. # Turns out, if we already have a code object that is the exact same as the # one we want to create (except for the actual names of the fields), it's - # 2-3x faster to just copy the other code object, fix up the names, and + # twice as fast to just copy the other code object, fix up the names, and # create a function using the types.FunctionType constructor. - # + # In order for this to work, we need to be able to detect the names of the # fields from the source. For this reason, it's *crucial* that anything # building source strings uses field._placeholder, *not* field.name! - # + # This trick requires two passes: + # - Strip out the field names from the source text, and replace them with # something else (like numbers). This is used to search for (or create) # cached code with the same structure. This happens in _extract_fields. + # - Make a copy of the cached code object, and fix up all of the stripped # names in the consts, names, and varnames. This happens in _patch_fields. - # + # This is surprisingly effective, since we generate lots of functions that # differ *only* in the names of their fields (we avoid something like 95% of # the exec calls in test_dataclasses.py). Credit for this neat idea goes to From fecf58f27eaebcbb7d55601757bdc7e28e5ffe1a Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 10 Jan 2023 07:57:15 -0800 Subject: [PATCH 16/16] blurb add --- .../Library/2023-01-10-07-56-42.gh-issue-100930.EPFqEr.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2023-01-10-07-56-42.gh-issue-100930.EPFqEr.rst diff --git a/Misc/NEWS.d/next/Library/2023-01-10-07-56-42.gh-issue-100930.EPFqEr.rst b/Misc/NEWS.d/next/Library/2023-01-10-07-56-42.gh-issue-100930.EPFqEr.rst new file mode 100644 index 000000000000000..71a9a47f25a7fc8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-01-10-07-56-42.gh-issue-100930.EPFqEr.rst @@ -0,0 +1,4 @@ +Double the speed of most :func:`dataclasses.dataclass` definitions by +reusing code objects from methods of similar dataclasses. This idea was +originally implemented by David Beazley in his `dataklasses proof-of-concept +`_.