Skip to content

bpo-38858: Small integer per interpreter #17315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ PyAPI_FUNC(int) _Py_IsLocaleCoercionTarget(const char *ctype_loc);

extern PyStatus _PyUnicode_Init(void);
extern int _PyStructSequence_Init(void);
extern int _PyLong_Init(void);
extern int _PyLong_Init(PyThreadState *tstate);
extern PyStatus _PyFaulthandler_Init(int enable);
extern int _PyTraceMalloc_Init(int enable);
extern PyObject * _PyBuiltin_Init(PyThreadState *tstate);
Expand Down Expand Up @@ -76,7 +76,7 @@ extern void _PyGC_Fini(PyThreadState *tstate);
extern void _PyType_Fini(void);
extern void _Py_HashRandomization_Fini(void);
extern void _PyUnicode_Fini(PyThreadState *tstate);
extern void _PyLong_Fini(void);
extern void _PyLong_Fini(PyThreadState *tstate);
extern void _PyFaulthandler_Fini(void);
extern void _PyHash_Fini(void);
extern void _PyTraceMalloc_Fini(void);
Expand Down
12 changes: 12 additions & 0 deletions Include/internal/pycore_pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ struct _ceval_runtime_state {

typedef PyObject* (*_PyFrameEvalFunction)(struct _frame *, int);

#define _PY_NSMALLPOSINTS 257
#define _PY_NSMALLNEGINTS 5

// The PyInterpreterState typedef is in Include/pystate.h.
struct _is {

Expand Down Expand Up @@ -139,6 +142,15 @@ struct _is {
int atbol;
} listnode;
} parser;

#if _PY_NSMALLNEGINTS + _PY_NSMALLPOSINTS > 0
/* Small integers are preallocated in this array so that they
can be shared.
The integers that are preallocated are those in the range
-_PY_NSMALLNEGINTS (inclusive) to _PY_NSMALLPOSINTS (not inclusive).
*/
PyLongObject* small_ints[_PY_NSMALLNEGINTS + _PY_NSMALLPOSINTS];
#endif
};

PyAPI_FUNC(struct _is*) _PyInterpreterState_LookUpID(PY_INT64_T);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Each Python subinterpreter now has its own "small integer singletons":
numbers in [-5; 257] range. It is no longer possible to change the number of
small integers at build time by overriding ``NSMALLNEGINTS`` and
``NSMALLPOSINTS`` macros: macros should now be modified manually in
``pycore_pystate.h`` header file.
59 changes: 28 additions & 31 deletions Objects/longobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/* XXX The functional organization of this file is terrible */

#include "Python.h"
#include "pycore_pystate.h" /* _Py_IsMainInterpreter() */
#include "longintrepr.h"

#include <float.h>
Expand All @@ -15,12 +16,8 @@ class int "PyObject *" "&PyLong_Type"
[clinic start generated code]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=ec0275e3422a36e3]*/

#ifndef NSMALLPOSINTS
#define NSMALLPOSINTS 257
#endif
#ifndef NSMALLNEGINTS
#define NSMALLNEGINTS 5
#endif
#define NSMALLPOSINTS _PY_NSMALLPOSINTS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is effectively a backward-incompatible change (though it only affects folks building their own python binary). So there should be a note in the whatsnew doc (porting section) indicating what folks must do if they want the previous behavior.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really expect that anyone would recompile their Python with different NSMALLPOSINTS and NSMALLNEGINTS constants? Disabling these singletons is likely to make Python faster. I'm not sure about increasing the number of singletons. I don't expect any significant performance difference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completed the NEWS entry. I prefer to not document it in the What's New in Python 3.9 document. IMHO it's too low-level and obscure.

#define NSMALLNEGINTS _PY_NSMALLNEGINTS

_Py_IDENTIFIER(little);
_Py_IDENTIFIER(big);
Expand All @@ -35,13 +32,6 @@ PyObject *_PyLong_Zero = NULL;
PyObject *_PyLong_One = NULL;

#if NSMALLNEGINTS + NSMALLPOSINTS > 0
/* Small integers are preallocated in this array so that they
can be shared.
The integers that are preallocated are those in the range
-NSMALLNEGINTS (inclusive) to NSMALLPOSINTS (not inclusive).
*/
static PyLongObject* small_ints[NSMALLNEGINTS + NSMALLPOSINTS] = {0};

#define IS_SMALL_INT(ival) (-NSMALLNEGINTS <= (ival) && (ival) < NSMALLPOSINTS)
#define IS_SMALL_UINT(ival) ((ival) < NSMALLPOSINTS)

