bpo-37207: Use vectorcall for range() #18464

encukou · 2020-02-11T16:37:42Z

This continues the range() part of #13930. The complete pull request is stalled on discussions around dicts, but range() should not be controversial. (And I plan to open PRs for other parts if this is merged.)
On top of Mark's change, I unified range_new and range_vectorcall, which had a lot of duplicate code.

https://bugs.python.org/issue37207

Automerge-Triggered-By: @encukou


        Use vectorcall for range()


        Add news entry.


        Unify implementations of range_new and range_vectorcall

vstinner · 2020-02-11T16:52:21Z

Objects/rangeobject.c

+range_vectorcall(PyTypeObject *type, PyObject *const *args,
+                 size_t nargsf, PyObject *kwnames)
+{
+    size_t nargs = PyVectorcall_NARGS(nargsf);


Oh, I didn't expect a size_t here. I opened an issue to discuss PyVectorcall_NARGS() return type: https://bugs.python.org/issue39611

The return type already is Py_ssize_t. The bug was only in this patch.
(When Mark was originally writing the patch, there was a discussion of Py_ssize_t vs. size_t. Mark is a big proponent of the unsigned type, which is more correct. But also much less compatible with the existing codebase.)

vstinner · 2020-02-11T16:52:21Z

Objects/rangeobject.c

@@ -71,43 +72,35 @@ make_range_object(PyTypeObject *type, PyObject *start,
   range(0, 5, -1)
 */
 static PyObject *
-range_new(PyTypeObject *type, PyObject *args, PyObject *kw)
+range_from_array(PyTypeObject *type, PyObject *const *args, size_t num_args)


I would prefer to use Py_ssize_t type for num_args. Usually, it's called "nargs", but I'm fine with "num_args" ;-)

Right, Py_ssize_t is more consistent.
But I find num_args more descriptive, so I'll keep that name.

codecov · 2020-02-11T17:04:45Z

Codecov Report

Merging #18464 into master will increase coverage by 1.08%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #18464       +/-   ##
===========================================
+ Coverage   82.11%   83.20%    +1.08%     
===========================================
  Files        1955     1571      -384     
  Lines      588958   414749   -174209     
  Branches    44428    44456       +28     
===========================================
- Hits       483632   345077   -138555     
+ Misses      95677    60024    -35653     
+ Partials     9649     9648        -1

Impacted Files	Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py	`30.00% <0.00%> (-65.00%)`	⬇️
Lib/distutils/command/bdist_rpm.py	`7.63% <0.00%> (-56.88%)`	⬇️
Lib/test/test_urllib2net.py	`76.92% <0.00%> (-13.85%)`	⬇️
Lib/test/test_smtpnet.py	`78.57% <0.00%> (-7.15%)`	⬇️
Lib/ftplib.py	`63.85% <0.00%> (-6.06%)`	⬇️
Lib/test/test_ftplib.py	`87.11% <0.00%> (-4.72%)`	⬇️
Tools/scripts/db2pickle.py	`17.82% <0.00%> (-3.97%)`	⬇️
Tools/scripts/pickle2db.py	`16.98% <0.00%> (-3.78%)`	⬇️
Lib/test/test_socket.py	`71.94% <0.00%> (-3.77%)`	⬇️
Lib/test/test_asyncio/test_base_events.py	`91.84% <0.00%> (-3.30%)`	⬇️
... and 465 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3e7ea5...b4559ce. Read the comment docs.


        Use Py_ssize_t for number of arguments

vstinner · 2020-02-18T13:41:00Z

Objects/rangeobject.c

@@ -122,14 +115,14 @@ range_new(PyTypeObject *type, PyObject *args, PyObject *kw)
            return NULL;
        default:
            PyErr_Format(PyExc_TypeError,
-                         "range expected at most 3 arguments, got %zd",
+                         "range expected at most 3 arguments, got %zu",
                         num_args);


It should be reset to %zd, since num_args is signed, no?

You're right.


        Reset format argument for Py_ssize_t to %zd

vstinner

LGTM.


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207


        bpo-37207: Use vectorcall for range() (pythonGH-18464)

This continues the `range()` part of python#13930. The complete pull request is stalled on discussions around dicts, but `range()` should not be controversial. (And I plan to open PRs for other parts if this is merged.) On top of Mark's change, I unified `range_new` and `range_vectorcall`, which had a lot of duplicate code. https://bugs.python.org/issue37207

markshannon and others added 3 commits Apr 1, 2019

Use vectorcall for range()

b4f3271

Add news entry.

6d8063b

Unify implementations of range_new and range_vectorcall

Loading status checks…

804e18c

encukou requested review from methane, jdemeyer, tirkarthi and markshannon Feb 11, 2020

the-knights-who-say-ni added the CLA signed label Feb 11, 2020

bedevere-bot added the awaiting core review label Feb 11, 2020

encukou mentioned this pull request Feb 11, 2020

bpo-37207: Use PEP 590 vectorcall to speed up range(), list() and dict() by about 30% #13930

Open

vstinner reviewed Feb 11, 2020

View changes

Use Py_ssize_t for number of arguments

Loading status checks…

663d8cc

vstinner reviewed Feb 18, 2020

View changes

Reset format argument for Py_ssize_t to %zd

Loading status checks…

b4559ce

vstinner approved these changes Feb 18, 2020

View changes

bedevere-bot added awaiting merge and removed awaiting core review labels Feb 18, 2020

encukou added the 🤖 automerge label Feb 18, 2020

bedevere-bot removed the awaiting merge label Feb 18, 2020

encukou deleted the encukou:vectorcall-newrange branch Feb 18, 2020

python / cpython

bpo-37207: Use vectorcall for range() #18464

bpo-37207: Use vectorcall for range() #18464

encukou commented Feb 11, 2020 •

edited by miss-islington

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

codecov bot commented Feb 11, 2020 •

edited

This comment has been minimized.

This comment has been minimized.

vstinner left a comment

python / cpython

Sponsor python/cpython

Join GitHub today

bpo-37207: Use vectorcall for range() #18464

bpo-37207: Use vectorcall for range() #18464

Conversation

encukou commented Feb 11, 2020 • edited by miss-islington

This comment has been minimized.

vstinner Feb 11, 2020

This comment has been minimized.

encukou Feb 18, 2020 • edited

This comment has been minimized.

vstinner Feb 11, 2020

This comment has been minimized.

encukou Feb 18, 2020

This comment has been minimized.

codecov bot commented Feb 11, 2020 • edited

Codecov Report

This comment has been minimized.

vstinner Feb 18, 2020

This comment has been minimized.

encukou Feb 18, 2020

vstinner left a comment

encukou commented Feb 11, 2020 •

edited by miss-islington

encukou Feb 18, 2020 •

edited

codecov bot commented Feb 11, 2020 •

edited