[mypyc] Detect always defined attributes #12600

JukkaL · 2022-04-16T16:41:50Z

Use static analysis to find attributes that are always defined. Always defined
attributes don't require checks on each access. This makes them faster and
also reduces code size.

Attributes defined in the class body and assigned to in all code paths in
__init__ are always defined. We need to know all subclasses statically
to determine whether __init__ always defines an attribute in every case,
including in subclasses.

The analysis looks at __init__ methods and supports limited inter-procedural
analysis over super().__init__(...) calls. Otherwise we rely on intra-procedural
analysis to keep the analysis fast.

As a side effect, __init__ will now always be called when constructing an object.
This means that copy.copy (and others like it) won't be supported for native
classes unless __init__ can be called without arguments.

mypyc/analysis/attrdefined.py has more details about the algorithm in
docstrings.

Performance impact to selected benchmarks (with clang):

richards +28%
deltablue +10%
hexiom +1%

The richards result is probably an outlier.

This will also significantly help with native integers (mypyc/mypyc#837, as tracking
undefined values would otherwise require extra memory use.

Closes mypyc/mypyc#836.

This way we can more easily implement data flow analyses that don't range over Values.

Improve analysis and add tests Use always defined attributes to simplify get attr operations Support conditionally defined always defined attributes Update docstring Better handle conditionally defined attributes Update docstring Comment updates Collect initializers [WIP] Faster initialization of always defined attributes Simplify generated code a little [WIP] Simplify code generated for set attr Tests are still failing! Update assert Fix properties WIP update wrapper functions Fix after rebase Attributes that are __deletable__ are never always defiend Rewrite always defined analysis to default to disallow This is safer that defaulting to allow. Allow construction of collections Improve always defined analysis Support True and False Support reference to module-level final attribute Rename mypyc.analysis.defined to mypyc.irbuild.defined Update docstring Fix type check Fix lint WIP new implementation over IR

If we know that some helper won't arbitrarily mutate heap, etc., we can perform some additional optimizations. I'm going to use this for detecting always defined attributes, but this could also be used for common subexpression elimination and other things. Also manually tag some functions that won't run arbitrary code. Many of these are actually not quite correct. I think that anything that may allocate or free memory is actually able to run arbitrary code through user-defined `__del__` methods, possibly called by the cyclic garbage collector. We optimistically ignore this, since `__del__` methods that do weird stuff seem very unlikely (and are honestly asking for trouble). If we are totally conservative, there is very little we can assume about side effects. Here's is a simple example where this can be useful: ``` @Final class C: def __init__(self) -> None" self.a: list[int] = [] self.b = 1 ``` If we can assume that `[]` doesn't call arbitrary code that might read `self.b`, we can infer that both `a` and `b` attributes are always defined, and we don't need to check whether they are defined on each get or set operation.

JukkaL · 2022-04-16T17:04:37Z

In particular I'd love to hear ideas about tricky edge cases that the implementation might not support correctly. I've fixed a bunch, but there may well be others that I've overlooked. This turned out to be a fairly complex problem.

JukkaL · 2022-04-16T17:06:54Z

The analysis isn't super helpful with mypy, since we tend to call other methods in __init__ methods, defeating the analysis. My intention is to eventually refactor some of the more widely used mypy implementation classes to better support always defined attribute analysis (in follow-up PRs).

jhance · 2022-04-19T18:05:08Z

mypyc/test/test_serialization.py

@@ -58,7 +58,7 @@ def assert_blobs_same(x: Any, y: Any, trail: Tuple[Any, ...]) -> None:
        assert x.keys() == y.keys(), "Keys mismatch at {}".format(trail)
        for k in x.keys():
            assert_blobs_same(x[k], y[k], trail + (k,))
-    elif isinstance(x, Iterable) and not isinstance(x, str):
+    elif isinstance(x, Iterable) and not isinstance(x, (str, set)):


It is a bit odd that we treat sets different from other iterables (I understand treating strs differently)

Sets have unpredictable ordering, so we can't use the special casing for them.

jhance · 2022-04-19T18:07:30Z

mypyc/analysis/attrdefined.py

+            # If an attribute we *set* may be sometimes undefined and
+            # sometimes defined, don't consider it always defined.
+            if isinstance(op, SetAttr) and op.obj is self_reg:
+                attr = op.attr


Lets use op.attr here like above? I don't think we get much benefit, and avoiding clutting the namespace of a large-ish function would be better. (And it would be more similar to above)

jhance · 2022-04-19T18:10:05Z

mypyc/analysis/attrdefined.py

+                if op.attr in maybe_undefined.before[block, i]:
+                    attrs.discard(op.attr)
+            # If an attribute we *set* may be sometimes undefined and
+            # sometimes defined, don't consider it always defined.


Can you explain what makes it different from the get case?

jhance · 2022-04-19T18:12:31Z

mypyc/analysis/attrdefined.py

+dump_always_defined: Final = False
+
+
+def analyze_always_defined_attrs(class_irs: List[ClassIR]) -> None:


Either here, or in analyze_alays_defined_attrs_in_class should explain which attributes are being set on the class IR

jhance · 2022-04-19T18:14:35Z

mypyc/ir/class_ir.py

+        # Instance attributes that are initialized in the class body.
+        self.attrs_with_defaults: Set[str] = set()
+
+        # Attributes that are always initialized in __init__ or class body


Explain where they should be set here too, I think. Or choose one of this and the other comment to put the explanation.

msullivan · 2022-04-22T01:12:23Z

mypy/copytype.py

+
+    This can be used to mutate the copy with truthiness information.
+
+    Classes compiled with mypyc don't support copy.copy(), so we need


Not supporting copy.copy is pretty rough. This will also mean that we don't support pickle, right?

I don't think that we can reasonably break those without providing an opt-opt.

That's a good point. I was planning to provide a way to do this later, but I can add it to this PR. We could have a per-class flag saying that this class (and any subclasses, probably) is picklable, and no attribute would be treated as always defined in the class, and thus we can always provide a way of creating an uninitialized instance.

Now it's supported by using @mypyc_attr(serializable=True).

msullivan · 2022-04-22T01:21:10Z

mypyc/analysis/dataflow.py

@@ -128,100 +128,100 @@ def __str__(self) -> str:
        return 'before: %s\nafter: %s\n' % (self.before, self.after)


-GenAndKill = Tuple[Set[Value], Set[Value]]


Why not GenAndKill = Tuple[Set[T], Set[T]]?

A good idea! Will do.

ilevkivskyi

👍

ilevkivskyi · 2022-05-16T12:21:23Z

mypyc/test-data/run-classes.test

+
+from m import define_interpreted_subclass
+
+@mypyc_attr(allow_interpreted_subclasses=True)


Would it make sense to add a test with both allow_interpreted_subclasses and serializable? For example, what if they are "conflicting" like former is True but latter is False.

This would be reasonable. allow_interpreted_subclasses would take precedence and the class would be serializable. In the future we could reject incompatible attribute values.

ilevkivskyi · 2022-05-16T12:25:08Z

mypyc/doc/differences_from_python.rst

+            self.n = n
+
+    data = pickle.dumps(Cls(5))
+    obj = pickle.loads(data)  # OK


Maybe mention what happens without the attr? Does it segfault or raises an exception etc.

A good idea. I'll update this. It raises an exception about __init__ being called with too few arguments, IIRC.

github-actions · 2022-05-17T09:53:50Z

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

JukkaL added 30 commits April 15, 2022 09:52

[mypyc] Make BaseAnalysisVisitor generic

7f3c385

This way we can more easily implement data flow analyses that don't range over Values.

Fix issues caused by rebase

7c7db8e

Add missing import

89c65cf

Fixed to arbitrary execution visitor

79d2dd4

Add missing cases to visitor

9ba9a77

Remove attr initialization detection from main ir build phase

05f596b

Revert more changes

f3d55e0

Add attribute defined implementation

41c7c9e

Remove unused imports

31e413c

Partial implementation of always defined attr analysis using IR

8540685

Fix some special cases

c329c6f

Functions that can't run arbitrary code are fine

32e4cab

Add docstring

70c18a7

Remove original attempt that worked on mypy ATSs

129ec0d

Detect initialization-only setattr ops

4ee1558

Fix initialization of always defined attributes

cd2132a

Add test cases

08262f1

Add test cases

f9a9741

Add comment

181a334

Fix asserts

b9b601b

Refactor + add docstrings and comments

4b385d4

Record which attributes have default values in class body

d2fe861

Treat attributes initialized in class body as always defined

ae2568c

Add IR build test case for attribute default

b076a42

Add tests

3194d92

Fix issues caused by rebase

d79baad

Update singledispatch test cases

37e5508

Analyze always defined attributes across class hierarchy

ef38388

This comment has been minimized.

Sign in to view

jhance reviewed Apr 19, 2022

View reviewed changes

JukkaL requested a review from msullivan April 19, 2022 18:06

jhance reviewed Apr 19, 2022

View reviewed changes

msullivan reviewed Apr 22, 2022

View reviewed changes

JukkaL added 6 commits May 15, 2022 11:10

Merge branch 'master' into always-defined-inheritance-5

2477704

Support TypeVarTupleType

96e99d6

Address some feedback

fed8ecd

Rename init_unknown_code to init_self_leak

53caf2b

Update comments based on feedback

8e371a7

Adjust GenAndKill based on feedback

dd436fb

This comment has been minimized.

Sign in to view

JukkaL added 3 commits May 15, 2022 15:04

Allow supporting pickle/copy by using @mypyc_attr(serialize=True)

e409f5d

Document the serializable flag

d3cc68b

Fix example

49efa36

This comment has been minimized.

Sign in to view

ilevkivskyi approved these changes May 16, 2022

View reviewed changes

Address feedback

51e23b0

JukkaL merged commit 7bd6fdd into master May 17, 2022

JukkaL deleted the always-defined-inheritance-5 branch May 17, 2022 17:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mypyc] Detect always defined attributes #12600

