From 815280eb160af637e1347213659f9236adf78f80 Mon Sep 17 00:00:00 2001 From: Stefan Krah Date: Sat, 29 Feb 2020 19:43:42 +0100 Subject: [PATCH] bpo-39794: Add --without-decimal-contextvar (#18702) --- Doc/library/decimal.rst | 15 +- Lib/_pydecimal.py | 8 +- Lib/test/test_asyncio/test_context.py | 1 + .../2020-02-29-19-17-39.bpo-39794.7VjatS.rst | 2 + Modules/_decimal/_decimal.c | 169 ++++++++++++++++-- .../_decimal/tests/runall-memorydebugger.sh | 36 ++-- PC/pyconfig.h | 4 + aclocal.m4 | 4 +- configure | 26 +++ configure.ac | 15 ++ pyconfig.h.in | 4 + 11 files changed, 248 insertions(+), 36 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-02-29-19-17-39.bpo-39794.7VjatS.rst diff --git a/Doc/library/decimal.rst b/Doc/library/decimal.rst index 4e640cc6959..9e317816abb 100644 --- a/Doc/library/decimal.rst +++ b/Doc/library/decimal.rst @@ -1475,9 +1475,18 @@ are also included in the pure Python version for compatibility. .. data:: HAVE_THREADS - The default value is ``True``. If Python is compiled without threads, the - C version automatically disables the expensive thread local context - machinery. In this case, the value is ``False``. + The value is ``True``. Deprecated, because Python now always has threads. + +.. deprecated:: 3.9 + +.. data:: HAVE_CONTEXTVAR + + The default value is ``True``. If Python is compiled ``--without-decimal-contextvar``, + the C version uses a thread-local rather than a coroutine-local context and the value + is ``False``. This is slightly faster in some nested context scenarios. + +.. versionadded:: 3.9 + Rounding modes -------------- diff --git a/Lib/_pydecimal.py b/Lib/_pydecimal.py index c14d8ca86a1..ab989e5206a 100644 --- a/Lib/_pydecimal.py +++ b/Lib/_pydecimal.py @@ -140,8 +140,11 @@ __all__ = [ # Limits for the C version for compatibility 'MAX_PREC', 'MAX_EMAX', 'MIN_EMIN', 'MIN_ETINY', - # C version: compile time choice that enables the thread local context - 'HAVE_THREADS' + # C version: compile time choice that enables the thread local context (deprecated, now always true) + 'HAVE_THREADS', + + # C version: compile time choice that enables the coroutine local context + 'HAVE_CONTEXTVAR' ] __xname__ = __name__ # sys.modules lookup (--without-threads) @@ -172,6 +175,7 @@ ROUND_05UP = 'ROUND_05UP' # Compatibility with the C version HAVE_THREADS = True +HAVE_CONTEXTVAR = True if sys.maxsize == 2**63-1: MAX_PREC = 999999999999999999 MAX_EMAX = 999999999999999999 diff --git a/Lib/test/test_asyncio/test_context.py b/Lib/test/test_asyncio/test_context.py index c309faa9006..63b1eb320ce 100644 --- a/Lib/test/test_asyncio/test_context.py +++ b/Lib/test/test_asyncio/test_context.py @@ -7,6 +7,7 @@ def tearDownModule(): asyncio.set_event_loop_policy(None) +@unittest.skipUnless(decimal.HAVE_CONTEXTVAR, "decimal is built with a thread-local context") class DecimalContextTest(unittest.TestCase): def test_asyncio_task_decimal_context(self): diff --git a/Misc/NEWS.d/next/Library/2020-02-29-19-17-39.bpo-39794.7VjatS.rst b/Misc/NEWS.d/next/Library/2020-02-29-19-17-39.bpo-39794.7VjatS.rst new file mode 100644 index 00000000000..b2a4726068a --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-02-29-19-17-39.bpo-39794.7VjatS.rst @@ -0,0 +1,2 @@ +Add --without-decimal-contextvar build option. This enables a thread-local +rather than a coroutine local context. diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index e2f6ea17f89..617941bc5ec 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -122,7 +122,14 @@ incr_false(void) } +#ifndef WITH_DECIMAL_CONTEXTVAR +/* Key for thread state dictionary */ +static PyObject *tls_context_key = NULL; +/* Invariant: NULL or the most recently accessed thread local context */ +static PyDecContextObject *cached_context = NULL; +#else static PyObject *current_context_var; +#endif /* Template for creating new thread contexts, calling Context() without * arguments and initializing the module_context on first access. */ @@ -1217,6 +1224,12 @@ context_new(PyTypeObject *type, PyObject *args UNUSED, PyObject *kwds UNUSED) static void context_dealloc(PyDecContextObject *self) { +#ifndef WITH_DECIMAL_CONTEXTVAR + if (self == cached_context) { + cached_context = NULL; + } +#endif + Py_XDECREF(self->traps); Py_XDECREF(self->flags); Py_TYPE(self)->tp_free(self); @@ -1491,6 +1504,134 @@ static PyGetSetDef context_getsets [] = * operation. */ +#ifndef WITH_DECIMAL_CONTEXTVAR +/* Get the context from the thread state dictionary. */ +static PyObject * +current_context_from_dict(void) +{ + PyObject *dict; + PyObject *tl_context; + PyThreadState *tstate; + + dict = PyThreadState_GetDict(); + if (dict == NULL) { + PyErr_SetString(PyExc_RuntimeError, + "cannot get thread state"); + return NULL; + } + + tl_context = PyDict_GetItemWithError(dict, tls_context_key); + if (tl_context != NULL) { + /* We already have a thread local context. */ + CONTEXT_CHECK(tl_context); + } + else { + if (PyErr_Occurred()) { + return NULL; + } + + /* Set up a new thread local context. */ + tl_context = context_copy(default_context_template, NULL); + if (tl_context == NULL) { + return NULL; + } + CTX(tl_context)->status = 0; + + if (PyDict_SetItem(dict, tls_context_key, tl_context) < 0) { + Py_DECREF(tl_context); + return NULL; + } + Py_DECREF(tl_context); + } + + /* Cache the context of the current thread, assuming that it + * will be accessed several times before a thread switch. */ + tstate = PyThreadState_GET(); + if (tstate) { + cached_context = (PyDecContextObject *)tl_context; + cached_context->tstate = tstate; + } + + /* Borrowed reference with refcount==1 */ + return tl_context; +} + +/* Return borrowed reference to thread local context. */ +static PyObject * +current_context(void) +{ + PyThreadState *tstate; + + tstate = PyThreadState_GET(); + if (cached_context && cached_context->tstate == tstate) { + return (PyObject *)cached_context; + } + + return current_context_from_dict(); +} + +/* ctxobj := borrowed reference to the current context */ +#define CURRENT_CONTEXT(ctxobj) \ + ctxobj = current_context(); \ + if (ctxobj == NULL) { \ + return NULL; \ + } + +/* Return a new reference to the current context */ +static PyObject * +PyDec_GetCurrentContext(PyObject *self UNUSED, PyObject *args UNUSED) +{ + PyObject *context; + + context = current_context(); + if (context == NULL) { + return NULL; + } + + Py_INCREF(context); + return context; +} + +/* Set the thread local context to a new context, decrement old reference */ +static PyObject * +PyDec_SetCurrentContext(PyObject *self UNUSED, PyObject *v) +{ + PyObject *dict; + + CONTEXT_CHECK(v); + + dict = PyThreadState_GetDict(); + if (dict == NULL) { + PyErr_SetString(PyExc_RuntimeError, + "cannot get thread state"); + return NULL; + } + + /* If the new context is one of the templates, make a copy. + * This is the current behavior of decimal.py. */ + if (v == default_context_template || + v == basic_context_template || + v == extended_context_template) { + v = context_copy(v, NULL); + if (v == NULL) { + return NULL; + } + CTX(v)->status = 0; + } + else { + Py_INCREF(v); + } + + cached_context = NULL; + if (PyDict_SetItem(dict, tls_context_key, v) < 0) { + Py_DECREF(v); + return NULL; + } + + Py_DECREF(v); + Py_RETURN_NONE; +} +#else static PyObject * init_current_context(void) { @@ -1570,6 +1711,7 @@ PyDec_SetCurrentContext(PyObject *self UNUSED, PyObject *v) Py_RETURN_NONE; } +#endif /* Context manager object for the 'with' statement. The manager * owns one reference to the global (outer) context and one @@ -4388,15 +4530,8 @@ _dec_hash(PyDecObject *v) mpd_ssize_t exp; uint32_t status = 0; mpd_context_t maxctx; - PyObject *context; - context = current_context(); - if (context == NULL) { - return -1; - } - Py_DECREF(context); - if (mpd_isspecial(MPD(v))) { if (mpd_issnan(MPD(v))) { PyErr_SetString(PyExc_TypeError, @@ -5538,11 +5673,6 @@ PyInit__decimal(void) mpd_free = PyMem_Free; mpd_setminalloc(_Py_DEC_MINALLOC); - /* Init context variable */ - current_context_var = PyContextVar_New("decimal_context", NULL); - if (current_context_var == NULL) { - goto error; - } /* Init external C-API functions */ _py_long_multiply = PyLong_Type.tp_as_number->nb_multiply; @@ -5714,6 +5844,15 @@ PyInit__decimal(void) CHECK_INT(PyModule_AddObject(m, "DefaultContext", default_context_template)); +#ifndef WITH_DECIMAL_CONTEXTVAR + ASSIGN_PTR(tls_context_key, PyUnicode_FromString("___DECIMAL_CTX__")); + Py_INCREF(Py_False); + CHECK_INT(PyModule_AddObject(m, "HAVE_CONTEXTVAR", Py_False)); +#else + ASSIGN_PTR(current_context_var, PyContextVar_New("decimal_context", NULL)); + Py_INCREF(Py_True); + CHECK_INT(PyModule_AddObject(m, "HAVE_CONTEXTVAR", Py_True)); +#endif Py_INCREF(Py_True); CHECK_INT(PyModule_AddObject(m, "HAVE_THREADS", Py_True)); @@ -5773,9 +5912,13 @@ error: Py_CLEAR(SignalTuple); /* GCOV_NOT_REACHED */ Py_CLEAR(DecimalTuple); /* GCOV_NOT_REACHED */ Py_CLEAR(default_context_template); /* GCOV_NOT_REACHED */ +#ifndef WITH_DECIMAL_CONTEXTVAR + Py_CLEAR(tls_context_key); /* GCOV_NOT_REACHED */ +#else + Py_CLEAR(current_context_var); /* GCOV_NOT_REACHED */ +#endif Py_CLEAR(basic_context_template); /* GCOV_NOT_REACHED */ Py_CLEAR(extended_context_template); /* GCOV_NOT_REACHED */ - Py_CLEAR(current_context_var); /* GCOV_NOT_REACHED */ Py_CLEAR(m); /* GCOV_NOT_REACHED */ return NULL; /* GCOV_NOT_REACHED */ diff --git a/Modules/_decimal/tests/runall-memorydebugger.sh b/Modules/_decimal/tests/runall-memorydebugger.sh index 29b7723dd50..7f3e5274618 100755 --- a/Modules/_decimal/tests/runall-memorydebugger.sh +++ b/Modules/_decimal/tests/runall-memorydebugger.sh @@ -1,8 +1,8 @@ #!/bin/sh # -# Purpose: test all machine configurations, pydebug, refleaks, release build -# and release build with valgrind. +# Purpose: test with and without contextvar, all machine configurations, pydebug, +# refleaks, release build and release build with valgrind. # # Synopsis: ./runall-memorydebugger.sh [--all-configs64 | --all-configs32] # @@ -57,7 +57,8 @@ print_config () cd .. # test_decimal: refleak, regular and Valgrind tests -for config in $CONFIGS; do +for args in "--without-decimal-contextvar" ""; do + for config in $CONFIGS; do unset PYTHON_DECIMAL_WITH_MACHINE libmpdec_config=$config @@ -69,12 +70,12 @@ for config in $CONFIGS; do fi ############ refleak tests ########### - print_config "refleak tests: config=$config" + print_config "refleak tests: config=$config" $args printf "\nbuilding python ...\n\n" cd ../../ $GMAKE distclean > /dev/null 2>&1 - ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" --with-pydebug > /dev/null 2>&1 + ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" --with-pydebug $args > /dev/null 2>&1 $GMAKE | grep _decimal printf "\n\n# ======================== refleak tests ===========================\n\n" @@ -82,11 +83,11 @@ for config in $CONFIGS; do ############ regular tests ########### - print_config "regular tests: config=$config" + print_config "regular tests: config=$config" $args printf "\nbuilding python ...\n\n" $GMAKE distclean > /dev/null 2>&1 - ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" > /dev/null 2>&1 + ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" $args > /dev/null 2>&1 $GMAKE | grep _decimal printf "\n\n# ======================== regular tests ===========================\n\n" @@ -103,21 +104,23 @@ for config in $CONFIGS; do esac esac - print_config "valgrind tests: config=$config" + print_config "valgrind tests: config=$config" $args printf "\nbuilding python ...\n\n" $GMAKE distclean > /dev/null 2>&1 - ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" --without-pymalloc > /dev/null 2>&1 + ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" --without-pymalloc $args > /dev/null 2>&1 $GMAKE | grep _decimal printf "\n\n# ======================== valgrind tests ===========================\n\n" $valgrind ./python -m test -uall test_decimal cd Modules/_decimal + done done # deccheck cd ../../ -for config in $CONFIGS; do +for args in "--without-decimal-contextvar" ""; do + for config in $CONFIGS; do unset PYTHON_DECIMAL_WITH_MACHINE if [ X"$config" != X"auto" ]; then @@ -126,22 +129,22 @@ for config in $CONFIGS; do fi ############ debug ############ - print_config "deccheck: config=$config --with-pydebug" + print_config "deccheck: config=$config --with-pydebug" $args printf "\nbuilding python ...\n\n" $GMAKE distclean > /dev/null 2>&1 - ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" --with-pydebug > /dev/null 2>&1 + ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" --with-pydebug $args > /dev/null 2>&1 $GMAKE | grep _decimal printf "\n\n# ========================== debug ===========================\n\n" ./python Modules/_decimal/tests/deccheck.py ########### regular ########### - print_config "deccheck: config=$config " + print_config "deccheck: config=$config" $args printf "\nbuilding python ...\n\n" $GMAKE distclean > /dev/null 2>&1 - ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" > /dev/null 2>&1 + ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" $args > /dev/null 2>&1 $GMAKE | grep _decimal printf "\n\n# ======================== regular ===========================\n\n" @@ -157,15 +160,16 @@ for config in $CONFIGS; do esac esac - print_config "valgrind deccheck: config=$config " + print_config "valgrind deccheck: config=$config" $args printf "\nbuilding python ...\n\n" $GMAKE distclean > /dev/null 2>&1 - ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" --without-pymalloc > /dev/null 2>&1 + ./configure CFLAGS="$ADD_CFLAGS" LDFLAGS="$ADD_LDFLAGS" --without-pymalloc $args > /dev/null 2>&1 $GMAKE | grep _decimal printf "\n\n# ======================== valgrind ==========================\n\n" $valgrind ./python Modules/_decimal/tests/deccheck.py + done done diff --git a/PC/pyconfig.h b/PC/pyconfig.h index 424c5be3709..0aa4f95bf8d 100644 --- a/PC/pyconfig.h +++ b/PC/pyconfig.h @@ -465,6 +465,10 @@ Py_NO_ENABLE_SHARED to find out. Also support MS_NO_COREDLL for b/w compat */ (which you can't on SCO ODT 3.0). */ /* #undef SYS_SELECT_WITH_SYS_TIME */ +/* Define if you want build the _decimal module using a coroutine-local rather + than a thread-local context */ +#define WITH_DECIMAL_CONTEXTVAR 1 + /* Define if you want documentation strings in extension modules */ #define WITH_DOC_STRINGS 1 diff --git a/aclocal.m4 b/aclocal.m4 index 85f00dd5fac..f98db73656d 100644 --- a/aclocal.m4 +++ b/aclocal.m4 @@ -1,6 +1,6 @@ -# generated automatically by aclocal 1.16.1 -*- Autoconf -*- +# generated automatically by aclocal 1.15 -*- Autoconf -*- -# Copyright (C) 1996-2018 Free Software Foundation, Inc. +# Copyright (C) 1996-2014 Free Software Foundation, Inc. # This file is free software; the Free Software Foundation # gives unlimited permission to copy and/or distribute it, diff --git a/configure b/configure index 846116e1128..0bffe8b0356 100755 --- a/configure +++ b/configure @@ -826,6 +826,7 @@ with_libs with_system_expat with_system_ffi with_system_libmpdec +with_decimal_contextvar enable_loadable_sqlite_extensions with_tcltk_includes with_tcltk_libs @@ -1548,6 +1549,9 @@ Optional Packages: system-dependent) --with-system-libmpdec build _decimal module using an installed libmpdec library, see Doc/library/decimal.rst (default is no) + --with-decimal-contextvar + build _decimal module using a coroutine-local rather + than a thread-local context (default is yes) --with-tcltk-includes='-I...' override search for Tcl and Tk include files --with-tcltk-libs='-L...' @@ -10496,6 +10500,28 @@ fi { $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_system_libmpdec" >&5 $as_echo "$with_system_libmpdec" >&6; } +# Check whether _decimal should use a coroutine-local or thread-local context +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for --with-decimal-contextvar" >&5 +$as_echo_n "checking for --with-decimal-contextvar... " >&6; } + +# Check whether --with-decimal_contextvar was given. +if test "${with_decimal_contextvar+set}" = set; then : + withval=$with_decimal_contextvar; +else + with_decimal_contextvar="yes" +fi + + +if test "$with_decimal_contextvar" != "no" +then + +$as_echo "#define WITH_DECIMAL_CONTEXTVAR 1" >>confdefs.h + +fi + +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_decimal_contextvar" >&5 +$as_echo "$with_decimal_contextvar" >&6; } + # Check for support for loadable sqlite extensions { $as_echo "$as_me:${as_lineno-$LINENO}: checking for --enable-loadable-sqlite-extensions" >&5 $as_echo_n "checking for --enable-loadable-sqlite-extensions... " >&6; } diff --git a/configure.ac b/configure.ac index 840caf352d1..bdd607390da 100644 --- a/configure.ac +++ b/configure.ac @@ -3050,6 +3050,21 @@ AC_ARG_WITH(system_libmpdec, AC_MSG_RESULT($with_system_libmpdec) +# Check whether _decimal should use a coroutine-local or thread-local context +AC_MSG_CHECKING(for --with-decimal-contextvar) +AC_ARG_WITH(decimal_contextvar, + AS_HELP_STRING([--with-decimal-contextvar], [build _decimal module using a coroutine-local rather than a thread-local context (default is yes)]), + [], + [with_decimal_contextvar="yes"]) + +if test "$with_decimal_contextvar" != "no" +then + AC_DEFINE(WITH_DECIMAL_CONTEXTVAR, 1, + [Define if you want build the _decimal module using a coroutine-local rather than a thread-local context]) +fi + +AC_MSG_RESULT($with_decimal_contextvar) + # Check for support for loadable sqlite extensions AC_MSG_CHECKING(for --enable-loadable-sqlite-extensions) AC_ARG_ENABLE(loadable-sqlite-extensions, diff --git a/pyconfig.h.in b/pyconfig.h.in index b5602134d70..2a72e9ed7fe 100644 --- a/pyconfig.h.in +++ b/pyconfig.h.in @@ -1512,6 +1512,10 @@ /* Define if WINDOW in curses.h offers a field _flags. */ #undef WINDOW_HAS_FLAGS +/* Define if you want build the _decimal module using a coroutine-local rather + than a thread-local context */ +#undef WITH_DECIMAL_CONTEXTVAR + /* Define if you want documentation strings in extension modules */ #undef WITH_DOC_STRINGS