Expand All @@ -53,7 +43,8 @@ static PyObject *
get_small_int(sdigit ival)
{
assert(IS_SMALL_INT(ival));
PyObject *v = (PyObject*)small_ints[ival + NSMALLNEGINTS];
PyThreadState *tstate = _PyThreadState_GET();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this was simpler than adding a tstate arg to get_small_int()...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I began by passing tstate to get_small_int() but I don't see any benefit, since no caller requires tstate, or already have tstate, currently. For now, it seems simpler to only get tstate inside get_small_int(). I don't expect any performance issue for now.

PyObject *v = (PyObject*)tstate->interp->small_ints[ival + NSMALLNEGINTS];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a static value any more, so is it possible to run into problems during interpreter/runtime finalization?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure of what you mean. A subinterpreter is supposed to not leak any object to other interpreters, right? If an subinterpreter object survives after the subinterpreter is destroyed, it's a bug, no?

Py_INCREF(v);
#ifdef COUNT_ALLOCS
if (ival >= 0)
Expand Down Expand Up @@ -5782,7 +5773,7 @@ PyLong_GetInfo(void)
}

int
_PyLong_Init(void)
_PyLong_Init(PyThreadState *tstate)
{
#if NSMALLNEGINTS + NSMALLPOSINTS > 0
for (Py_ssize_t i=0; i < NSMALLNEGINTS + NSMALLPOSINTS; i++) {
Expand All @@ -5797,37 +5788,43 @@ _PyLong_Init(void)
Py_SIZE(v) = size;
v->ob_digit[0] = (digit)abs(ival);

small_ints[i] = v;
tstate->interp->small_ints[i] = v;
}
#endif
_PyLong_Zero = PyLong_FromLong(0);
if (_PyLong_Zero == NULL) {
return 0;
}

_PyLong_One = PyLong_FromLong(1);
if (_PyLong_One == NULL) {
return 0;
}
if (_Py_IsMainInterpreter(tstate)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yuck!

It would be great to have a comment here about how we would like to get rid of this special case. Bonus points if you open an issue for that and link to it in the comment. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no plan to remove _PyLong_One and _PyLong_Zero yet. It seems like there are too many things to do for subinterpreters, I am not excited by long TODO lists. They depress me, as if I will never be able to go to the end. I like to make tiny incremental changes :-)

In general, searching for "_Py_IsMainInterpreter" became a nice hint for "subinterpreters TODO".

_PyLong_Zero = PyLong_FromLong(0);
if (_PyLong_Zero == NULL) {
return 0;
}

/* initialize int_info */
if (Int_InfoType.tp_name == NULL) {
if (PyStructSequence_InitType2(&Int_InfoType, &int_info_desc) < 0) {
_PyLong_One = PyLong_FromLong(1);
if (_PyLong_One == NULL) {
return 0;
}

/* initialize int_info */
if (Int_InfoType.tp_name == NULL) {
if (PyStructSequence_InitType2(&Int_InfoType, &int_info_desc) < 0) {
return 0;
}
}
}

return 1;
}

void
_PyLong_Fini(void)
_PyLong_Fini(PyThreadState *tstate)
{
Py_CLEAR(_PyLong_One);
Py_CLEAR(_PyLong_Zero);
if (_Py_IsMainInterpreter(tstate)) {
Py_CLEAR(_PyLong_One);
Py_CLEAR(_PyLong_Zero);
}

#if NSMALLNEGINTS + NSMALLPOSINTS > 0
for (Py_ssize_t i = 0; i < NSMALLNEGINTS + NSMALLPOSINTS; i++) {
Py_CLEAR(small_ints[i]);
Py_CLEAR(tstate->interp->small_ints[i]);
}
#endif
}
13 changes: 9 additions & 4 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -576,10 +576,11 @@ pycore_init_types(PyThreadState *tstate)
if (_PyStatus_EXCEPTION(status)) {
return status;
}
}

if (!_PyLong_Init()) {
return _PyStatus_ERR("can't init longs");
}

if (!_PyLong_Init(tstate)) {
return _PyStatus_ERR("can't init longs");
}

if (is_main_interp) {
Expand Down Expand Up @@ -1251,7 +1252,11 @@ finalize_interp_types(PyThreadState *tstate, int is_main_interp)
_PyList_Fini();
_PySet_Fini();
_PyBytes_Fini();
_PyLong_Fini();
}

_PyLong_Fini(tstate);

if (is_main_interp) {
_PyFloat_Fini();
_PyDict_Fini();
_PySlice_Fini();
Expand Down