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?