Skip to content

Commit e1f6d6b

Browse files
authored
[mypyc] Avoid cyclic reference in nested functions (#16268)
Mypyc used to always put nested functions into the environment object, which results in cyclic references, since the function object contains a reference to the environment. Now we only do this if the body of a nested function refers to a nested function (e.g. due to a recursive call). This means that in the majority of cases we can avoid the cyclic reference. This speeds up self check by an impressive 7%. I'm not sure exactly why the impact is so big, but spending less time in the cyclic garbage collector is probably a big part.
1 parent 838a1d4 commit e1f6d6b

File tree

6 files changed

+305
-346
lines changed

6 files changed

+305
-346
lines changed

mypyc/irbuild/builder.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,11 @@ def non_function_scope(self) -> bool:
502502
# Currently the stack always has at least two items: dummy and top-level.
503503
return len(self.fn_infos) <= 2
504504

505+
def top_level_fn_info(self) -> FuncInfo | None:
506+
if self.non_function_scope():
507+
return None
508+
return self.fn_infos[2]
509+
505510
def init_final_static(
506511
self,
507512
lvalue: Lvalue,

mypyc/irbuild/context.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ def __init__(
2222
contains_nested: bool = False,
2323
is_decorated: bool = False,
2424
in_non_ext: bool = False,
25+
add_nested_funcs_to_env: bool = False,
2526
) -> None:
2627
self.fitem = fitem
2728
self.name = name
@@ -47,6 +48,7 @@ def __init__(
4748
self.contains_nested = contains_nested
4849
self.is_decorated = is_decorated
4950
self.in_non_ext = in_non_ext
51+
self.add_nested_funcs_to_env = add_nested_funcs_to_env
5052

5153
# TODO: add field for ret_type: RType = none_rprimitive
5254

mypyc/irbuild/env_class.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ def load_env_registers(builder: IRBuilder) -> None:
107107
load_outer_envs(builder, fn_info.callable_class)
108108
# If this is a FuncDef, then make sure to load the FuncDef into its own environment
109109
# class so that the function can be called recursively.
110-
if isinstance(fitem, FuncDef):
110+
if isinstance(fitem, FuncDef) and fn_info.add_nested_funcs_to_env:
111111
setup_func_for_recursive_call(builder, fitem, fn_info.callable_class)
112112

113113

mypyc/irbuild/function.py

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
ArgKind,
2020
ClassDef,
2121
Decorator,
22+
FuncBase,
2223
FuncDef,
2324
FuncItem,
2425
LambdaExpr,
@@ -222,6 +223,7 @@ def c() -> None:
222223
is_decorated = fitem in builder.fdefs_to_decorators
223224
is_singledispatch = fitem in builder.singledispatch_impls
224225
in_non_ext = False
226+
add_nested_funcs_to_env = has_nested_func_self_reference(builder, fitem)
225227
class_name = None
226228
if cdef:
227229
ir = builder.mapper.type_to_ir[cdef.info]
@@ -234,14 +236,15 @@ def c() -> None:
234236
func_name = name
235237
builder.enter(
236238
FuncInfo(
237-
fitem,
238-
func_name,
239-
class_name,
240-
gen_func_ns(builder),
241-
is_nested,
242-
contains_nested,
243-
is_decorated,
244-
in_non_ext,
239+
fitem=fitem,
240+
name=func_name,
241+
class_name=class_name,
242+
namespace=gen_func_ns(builder),
243+
is_nested=is_nested,
244+
contains_nested=contains_nested,
245+
is_decorated=is_decorated,
246+
in_non_ext=in_non_ext,
247+
add_nested_funcs_to_env=add_nested_funcs_to_env,
245248
)
246249
)
247250

@@ -267,7 +270,13 @@ def c() -> None:
267270
builder.enter(fn_info)
268271
setup_env_for_generator_class(builder)
269272
load_outer_envs(builder, builder.fn_info.generator_class)
270-
if builder.fn_info.is_nested and isinstance(fitem, FuncDef):
273+
top_level = builder.top_level_fn_info()
274+
if (
275+
builder.fn_info.is_nested
276+
and isinstance(fitem, FuncDef)
277+
and top_level
278+
and top_level.add_nested_funcs_to_env
279+
):
271280
setup_func_for_recursive_call(builder, fitem, builder.fn_info.generator_class)
272281
create_switch_for_generator_class(builder)
273282
add_raise_exception_blocks_to_generator_class(builder, fitem.line)
@@ -344,6 +353,20 @@ def c() -> None:
344353
return func_ir, func_reg
345354

346355

356+
def has_nested_func_self_reference(builder: IRBuilder, fitem: FuncItem) -> bool:
357+
"""Does a nested function contain a self-reference in its body?
358+
359+
If a nested function only has references in the surrounding function,
360+
we don't need to add it to the environment.
361+
"""
362+
if any(isinstance(sym, FuncBase) for sym in builder.free_variables.get(fitem, set())):
363+
return True
364+
return any(
365+
has_nested_func_self_reference(builder, nested)
366+
for nested in builder.encapsulating_funcs.get(fitem, [])
367+
)
368+
369+
347370
def gen_func_ir(
348371
builder: IRBuilder,
349372
args: list[Register],
@@ -768,7 +791,7 @@ def get_func_target(builder: IRBuilder, fdef: FuncDef) -> AssignmentTarget:
768791
# Get the target associated with the previously defined FuncDef.
769792
return builder.lookup(fdef.original_def)
770793

771-
if builder.fn_info.is_generator or builder.fn_info.contains_nested:
794+
if builder.fn_info.is_generator or builder.fn_info.add_nested_funcs_to_env:
772795
return builder.lookup(fdef)
773796

774797
return builder.add_local_reg(fdef, object_rprimitive)

0 commit comments

Comments
 (0)