[mypyc] Detect always defined attributes #12600

JukkaL commented Apr 16, 2022

This comment has been minimized.

JukkaL commented Apr 16, 2022

JukkaL commented Apr 16, 2022

This comment has been minimized.

This comment has been minimized.

jhance Apr 19, 2022

JukkaL May 15, 2022

jhance Apr 19, 2022

jhance Apr 19, 2022

jhance Apr 19, 2022 •

edited

Loading

jhance Apr 19, 2022

msullivan Apr 22, 2022

JukkaL Apr 22, 2022

JukkaL May 15, 2022

msullivan Apr 22, 2022

JukkaL May 15, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ilevkivskyi left a comment

ilevkivskyi May 16, 2022

JukkaL May 16, 2022

ilevkivskyi May 16, 2022

JukkaL May 16, 2022 •

edited

Loading

github-actions bot commented May 17, 2022

		dump_always_defined: Final = False


		def analyze_always_defined_attrs(class_irs: List[ClassIR]) -> None:


		This can be used to mutate the copy with truthiness information.

		Classes compiled with mypyc don't support copy.copy(), so we need

		@@ -128,100 +128,100 @@ def __str__(self) -> str:
		return 'before: %s\nafter: %s\n' % (self.before, self.after)


		GenAndKill = Tuple[Set[Value], Set[Value]]


		from m import define_interpreted_subclass

		@mypyc_attr(allow_interpreted_subclasses=True)

[mypyc] Detect always defined attributes #12600

[mypyc] Detect always defined attributes #12600

Conversation

JukkaL commented Apr 16, 2022

This comment has been minimized.

JukkaL commented Apr 16, 2022

JukkaL commented Apr 16, 2022

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhance Apr 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ilevkivskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JukkaL May 16, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented May 17, 2022

jhance Apr 19, 2022 •

edited

Loading

JukkaL May 16, 2022 •

edited

Loading