classification
Title: Small ints aren't always cached properly
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brandtbucher Nosy List: brandtbucher, mark.dickinson, rhettinger
Priority: normal Keywords: patch

Created on 2022-01-12 22:50 by brandtbucher, last changed 2022-01-16 16:07 by mark.dickinson. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 30583 merged brandtbucher, 2022-01-13 19:49
Messages (6)
msg410437 - (view) Author: Brandt Bucher (brandtbucher) * (Python committer) Date: 2022-01-12 22:50
To my surprise, it seems that it's possible to create "small" integers that should live in _PyLong_SMALL_INTS, but don't. Here are two examples I've found:

>>> import decimal
>>> i = int(decimal.Decimal(42))  # Modules/_decimal/_decimal.c:dec_as_long
>>> i
42
>>> i is 42
<stdin>:1: SyntaxWarning: "is" with a literal. Did you mean "=="?
False
>>> i = int.from_bytes(bytes([42]))  # Objects/longobject.c:_PyLong_FromByteArray
>>> i
42
>>> i is 42
<stdin>:1: SyntaxWarning: "is" with a literal. Did you mean "=="?
False

I'm not entirely sure if this is "allowed" or not, but in any case it seems beneficial to reach into the small ints here (provided it doesn't hurt performance, of course).

I'm testing out simple fixes for both of these now.
msg410468 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2022-01-13 08:36
I don't *think* we currently rely on small integers being cached anywhere in the implementation (and neither do we guarantee anywhere in the docs that small integers will be cached), so as far as I can tell these omissions shouldn't lead to user-visible bugs.

I agree that these cases should be fixed, though.
msg410469 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2022-01-13 08:43
Hmm. This sort of thing is a little dodgy, though (despite the comment that it's "okay"): 

https://github.com/python/cpython/blob/1de60155d5d01be2924e72fb68dd13d4fd00acd7/Modules/mathmodule.c#L939

    PyObject *zero = _PyLong_GetZero();  // borrowed ref
    for (i = 1; i < nargs; i++) {
        /* --- 8< --- snipped code */
        if (res == zero) {
            /* Fast path: just check arguments.
               It is okay to use identity comparison here. */
            Py_DECREF(x);
            continue;
        }
        /* --- 8< --- snipped code*/
    }
msg410472 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2022-01-13 08:53
And there are some similar things going on in rangeobject.c.

https://github.com/python/cpython/blob/1de60155d5d01be2924e72fb68dd13d4fd00acd7/Objects/rangeobject.c#L598

        if (r->step == _PyLong_GetOne()) {
            return idx;
        }

Again, technically "okay", since it's only a fast path and the slow path that follows will still do the right thing with a 1 that's not "the" 1, but it feels fragile.
msg410514 - (view) Author: Brandt Bucher (brandtbucher) * (Python committer) Date: 2022-01-13 19:52
The attached PR doesn't seem to have any impact on Decimal performance (non-optimized, non-debug build on a fairly quiet laptop):

main:

Convert 262,000 Decimals to "small" ints: 31.7 ms +- 5.3 ms
Convert 256,000 Decimals to 1-digit ints: 29.9 ms +- 3.1 ms
Convert 256,000 Decimals to 2-digit ints: 30.4 ms +- 2.8 ms
Convert 256,000 Decimals to 3-digit ints: 31.2 ms +- 3.1 ms

patched:

Convert 262,000 Decimals to "small" ints: 30.9 ms +- 4.0 ms
Convert 256,000 Decimals to 1-digit ints: 29.5 ms +- 3.0 ms
Convert 256,000 Decimals to 1-digit ints: 30.5 ms +- 2.5 ms
Convert 256,000 Decimals to 1-digit ints: 31.0 ms +- 2.3 ms
msg410698 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2022-01-16 16:06
New changeset 5cd9a162cd02a3d0f1b0a182d80feeb17439e84f by Brandt Bucher in branch 'main':
bpo-46361: Fix "small" `int` caching (GH-30583)
https://github.com/python/cpython/commit/5cd9a162cd02a3d0f1b0a182d80feeb17439e84f
History
Date User Action Args
2022-01-16 16:07:12mark.dickinsonsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2022-01-16 16:06:45mark.dickinsonsetmessages: + msg410698
2022-01-13 19:52:16brandtbuchersetmessages: + msg410514
2022-01-13 19:49:29brandtbuchersetkeywords: + patch
stage: patch review
pull_requests: + pull_request28781
2022-01-13 08:53:24mark.dickinsonsetmessages: + msg410472
2022-01-13 08:43:53mark.dickinsonsetmessages: + msg410469
2022-01-13 08:36:13mark.dickinsonsetnosy: + mark.dickinson
messages: + msg410468
2022-01-13 00:01:38rhettingersetnosy: + rhettinger
2022-01-12 22:50:40brandtbuchercreate