performance degradation creating a mock object (by factor 7-8)

[julianhille]

BPO	38895
Nosy	@cjw296, @voidspace, @lisroach, @mariocj89, @tirkarthi, @julianhille, @marseel

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-11-22.15:57:32.933>
labels = ['3.7', '3.8', '3.9', 'performance']
title = 'performance degradation creating a mock object (by factor 7-8)'
updated_at = <Date 2019-11-26.04:51:16.008>
user = 'https://github.com/julianhille'

bugs.python.org fields:

activity = <Date 2019-11-26.04:51:16.008>
actor = 'xtreak'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2019-11-22.15:57:32.933>
creator = 'julianhille'
dependencies = []
files = []
hgrepos = []
issue_num = 38895
keywords = []
message_count = 7.0
messages = ['357297', '357379', '357382', '357431', '357435', '357450', '357481']
nosy_count = 7.0
nosy_names = ['cjw296', 'michael.foord', 'lisroach', 'mariocj89', 'xtreak', 'julianhille', 'marseel']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue38895'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

Linked PRs

• gh-83076: 3.8x speed improvement in (Async)Mock instantiation #100252

[julianhille]

There seems to be a performance issue when creating a Mock() object from unittest module.
The performance difference between 3.7.x and 3.8.0 is about 7-8 times slower in 3.8

Heres the smalles sample i could generate:

Using python 3.7.5

python3 -m timeit -v --number=100000 --setup="from unittest.mock import Mock" "Mock()"
raw times: 2.99 sec, 2.96 sec, 3.33 sec, 2.98 sec, 2.92 sec

100000 loops, best of 5: 29.2 usec per loop

Using python 3.8.0

python3 -m timeit -v --number=100000 --setup="from unittest.mock import Mock" "Mock()"
raw times: 16.9 sec, 17 sec, 17.7 sec, 18.1 sec, 16.3 sec

100000 loops, best of 5: 163 usec per loop

I did not find that issue, but a co-worker.

[marseel]

I've also tested it and can confirm it.

Master branch:
raw times: 8.43 sec, 7.26 sec, 8.16 sec, 8.4 sec, 7.31 sec 100000 loops, best of 5: 72.6 usec per loop

v3.8.0:
raw times: 13.6 sec, 11.9 sec, 11.6 sec, 11.7 sec, 12.3 sec 100000 loops, best of 5: 116 usec per loop

v3.7.4:
raw times: 2.55 sec, 1.9 sec, 2.7 sec, 2.42 sec, 2.17 sec 100000 loops, best of 5: 19 usec per loop

[marseel]

This is the first commit I've observed slow down:

77b3b77

Especially this part looks suspicious
77b3b77#diff-ff75b1b83c21770847ade91fa5bb2525R366

[tirkarthi]

Thanks Marcel for the pointer. I can confirm the performance impact. This occurs in the common case where not being an AsyncMock the signature of NonCallableMock.__init__ is created every time and then bind_partial is used to detect the spec being supplied to be async. It seems creating the signature of NonCallableMock.__init__ per mock creation is expensive and since it doesn't change can we just create the signature once and set it as a module level attribute? There might still be room for some more optimisations here to reduce the impact.

$ python3.7 -m timeit -s 'from unittest.mock import Mock' 'Mock()'
20000 loops, best of 5: 17.6 usec per loop

# Creating signature object per run (Python 3.8.0)

$ ./python.exe -m timeit -s 'from unittest.mock import Mock' 'Mock()'
2000 loops, best of 5: 109 usec per loop

# Set the signature object of NonCallableMock.__init__ as a private module level attribute (Python 3.8.0)

./python.exe -m timeit -s 'from unittest.mock import Mock' 'Mock()'
5000 loops, best of 5: 66.4 usec per loop

[tirkarthi]

Another point is that _spec_asyncs is a list of attributes that pass asyncio.iscoroutinefunction which could be also little expensive [0]. The check is made for the attribute to be async only when the child mock is created to return an AsyncMock [1] during creation. This could be moved to _get_child_mock so that the Mock creation itself for all other mocks and common use case is faster. Creating child mocks will have the iscoroutine function check performed where maybe we can populate the _spec_async list and use it for subsequent calls.

# Baseline 3.7

$ python3.7 -m timeit -s 'from unittest.mock import Mock' 'Mock()'
20000 loops, best of 5: 17.6 usec per loop

# Move NonCallableMock.__init__ signature to module level attribute. (Python 3.8 branch HEAD)

$ ./python.exe -m timeit -s 'from unittest.mock import Mock' 'Mock()'
5000 loops, best of 5: 62.1 usec per loop

# Move the iscoroutinefunction check to the child mock creation. I didn't do the child mock creation benchmark yet and populating _spec_async as the attribute is found to be async would resolve doing iscoroutinefunction check everytime. (Python 3.8 branch HEAD)

$ ./python.exe -m timeit -s 'from unittest.mock import Mock' 'Mock()'
10000 loops, best of 5: 28.3 usec per loop

[0]

cpython/Lib/unittest/mock.py

Lines 488 to 492 in 27fc3b6

	_spec_asyncs = []
	for attr in dir(spec):
	if asyncio.iscoroutinefunction(getattr(spec, attr, None)):
	_spec_asyncs.append(attr)

[1]

cpython/Lib/unittest/mock.py

