Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Dec 23, 2022

Conversation

carljm
Copy link
Member

@carljm carljm commented Dec 14, 2022

This PR makes several performance optimizations to instantiation of Mock() and AsyncMock(). The changes are discussed in more detail in the linked issue #83076. The changes result in a net speedup of 3.8x for creation of both kinds of mocks.

Before this PR:

➜ ./python -m timeit --setup="from unittest.mock import Mock" "Mock()"
2000 loops, best of 5: 97.9 usec per loop

➜ ./python -m timeit --setup="from unittest.mock import AsyncMock" "AsyncMock()"
500 loops, best of 5: 678 usec per loop

After this PR:

➜ ./python -m timeit --setup="from unittest.mock import Mock" "Mock()"
10000 loops, best of 5: 25.6 usec per loop

➜ ./python -m timeit --setup="from unittest.mock import AsyncMock" "AsyncMock()"
2000 loops, best of 5: 181 usec per loop

There's also one simple code cleanup here: the AsyncMagicMixin.__init__ method was an exact duplicate of the inherited MagicMixin.__init__. This is unnecessary duplication, we can just inherit it instead.

@netlify
Copy link

netlify bot commented Dec 14, 2022

Deploy Preview for python-cpython-preview canceled.

Name Link
🔨 Latest commit 246772c
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/639a0b711b176f0007bd9ca0

@carljm carljm changed the title gh-83076: ~4x speed improvement in (Async)Mock instantiation gh-83076: 3.8x speed improvement in (Async)Mock instantiation Dec 14, 2022
facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request Dec 14, 2022
Summary:
Modified port of D35118477 (793f7d0) from 3.8, and backport of upstream PR
python/cpython#100252

Changes from the 3.8 version:

- We don't need to keep `_spec_asyncs` around at all, it's not public API.
- Changing the `__code__` attribute of an `AsyncMock` to be a real code object
  instead of a mock is probably fine, but it's technically backwards incompatible
  and could break someone's test suite, so I wasn't sure upstream would go for it.
  Instead we can create the mock object in a more manual way that avoids a lot of
  the introspection cost (by doing it just once up front.)

Reviewed By: mpage

Differential Revision: D42039568

fbshipit-source-id: a939f9c
Copy link
Contributor

@itamaro itamaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw, I've confirmed that a backport of this PR to 3.10 passes tests, including the instagram test suite

