- Python Home
- About
- News
- Documentation
- Downloads
- Community
- Foundation
- Developer's Guide
- Issue Tracker
- Issues
- Summaries
- User
- Administration
- Help
Issue41361
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2020-07-21 15:26 by corona10, last changed 2022-04-11 14:59 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 21584 | closed | corona10, 2020-07-21 15:27 | |
| PR 24796 | merged | Dennis Sweeney, 2021-03-09 05:18 | |
| Messages (9) | |||
|---|---|---|---|
| msg374068 - (view) | Author: Dong-hee Na (corona10) * ![]() |
Date: 2020-07-21 15:26 | |
There was no performance regressin with this change. |
|||
| msg374078 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2020-07-21 19:24 | |
See also issue20180. collections.deque was intentionally not converted to Argument Clinic, but some of methods were made using more efficient code for parsing arguments by inlining the code generated by Argument Clinic at that time. |
|||
| msg374082 - (view) | Author: Dong-hee Na (corona10) * ![]() |
Date: 2020-07-22 02:16 | |
> collections.deque was intentionally not converted to Argument Clinic, Oh, I see .. |
|||
| msg374085 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2020-07-22 05:26 | |
Now Argument Clinic generates more efficient (but more cumbersome) code, so there may be new reasons of using it. Please test the performance of the deque constructor (deque(), deque(()), deque((), 10), deque((), maxlen=10)), methods index() and rotate(), builtins iter() and reversed(). It is hard to test insert() because it mutates the deque and I do not expect anything beside noise for other methods. |
|||
| msg374087 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2020-07-22 07:19 | |
I think we should skip applying the clinic to the code. It jumbles the code quite a bit but doesn't give any real benefit. |
|||
| msg388226 - (view) | Author: Dennis Sweeney (Dennis Sweeney) * ![]() |
Date: 2021-03-07 06:26 | |
If the argument clinic is too disruptive, would it be okay to inline the equivalent code like this? diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 90bafb0ea8..d75388abc8 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -880,9 +880,19 @@ deque_rotate(dequeobject *deque, PyObject *const *args, Py_ssize_t nargs) { Py_ssize_t n=1; - if (!_PyArg_ParseStack(args, nargs, "|n:rotate", &n)) { + if (!_PyArg_CheckPositional("deque.rotate", nargs, 0, 1)) { return NULL; } + if (nargs) { + PyObject *index = _PyNumber_Index(args[0]); + if (index == NULL) { + return NULL; + } + n = PyLong_AsSsize_t(index); + if (n == -1 && PyErr_Occurred()) { + return NULL; + } + } if (!_deque_rotate(deque, n)) Py_RETURN_NONE; Benchmarks for this change: .\python.bat -m pyperf timeit -s "from collections import deque; d = deque(range(100))" "d.rotate()" Before: Mean +- std dev: 51.2 ns +- 0.9 ns After: Mean +- std dev: 39.3 ns +- 0.3 ns .\python.bat -m pyperf timeit -s "from collections import deque; d = deque(range(100))" "d.rotate(-1)" Before: Mean +- std dev: 64.5 ns +- 0.3 ns After: Mean +- std dev: 46.2 ns +- 0.3 ns .\python.bat -m pyperf timeit -s "from collections import deque; d = deque(range(100))" "d.rotate(1)" Before: Mean +- std dev: 64.4 ns +- 0.3 ns After: Mean +- std dev: 45.7 ns +- 0.2 ns Similar changes could apply to deque.insert() and deque.index(). |
|||
| msg388323 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2021-03-09 02:12 | |
Go ahead with in-lining the argument handling for rotate(). The no argument form and the +1 and the -1 case are important enough to warrant the change. Please skip index() and insert() which aren't essential methods. Also, those methods aren't called with end-point specific arguments, so the argument processing time doesn't dominate and it isn't worth it. |
|||
| msg388806 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2021-03-16 04:02 | |
New changeset 448801da96c70699e2ca0798efdfe86d45f37f06 by Dennis Sweeney in branch 'master': bpo-41361: Optimized argument parsing for deque_rotate (GH-24796) https://github.com/python/cpython/commit/448801da96c70699e2ca0798efdfe86d45f37f06 |
|||
| msg388807 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2021-03-16 04:02 | |
Thanks for the PR :-) |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:33 | admin | set | github: 85533 |
| 2021-03-16 04:02:51 | rhettinger | set | messages: + msg388807 |
| 2021-03-16 04:02:34 | rhettinger | set | messages: + msg388806 |
| 2021-03-09 05:18:19 | Dennis Sweeney | set | pull_requests: + pull_request23564 |
| 2021-03-09 02:12:09 | rhettinger | set | messages: + msg388323 |
| 2021-03-07 06:26:43 | Dennis Sweeney | set | nosy:
+ Dennis Sweeney messages: + msg388226 |
| 2020-07-22 07:19:48 | rhettinger | set | status: open -> closed resolution: not a bug messages: + msg374087 stage: patch review -> resolved |
| 2020-07-22 06:54:37 | rhettinger | set | assignee: rhettinger |
| 2020-07-22 05:26:49 | serhiy.storchaka | set | messages: + msg374085 |
| 2020-07-22 02:16:02 | corona10 | set | messages: + msg374082 |
| 2020-07-21 19:24:26 | serhiy.storchaka | set | nosy:
+ rhettinger, serhiy.storchaka messages: + msg374078 |
| 2020-07-21 15:27:40 | corona10 | set | keywords:
+ patch stage: patch review pull_requests: + pull_request20725 |
| 2020-07-21 15:26:01 | corona10 | create | |


