[issue41361] Converting collections.deque methods to Argument Clinic

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Thanks for the PR :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Raymond Hettinger]

Raymond Hettinger  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Dennis Sweeney]

Change by Dennis Sweeney :


--
pull_requests: +23564
pull_request: https://github.com/python/cpython/pull/24796

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Raymond Hettinger]

Raymond Hettinger  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Dennis Sweeney]

Dennis Sweeney  added the comment:

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", )) {
+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().

--
nosy: +Dennis Sweeney

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Raymond Hettinger]

Raymond Hettinger  added the comment:

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.

--
resolution:  -> not a bug
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Raymond Hettinger]

Change by Raymond Hettinger :


--
assignee:  -> rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Dong-hee Na]

Dong-hee Na  added the comment:

> collections.deque was intentionally not converted to Argument Clinic,

Oh, I see ..

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

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.

--
nosy: +rhettinger, serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Dong-hee Na]

Change by Dong-hee Na :


--
keywords: +patch
pull_requests: +20725
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/21584

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41361] Converting collections.deque methods to Argument Clinic

[Dong-hee Na]

New submission from Dong-hee Na :

There was no performance regressin with this change.

--
components: Argument Clinic
messages: 374068
nosy: corona10, larry
priority: normal
severity: normal
status: open
title: Converting collections.deque methods to Argument Clinic
type: enhancement
versions: Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com