@Fidget-Spinner Fidget-Spinner added the performance Performance or resource usage label Dec 15, 2022
Lib/unittest/mock.py Show resolved Hide resolved
Lib/unittest/mock.py Outdated Show resolved Hide resolved
* main:
  Improve stats presentation for calls. (pythonGH-100274)
  Better stats for `LOAD_ATTR` and `STORE_ATTR` (pythonGH-100295)
  pythongh-81057: Move the Cached Parser Dummy Name to _PyRuntimeState (python#100277)
  Document that zipfile's pwd parameter is a `bytes` object (python#100209)
  pythongh-99767: mark `PyTypeObject.tp_watched` as internal use only in table (python#100271)
  Fix typo in introduction.rst (python#100266)
  pythongh-78997: AttributeError if loading fails in LibraryLoader.__getattr__
  pythonGH-100234: Set a default value for random.expovariate() (pythonGH-100235)
  Remove uninformative itertools recipe (pythonGH-100253)
  pythonGH-99767: update PyTypeObject docs for type watchers (pythonGH-99928)
  Move stats for the method cache into the `Py_STAT` machinery (pythonGH-100255)
  pythonGH-100222: fix typo _py_set_opocde -> _py_set_opcode (pythonGH-100259)
  pythonGH-100000: Cleanup and polish various watchers code (pythonGH-99998)
  pythongh-90111: Minor Cleanup for Runtime-Global Objects (pythongh-100254)
* main:
  pythongh-99540: Constant hash for _PyNone_Type to aid reproducibility (pythonGH-99541)
  pythongh-100039: enhance __signature__ to work with str and callables (pythonGH-100168)
  pythongh-99830: asyncio: Document returns of remove_{reader,writer} (python#100302)
  "Compound statement" docs: Fix with-statement step indexing (python#100286)
  pythonGH-90043: Handle NaNs in COMPARE_OP_FLOAT_JUMP (pythonGH-100278)
Lib/unittest/mock.py Show resolved Hide resolved
Lib/unittest/mock.py Show resolved Hide resolved
@tirkarthi
Copy link
Member

Since this is a performance improvement I would suggest keeping them in main branch to target 3.12 release to get some more testing during alpha and beta candidates for regression than backporting it to 3.11 that had a patch release and this is not a bug fix.

@cjw296
Copy link
Contributor

cjw296 commented Dec 19, 2022

@tirkarthi - if there was a serious performance regression, then I'd suggest it's worth backporting.
That said, @carljm: for your uses, would the backport work? I haven't cranked that handle in a while...

@carljm
Copy link
Member Author

carljm commented Dec 19, 2022

The performance regression here occurred from 3.7 to 3.8. At least for basic instantiation of a Mock(), this PR eliminates that regression and brings us back to 3.7 perf level.

I don't have strong feelings about backport. For work where we are running into this regression, we use a patched Python anyway, we already carried a patch for this in 3.8 and I will backport this to whatever pre-3.12 versions we need it in, so I don't need it backported upstream.

I don't know what the usual policy in Python is for backporting perf regression fixes. The fact that this regressed in 3.8 and didn't get fixed until 3.12 suggests it's not a big issue for many people (which isn't surprising, I don't think many people have test suites large enough where a regression in Mock creation time would matter.) So maybe that suggests there's no pressing need to backport? OTOH I think the changes are safe and it's a nice speed boost.

* main:
  pythongh-89727: Fix os.walk RecursionError on deep trees (python#99803)
  Docs: Don't upload CI artifacts (python#100330)
  pythongh-94912: Added marker for non-standard coroutine function detection (python#99247)
  Correct CVE-2020-10735 documentation (python#100306)
  pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273)
  pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551)
  Docs: Use `PY_VERSION_HEX` for version comparison (python#100179)
  pythongh-97909: Fix markup for `PyMethodDef` members (python#100089)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205)
  pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469)
Copy link
Contributor

@cjw296 cjw296 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so let's not backport this, but I need to do a rolling backport release some time, so it'll go into that if folks need it.

iritkatriel added a commit to iritkatriel/cpython that referenced this pull request Dec 28, 2022
* Correct CVE-2020-10735 documentation (python#100306)

* pythongh-94912: Added marker for non-standard coroutine function detection (python#99247)

This introduces a new decorator `@inspect.markcoroutinefunction`,
which, applied to a sync function, makes it appear async to
`inspect.iscoroutinefunction()`.

* Docs: Don't upload CI artifacts (python#100330)

* pythongh-89727: Fix os.walk RecursionError on deep trees (python#99803)

Use a stack to implement os.walk iteratively instead of recursively to
avoid hitting recursion limits on deeply nested trees.

* pythongh-69929: re docs: Add more specific definition of \w (python#92015)

Co-authored-by: Jelle Zijlstra <[email protected]>

* pythongh-89051: Add ssl.OP_LEGACY_SERVER_CONNECT (python#93927)

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Christian Heimes <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Fixes python#89051

* pythongh-88211: Change lower-case and upper-case to match recommendations in imaplib docs (python#99625)

* pythongh-100348: Fix ref cycle in `asyncio._SelectorSocketTransport` with `_read_ready_cb` (python#100349)

* pythongh-99925: Fix inconsistency in `json.dumps()` error messages (pythonGH-99926)

* Clarify that every thread has its own default context in contextvars (python#99246)

* pythongh-99576: Fix cookiejar file that was not truncated for some classes (pythonGH-99616)

Co-authored-by: Łukasz Langa <[email protected]>

* pythongh-100188: Reduce misses in BINARY_SUBSCR_(LIST/TUPLE)_INT (python#100189)

Don't specialize if the index is negative.

* pythongh-99991: improve docs on str.encode and bytes.decode (python#100198)

Co-authored-by: C.A.M. Gerlach <[email protected]>

* pythongh-91081: Add note on WeakKeyDictionary behavior when deleting a replaced entry (python#91499)

Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>

* pythongh-85267: Improvements to inspect.signature __text_signature__ handling (python#98796)

This makes a couple related changes to inspect.signature's behaviour
when parsing a signature from `__text_signature__`.

First, `inspect.signature` is documented as only raising ValueError or
TypeError. However, in some cases, we could raise RuntimeError.  This PR
changes that, thereby fixing python#83685.

(Note that the new ValueErrors in RewriteSymbolics are caught and then
reraised with a message)

Second, `inspect.signature` could randomly drop parameters that it
didn't understand (corresponding to `return None` in the `p` function).
This is the core issue in python#85267. I think this is very surprising
behaviour and it seems better to fail outright.

Third, adding this new failure broke a couple tests. To fix them (and to
e.g. allow `inspect.signature(select.epoll.register)` as in python#85267), I
add constant folding of a couple binary operations to RewriteSymbolics.

(There's some discussion of making signature expression evaluation
arbitrary powerful in python#68155. I think that's out of scope. The
additional constant folding here is pretty straightforward, useful, and
not much of a slippery slope)

Fourth, while python#85267 is incorrect about the cause of the issue, it turns
out if you had consecutive newlines in __text_signature__, you'd get
`tokenize.TokenError`.

Finally, the `if name is invalid:` code path was dead, since
`parse_name` never returned `invalid`.

* pythonGH-100363: Speed up `asyncio.get_running_loop` (python#100364)

* pythonGH-100133: fix `asyncio` subprocess losing `stderr` and `stdout` output (python#100154)

* pythongh-100374: Fixed a bug in socket.getfqdn() (pythongh-100375)

* pythongh-100129: Add tests for pickling all builtin types and functions (pythonGH-100142)

* Remove unused variable from `dis._find_imports` (python#100396)

* pythongh-78878: Fix crash when creating an instance of `_ctypes.CField` (python#14837)

* pythonGH-69564: Clarify use of octal format of mode argument in help(os.chmod) (python#20621)

Co-authored-by: Kumar Aditya <[email protected]>

* pythonGH-99554: Pack location tables more effectively (pythonGH-99556)

* Correct typo in typing.py (python#100423)

In the docstring of `ParamSpec`, the name of `P = ParamSpec('P')` was
mistakenly written as `'T'`.

* pythongh-99761: Add `_PyLong_IsPositiveSingleDigit` function to check for single digit integers  (python#100064)

* pythonGH-99770: Make the correct call specialization fail kind show up in the stats (pythonGH-99771)

* pythongh-78997: fix bad rebase of moved test file (python#100424)

* pythongh-100344: Add C implementation for `asyncio.current_task` (python#100345)

Co-authored-by: pranavtbhat

* pythonGH-99554: Trim trailing whitespace (pythonGH-100435)



Automerge-Triggered-By: GH:brandtbucher

* pythongh-85432: Harmonise parameter names between C and pure-Python implementations of `datetime.time.strftime`, `datetime.datetime.fromtimestamp` (python#99993)

* pythongh-57762: fix misleading tkinter.Tk docstring (python#98837)

Mentioned as a desired change by terryjreedy on the corresponding issue,
since Tk is not a subclass of Toplevel.

* pythongh-48496: Added example and link to faq for UnboundLocalError in reference (python#93068)

* Fix typo in 3.12 What's New (python#100449)

* pythongh-76963: PEP3118 itemsize of an empty ctypes array should not be 0 (pythonGH-5576)

The itemsize returned in a memoryview of a ctypes array is now computed from the item type, instead of dividing the total size by the length and assuming that the length is not zero.

* pythonGH-100459: fix copy-paste errors in specialization stats (pythonGH-100460)

* pythongh-99110: Initialize `frame->previous` in init_frame to fix segmentation fault when accessing `frame.f_back` (python#100182)

* pythongh-98712: Clarify "readonly bytes-like object" semantics in C arg-parsing docs (python#98710)

* pythongh-92216: improve performance of `hasattr` for type objects (pythonGH-99979)

* pythongh-100288: Specialise LOAD_ATTR_METHOD for managed dictionaries (pythonGH-100289)

* Revert "pythongh-100288: Specialise LOAD_ATTR_METHOD for managed dictionaries (pythonGH-100289)" (python#100468)

This reverts commit c3c7848.

* pythongh-94155: Reduce hash collisions for code objects (python#100183)

* Uses a better hashing algorithm to get better dispersion and remove commutativity.

* Incorporates `co_firstlineno`, `Py_SIZE(co)`, and bytecode instructions.

* This is now the entire set of criteria used in `code_richcompare`, except for `_PyCode_ConstantKey` (which would incorporate the types of `co_consts` rather than just their values).

* pythongh-83076: 3.8x speed improvement in (Async)Mock instantiation (python#100252)

* pythongh-99482: remove `jython` compatibility parts from stdlib and tests (python#99484)

* bpo-40447: accept all path-like objects in compileall.compile_file (python#19883)

Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
Co-authored-by: Irit Katriel <[email protected]>
Co-authored-by: Shantanu <[email protected]>

* pythonGH-100425: Improve accuracy of builtin sum() for float inputs (pythonGH-100426)

* pythongh-68320, pythongh-88302 - Allow for private `pathlib.Path` subclassing (pythonGH-31691)

Users may wish to define subclasses of `pathlib.Path` to add or modify
existing methods. Before this change, attempting to instantiate a subclass
raised an exception like:

    AttributeError: type object 'PPath' has no attribute '_flavour'

Previously the `_flavour` attribute was assigned as follows:

    PurePath._flavour        = xxx not set!! xxx
    PurePosixPath._flavour   = _PosixFlavour()
    PureWindowsPath._flavour = _WindowsFlavour()

This change replaces it with a `_pathmod` attribute, set as follows:

    PurePath._pathmod        = os.path
    PurePosixPath._pathmod   = posixpath
    PureWindowsPath._pathmod = ntpath

Functionality from `_PosixFlavour` and `_WindowsFlavour` is moved into
`PurePath` as underscored-prefixed classmethods. Flavours are removed.

Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Eryk Sun <[email protected]>

* pythongh-99947: Ensure unreported errors are chained for SystemError during import (pythonGH-99946)

* Add "strict" to dotproduct(). Add docstring. Factor-out common code. (pythonGH-100480)

* pythongh-94808: improve test coverage of number formatting (python#99472)

* pythongh-100454: Start running SSL tests with OpenSSL 3.1.0-beta1 (python#100456)

* pythongh-100268: Add is_integer method to int (python#100439)

This improves the lives of type annotation users of `float` - which type checkers implicitly treat as `int|float` because that is what most code actually wants. Before this change a `.is_integer()` method could not be assumed to exist on things annotated as `: float` due to the method not existing on both types.

* pythongh-77771: Add enterabs example in sched (python#92716)

Co-authored-by: Shantanu <[email protected]>

* pythonGH-91166: Implement zero copy writes for `SelectorSocketTransport` in asyncio (python#31871)

Co-authored-by: Guido van Rossum <[email protected]>

* pythonGH-91166: Implement zero copy writes for `SelectorSocketTransport` in asyncio (python#31871)

Co-authored-by: Guido van Rossum <[email protected]>

* Misc Itertools recipe tweaks (pythonGH-100493)

* pythongh-100357: Convert several functions in `bltinsmodule` to AC (python#100358)

* Remove wrong comment about `repr` in `test_unicode` (python#100495)

* pythongh-99908: Tutorial: Modernize the 'data-record class' example (python#100499)

Co-authored-by: Alex Waygood <[email protected]>

* pythongh-100474: Fix handling of dirs named index.html in http.server (pythonGH-100475)



If you had a directory called index.html or index.htm within a directory, it would cause http.server to return a 404 Not Found error instead of the directory listing. This came about due to not checking that the index was a regular file.

I have also added a test case for this situation.

Automerge-Triggered-By: GH:merwok

* pythongh-100287: Fix unittest.mock.seal with AsyncMock (python#100496)

* pythongh-99535: Add test for inheritance of annotations and update documentation (python#99990)

* pythongh-100428: Make float documentation more accurate (python#100437)

Previously, the grammar did not accept `float("10")`.
Also implement mdickinson's suggestion of removing the indirection.

* [Minor PR] Quotes in documentation changed into code blocks (python#99536)

Minor formatting fix in documentation

Co-authored-by: Shantanu <[email protected]>

* pythongh-100472: Fix docs claim that compileall parameters could be bytes (python#100473)

* pythongh-100519: simplification to `eff_request_host` in cookiejar.py (python#99588)

`IPV4_RE` includes a `.`, and the `.find(".") == -1` included here is already testing to make sure there's no dot, so this part of the expression is tautological. Instead use more modern `in` syntax to make it clear what the check is doing here. The simplified implementation more clearly matches the wording in RFC 2965.

Co-authored-by: hauntsaninja <[email protected]>

* pythongh-99308: Clarify re docs for byte pattern group names (python#99311)

* pythongh-92446: Improve argparse choices docs; revert bad change to lzma docs (python#94627)

Based on the definition of the collections.abc classes, it is more accurate to use "sequence" instead of "container" when describing argparse choices.

A previous attempt at fixing this in python#92450 was mistaken; this PR reverts that change.

Co-authored-by: Shantanu <[email protected]>

* Fix name of removed `inspect.Signature.from_builtin` method in 3.11.0a2 changelog (python#100525)

* pythongh-100520: Fix `rst` markup in `configparser`  docstrings (python#100524)

* pythongh-99509: Add `__class_getitem__` to `multiprocessing.queues.Queue` (python#99511)

* pythongh-94603: micro optimize list.pop (pythongh-94604)

* Remove `NoneType` redefinition from `clinic.py` (python#100551)

* pythongh-100553: Improve accuracy of sqlite3.Row iter test (python#100555)

* pythonGH-98831: Modernize a ton of simpler instructions (python#100545)

* load_const and load_fast aren't families for now
* Don't decref unmoved names
* Modernize GET_ANEXT
* Modernize GET_AWAITABLE
* Modernize ASYNC_GEN_WRAP
* Modernize YIELD_VALUE
* Modernize POP_EXCEPT (in more than one way)
* Modernize PREP_RERAISE_STAR
* Modernize LOAD_ASSERTION_ERROR
* Modernize LOAD_BUILD_CLASS
* Modernize STORE_NAME
* Modernize LOAD_NAME
* Modernize LOAD_CLASSDEREF
* Modernize LOAD_DEREF
* Modernize STORE_DEREF
* Modernize COPY_FREE_VARS (mark it as done)
* Modernize LIST_TO_TUPLE
* Modernize LIST_EXTEND
* Modernize SET_UPDATE
* Modernize SETUP_ANNOTATIONS
* Modernize DICT_UPDATE
* Modernize DICT_MERGE
* Modernize MAP_ADD
* Modernize IS_OP
* Modernize CONTAINS_OP
* Modernize CHECK_EXC_MATCH
* Modernize IMPORT_NAME
* Modernize IMPORT_STAR
* Modernize IMPORT_FROM
* Modernize JUMP_FORWARD (mark it as done)
* Modernize JUMP_BACKWARD (mark it as done)

Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
Co-authored-by: Jeremy Paige <[email protected]>
Co-authored-by: Carlton Gibson <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Jon Burdo <[email protected]>
Co-authored-by: Stanley <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Thomas Grainger <[email protected]>
Co-authored-by: Brad Wolfe <[email protected]>
Co-authored-by: Richard Kojedzinszky <[email protected]>
Co-authored-by: František Nesveda <[email protected]>
Co-authored-by: Pablo Galindo Salgado <[email protected]>
Co-authored-by: Nikita Sobolev <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
Co-authored-by: Dennis Sweeney <[email protected]>
Co-authored-by: Bisola Olasehinde <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: Shantanu <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Dominic Socular <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
Co-authored-by: Hai Shi <[email protected]>
Co-authored-by: amaajemyfren <[email protected]>
Co-authored-by: Brandt Bucher <[email protected]>
Co-authored-by: david-why <[email protected]>
Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: penguin_wwy <[email protected]>
Co-authored-by: Eli Schwartz <[email protected]>
Co-authored-by: Itamar Ostricher <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Eric Wieser <[email protected]>
Co-authored-by: Irit Katriel <[email protected]>
Co-authored-by: Bill Fisher <[email protected]>
Co-authored-by: Petr Viktorin <[email protected]>
Co-authored-by: Ken Jin <[email protected]>
Co-authored-by: Carl Meyer <[email protected]>
Co-authored-by: Filipe Laíns <[email protected]>
Co-authored-by: Raymond Hettinger <[email protected]>
Co-authored-by: Barney Gale <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Eryk Sun <[email protected]>
Co-authored-by: Sebastian Berg <[email protected]>
Co-authored-by: Illia Volochii <[email protected]>
Co-authored-by: JosephSBoyle <[email protected]>
Co-authored-by: James Frost <[email protected]>
Co-authored-by: MonadChains <[email protected]>
Co-authored-by: Bart Broere <[email protected]>
Co-authored-by: Glyph <[email protected]>
Co-authored-by: hauntsaninja <[email protected]>
Co-authored-by: Ilya Kulakov <[email protected]>
Co-authored-by: Guy Yagev <[email protected]>
Co-authored-by: Jakub Kuczys <[email protected]>
@tirkarthi
Copy link
Member

Thanks @hauntsaninja , @carljm and @cjw296

facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request Jan 10, 2023
Summary:
D42039568 (733b7b3) backported an early version of python/cpython#100252,
and there were some changes made before that PR was merged upstream.
Backport those changes so we match upstream behavior. Most importantly,
this restores `_spec_asyncs`, which although a private API does seem to be used.

Reviewed By: itamaro

Differential Revision: D42416767

fbshipit-source-id: a7e7a21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants