Remove private _PyLong_Zero and _PyLong_One variables

[vstinner]

BPO	42161
Nosy	@rhettinger, @mdickinson, @vstinner, @serhiy-storchaka, @corona10, @pablogsal, @miss-islington
PRs	bpo-42161: Add _PyLong_GetZero() and _PyLong_GetOne() #22993 bpo-42161: Use _PyLong_GetZero() and _PyLong_GetOne() #22995 bpo-42161: Use _PyLong_GetZero() and _PyLong_GetOne() #22998 bpo-42161: Remove private _PyLong_Zero and _PyLong_One #23003 bpo-42161: Micro-optimize _collections._count_elements() #23008 bpo-42161: mathmodule.c: move _PyLong_GetOne() loop invariant #26391 [3.10] bpo-42161: mathmodule.c: move _PyLong_GetOne() loop invariant (GH-26391) #26393 bpo-42161: Hoist the _PyLong_GetOne() call out of the inner loop. #30656

^{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 = <Date 2021-05-26.22:54:42.058>
created_at = <Date 2020-10-26.21:59:37.342>
labels = ['interpreter-core', '3.10']
title = 'Remove private _PyLong_Zero and _PyLong_One variables'
updated_at = <Date 2022-01-18.08:02:43.120>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2022-01-18.08:02:43.120>
actor = 'rhettinger'
assignee = 'none'
closed = True
closed_date = <Date 2021-05-26.22:54:42.058>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2020-10-26.21:59:37.342>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 42161
keywords = ['patch']
message_count = 16.0
messages = ['379691', '379700', '379709', '379768', '379798', '379799', '379800', '379804', '379805', '388988', '389002', '389049', '394496', '394498', '394499', '410847']
nosy_count = 7.0
nosy_names = ['rhettinger', 'mark.dickinson', 'vstinner', 'serhiy.storchaka', 'corona10', 'pablogsal', 'miss-islington']
pr_nums = ['22993', '22995', '22998', '23003', '23008', '26391', '26393', '30656']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue42161'
versions = ['Python 3.10']

[vstinner]

In bpo-38858, I made the small integer singletons per interpreter: commit 630c8df. _PyLong_Zero and _PyLong_One variables are still shared by all interpreters, whereas subinterpreters must not share Python objects: see bpo-40533.

I propose to add new _PyLong_GetZero() and _PyLong_GetOne() functions to replace _PyLong_Zero and _PyLong_One variables. These functions will retrieve the singletons from tstate->interp->small_ints.

[vstinner]

New changeset 8e3b9f9 by Victor Stinner in branch 'master':
bpo-42161: Add _PyLong_GetZero() and _PyLong_GetOne() (GH-22993)
8e3b9f9

[vstinner]

New changeset c9bc290 by Victor Stinner in branch 'master':
bpo-42161: Use _PyLong_GetZero() and _PyLong_GetOne() (GH-22995)
c9bc290

[vstinner]

New changeset 3783413 by Victor Stinner in branch 'master':
bpo-42161: Modules/ uses _PyLong_GetZero() and _PyLong_GetOne() (GH-22998)
3783413

[rhettinger]

Why did you put _PyLong_GetOne() inside the loop for the fast path and outside the loop for the slow path?

==========================================================================

diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c
index 8990071f51..0e6c64d1a6 100644
--- a/Modules/_collectionsmodule.c
+++ b/Modules/_collectionsmodule.c
@@ -2278,6 +2278,8 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
     PyObject *dict_get;
     PyObject *mapping_setitem;
     PyObject *dict_setitem;
+    PyObject *zero = _PyLong_GetZero();  // borrowed reference
+    PyObject *one = _PyLong_GetOne();    // borrowed reference

     it = PyObject_GetIter(iterable);
     if (it == NULL)
@@ -2324,10 +2326,10 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
             if (oldval == NULL) {
                 if (PyErr_Occurred())
                     goto done;
-                if (_PyDict_SetItem_KnownHash(mapping, key, _PyLong_GetOne(), hash) < 0)
+                if (_PyDict_SetItem_KnownHash(mapping, key, one, hash) < 0)
                     goto done;
             } else {
-                newval = PyNumber_Add(oldval, _PyLong_GetOne());
+                newval = PyNumber_Add(oldval, one);
                 if (newval == NULL)
                     goto done;
                 if (_PyDict_SetItem_KnownHash(mapping, key, newval, hash) < 0)
@@ -2341,8 +2343,6 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
         if (bound_get == NULL)
             goto done;

-        PyObject *zero = _PyLong_GetZero();  // borrowed reference
-        PyObject *one = _PyLong_GetOne();  // borrowed reference
         while (1) {
             key = PyIter_Next(it);
             if (key == NULL)

[vstinner]

Why did you put _PyLong_GetOne() inside the loop for the fast path and outside the loop for the slow path?

Oh, I didn't notice that the first part was also a loop. I wrote PR 23008 to move the call out of the loop.

I tried to avoid calling the function if it's possible that the variable it not used. But here, it's always used, so it's relevant to move the loop invariant out of the loop ;-)

[vstinner]

New changeset c310185 by Victor Stinner in branch 'master':
bpo-42161: Remove private _PyLong_Zero and _PyLong_One (GH-23003)
c310185

[vstinner]

New changeset 35b95aa by Victor Stinner in branch 'master':
bpo-42161: Micro-optimize _collections._count_elements() (GH-23008)
35b95aa

[vstinner]

Thanks Raymond, I fixed the code.

I close the issue, I removed _PyLong_Zero and _PyLong_One variables.