Speed improvements for typing by ilevkivskyi · Pull Request #439 · python/typing

ilevkivskyi · 2017-06-11T14:26:15Z

This PR contains two performance improvements based on profiling and inspired by #432:

Inline _gorg and _geqv
Remove (IMO unnecessary) "duplicates" from MRO.

This gives roughly 5% speed-up for mypy mypy. @JukkaL I will be grateful if you could do more benchmarking with some real-life large codebases.

ilevkivskyi · 2017-06-11T14:28:03Z

This now only touches Python 3 version. If Jukka's results will be promising, then I will also backport this to Python 2.

JelleZijlstra · 2017-06-11T16:23:46Z

I'm also seeing a few % speedup running mypy on my codebase.

gvanrossum

This makes sense as a speedup as long as you can convince me the semantics are unchanged.

gvanrossum · 2017-06-11T19:11:34Z

src/typing.py

-    """
-    assert isinstance(a, GenericMeta) and isinstance(b, GenericMeta)
-    # Reduce each to its origin.
-    return _gorg(a) is _gorg(b)


So is _gorb(b) entirely redundant here? I see that you've inlined this as a._gorg is b everywhere.

Probably this is just a coincidence, maybe it was different in older versions, but now b (and its ._gorg) is known "statically" in all places where _geqv is used.

gvanrossum · 2017-06-11T22:37:45Z

src/typing.py

+        new_bases = []
+        for base in bases:
+            if isinstance(base, GenericMeta):
+                bextra = getattr(base, '__extra__', None)


As long as we're micro-optimizing, this call is wasted when extra is falsey or origin is truthy (if I understand the condition below correctly).

OK, I reshuffled code here, so that we quickly escape if origin is truthy and then if extra is falsey.

gvanrossum · 2017-06-11T22:39:01Z

src/typing.py

+            if isinstance(base, GenericMeta):
+                bextra = getattr(base, '__extra__', None)
+                if (extra and bextra and not origin and bextra not in bases and
+                        type(bextra) is abc.ABCMeta):


I'm not sure how this is equivalent to the original code (I would have expected something simpler, matching _gorg(b) if isinstance(b, GenericMeta) else b.

The point is this is not totally equivalent. This makes more "aggressive" erasure for classes with __extra__, this shortens MROs for special typing types. Compare:
Before:

>>> pprint(List[int].__mro__) (typing.List[int], typing.List, <class 'list'>, typing.MutableSequence, <class 'collections.abc.MutableSequence'>, typing.Sequence, <class 'collections.abc.Sequence'>, typing.Reversible, <class 'collections.abc.Reversible'>, typing.Collection, <class 'collections.abc.Collection'>, <class 'collections.abc.Sized'>, typing.Iterable, <class 'collections.abc.Iterable'>, typing.Container, <class 'collections.abc.Container'>, typing.Generic, <class 'object'>)

and after:

(typing.List[int], typing.List, <class 'list'>, <class 'collections.abc.MutableSequence'>, <class 'collections.abc.Sequence'>, <class 'collections.abc.Reversible'>, <class 'collections.abc.Collection'>, <class 'collections.abc.Sized'>, <class 'collections.abc.Iterable'>, <class 'collections.abc.Container'>, <class 'object'>)

I am actually a bit worried maybe this is too aggressive, since sometimes we even lose Generic, for example:

>>> pprint(MutableMapping.__mro__) (typing.MutableMapping, <class 'collections.abc.MutableMapping'>, <class 'collections.abc.Mapping'>, <class 'collections.abc.Collection'>, <class 'collections.abc.Sized'>, <class 'collections.abc.Iterable'>, <class 'collections.abc.Container'>, <class 'object'>)

Although all tests pass, and I tried to play with this and it works as expected (it is still an instance of GenericMeta). Probably, I will add Generic at the end of bases in such cases (just for consistency).

gvanrossum · 2017-06-11T22:41:48Z

PS. I'm not sure how we would test this speedup with real code. Did you ask for us to have this patched into the Python version we use to run mypy and then run mypy over our large codebase? That makes sense except the time there is pretty variable (since it runs so long that all sorts of other activities on the same machine affect the timing).

ilevkivskyi · 2017-06-12T13:14:15Z

I am marking this as WIP until I figure out what is going on with Python 2.

@gvanrossum

PS. I'm not sure how we would test this speedup with real code. Did you ask for us to have this patched into the Python version we use to run mypy and then run mypy over our large codebase? That makes sense except the time there is pretty variable (since it runs so long that all sorts of other activities on the same machine affect the timing).

Yes, even if it is not super-stable it would be great to have few more data points.

ilevkivskyi · 2017-06-12T16:17:01Z

@gvanrossum

I decided to keep only obvious optimizations, and leave more aggressive things (that might change semantics) out for now. I think you should be happy with this now.

I will probably make another PR soon dealing with (excessively) long MROs.

gvanrossum · 2017-06-12T16:31:52Z

python2/typing.py

        namespace.update({'__origin__': origin, '__extra__': extra})
        self = super(GenericMeta, cls).__new__(cls, name, bases, namespace)
+        super(GenericMeta, self).__setattr__('_gorg', self if not origin
+                                             else origin._gorg)


Whitespace nit: I don't love how this line is broken up; either split before the comma or indent the else to align with the if.

gvanrossum · 2017-06-12T16:33:43Z

src/typing.py

-    """
-    assert isinstance(a, GenericMeta) and isinstance(b, GenericMeta)
-    # Reduce each to its origin.
-    return _gorg(a) is _gorg(b)