Line 987 in 27fc3b6

if _new_name in self.__dict__['_spec_asyncs']:

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 488ab1c23d..7ff99407ab 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -403,7 +403,6 @@ class NonCallableMock(Base):
         bases = (cls,)
         if not issubclass(cls, AsyncMock):
             # Check if spec is an async object or function
-            sig = inspect.signature(NonCallableMock.__init__)
             bound_args = sig.bind_partial(cls, *args, **kw).arguments
             spec_arg = [
                 arg for arg in bound_args.keys()
@@ -491,11 +490,6 @@ class NonCallableMock(Base):
                        _eat_self=False):
         _spec_class = None
         _spec_signature = None
-        _spec_asyncs = []
-
-        for attr in dir(spec):
-            if asyncio.iscoroutinefunction(getattr(spec, attr, None)):
-                _spec_asyncs.append(attr)
 
         if spec is not None and not _is_list(spec):
             if isinstance(spec, type):
@@ -513,7 +507,6 @@ class NonCallableMock(Base):
         __dict__['_spec_set'] = spec_set
         __dict__['_spec_signature'] = _spec_signature
         __dict__['_mock_methods'] = spec
-        __dict__['_spec_asyncs'] = _spec_asyncs
 
     def __get_return_value(self):
         ret = self._mock_return_value
@@ -989,7 +982,8 @@ class NonCallableMock(Base):
         For non-callable mocks the callable variant will be used (rather than
         any custom subclass)."""
         _new_name = kw.get("_new_name")
-        if _new_name in self.__dict__['_spec_asyncs']:
+        attribute = getattr(self.__dict__['_spec_class'], _new_name, None)
+        if asyncio.iscoroutinefunction(attribute):
             return AsyncMock(**kw)
 
         _type = type(self)
@@ -1032,6 +1026,8 @@ class NonCallableMock(Base):
         return f"\n{prefix}: {safe_repr(self.mock_calls)}."
 
 
+sig = inspect.signature(NonCallableMock.__init__)
+
 
 def _try_iter(obj):
     if obj is None:

[marseel]

"It seems creating the signature of NonCallableMock.__init__ per mock creation is expensive and since it doesn't change can we just create the signature once and set it as a module level attribute? There might still be room for some more optimisations here to reduce the impact."

This is already done in master branch ;)

"This could be moved to _get_child_mock so that the Mock creation itself for all other mocks and common use case is faster. Creating child mocks will have the iscoroutine function check performed where maybe we can populate the _spec_async list and use it for subsequent calls."

This seems like a reasonable solution.
I've tested it and it improves mock creation speed 2x.

Do you mind if I create PR for it? I would like to start contributing to CPython ;)

[tirkarthi]

This is already done in master branch ;)

Thanks, I noticed the speed improvement in master and 3.7 but assumed it's due to some other optimisation in master.

Do you mind if I create PR for it? I would like to start contributing to CPython ;)

Sure, I will be happy to review. I would like to know others thoughts on this too on deferring whether the attribute is async check to the mock's own creation in _get_child_mock instead of making the parent mock construction slightly expensive as per the current approach. We can at least backport the NonCallableMock.__init__ signature fix which seems safe to make an improvement.

Thanks

[iritkatriel]

Do you mind if I create PR for it? I would like to start contributing to CPython ;)

@marseel Are you still planning to work on this?

[carljm]

The slowness of Mock and AsyncMock creation has caused timeout/slowdown issues in our (very large) test suite that makes heavy use of mock objects. In addition to moving the check for async-function attributes to _get_child_mock (as described above) my profiling has shown a couple other opportunities for large improvements:

1. Even though getting the signature of NonCallableMock.__init__ has been moved to module level so it isn't done every time in NonCallableMock.__new__, there's still a significant cost to the bind_partial call to match up arguments. This really seems unnecessary, as the only gain from it is that we don't have to duplicate the __init__ argument list. In my testing, duplicating the argument list (instead of using *args, **kwargs in the signature of __new__) and removing the bind_partial call is an additional 2x improvement in the cost of instantiating a Mock object. Duplicating this argument list (only a few lines away, in the same file) seems like a very small price to pay for such a significant speed increase for all users of Mock.

2. A large chunk of the cost of creating an AsyncMock is the fact that every AsyncMock creation eagerly creates an internal NonCallableMock(spec_set=CodeType) for use as its __code__ attribute, so that the AsyncMock can pass iscoroutinefunction() checks. There are a couple possible improvements here. The biggest speed improvement (and simplest approach) is to use a real dummy async-func code object for the __code__ attribute, instead of a specced Mock. The downside here is that it could be backwards incompatible if anyone's test suite is actually relying on the __code__ attribute of an AsyncMock being a mock object itself; this seems quite unlikely but in theory it is possible. A second, fully compatible, approach (which still provides a 3x speed improvement all by itself) is to extract the attribute list and signature of CodeType just once at module level and construct the CodeType mock a bit more manually, bypassing a lot of the introspection cost of creating a specced mock.

I will submit a PR that does all three of these things (using the more-compatible version of (2)). Taken together, the PR provides an almost 4x improvement in both Mock() and AsyncMock() creation.

[hauntsaninja]

Closing, since we've clawed a good amount of performance back + it was decided not to backport Carl's PR. Thanks everyone!