gvanrossum · 2017-06-12T16:35:43Z

src/typing.py

        arg_attrs = arg.__dict__.copy()
        for attr, val in arg.__dict__.items():
-            if val in arg.__bases__:
+            if val in arg.__bases__ + (arg,):


Why is this added (arg,) needed? And why not in the PY2 version? (If there's a good reason please add a comment.)

Yes, it is better to have them in both versions, @no_type_chek can be also used in Python 2, fixed this.

Oh wait, it's needed because arg._gorg == arg. Fine.

Oh wait, it's needed because arg._gorg == arg

Exactly. Sorry, I have answered only half of your question.

gvanrossum · 2017-06-12T18:59:00Z

src/typing.py

        arg_attrs = arg.__dict__.copy()
        for attr, val in arg.__dict__.items():
-            if val in arg.__bases__:
+            if val in arg.__bases__ + (arg,):


Oh wait, it's needed because arg._gorg == arg. Fine.

gvanrossum · 2017-06-12T19:01:30Z

I'm torn about whether to merge this. Maybe we should wait until 3.6.2 has actually been released? The RC is supposed to be cut today, I expect the final release in a few weeks (see PEP 494).

ilevkivskyi · 2017-06-12T19:31:23Z

Maybe we should wait until 3.6.2 has actually been released?

This makes sense. We can keep the master "clean" (i.e. identical with current CPython state) for any emergency fixes (just in case). Then I can merge this right after the release of 3.6.2.final.

ilevkivskyi · 2017-07-17T13:57:49Z

OK, Python 3.6.2 was released earlier today, so that I am merging this now.

Incorporates change made by python#439, which was recently merged.

ilevkivskyi added 6 commits June 11, 2017 14:01

Inline _geqv and _gorg

e416127

Fix typo

9c60a07

Cut MROs in half

7d192a4

Avoid recursion on _gorg

f5881a2

Fix lint

0318571

Fxi lint (for realz)

e84c51f

the-knights-who-say-ni added the CLA signed label Jun 11, 2017

ilevkivskyi mentioned this pull request Jun 11, 2017

Mark Shannon's presentation at the 2017 Language Summit #432

Closed

gvanrossum reviewed Jun 11, 2017

View reviewed changes

Ivan Levkivskyi added 5 commits June 12, 2017 13:43

Refactor erasure logic to make it clearer (and probably faster)

b5152e2

Remove redundant check

369b252

Remove unnecessary comment

0d3dde6

Backport first part to Python 2

ba3ffbd

Backport second part to Python 2

630aeef

ilevkivskyi changed the title ~~Speed improvements for typing~~ [WIP] Speed improvements for typing Jun 12, 2017

Keep only obvious speed-ups and drop more aggressive optiomizations

995572f

ilevkivskyi changed the title ~~[WIP] Speed improvements for typing~~ Speed improvements for typing Jun 12, 2017

Fix _Protocol on Python 2

9d54a4a

gvanrossum reviewed Jun 12, 2017

View reviewed changes

Address CR

6b223a1

gvanrossum approved these changes Jun 12, 2017

View reviewed changes

ilevkivskyi mentioned this pull request Jun 27, 2017

Add typing extensions #443

Merged

ilevkivskyi merged commit 2b6932a into python:master Jul 17, 2017

ilevkivskyi deleted the speed-up-typing branch July 17, 2017 13:58

Michael0x2a added a commit to Michael0x2a/typing that referenced this pull request Jul 17, 2017

Incorporate speed improvements for typing

11c0fcf

Incorporates change made by python#439, which was recently merged.

ilevkivskyi mentioned this pull request Sep 13, 2017

bpo-28556: Minor updates to typing module python/cpython#3550

Merged

Conversation

ilevkivskyi commented Jun 11, 2017

Uh oh!

ilevkivskyi commented Jun 11, 2017

Uh oh!

JelleZijlstra commented Jun 11, 2017

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Jun 11, 2017

Uh oh!

ilevkivskyi commented Jun 12, 2017

Uh oh!

ilevkivskyi commented Jun 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Jun 12, 2017

Uh oh!

ilevkivskyi commented Jun 12, 2017

Uh oh!

ilevkivskyi commented Jul 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants