Repository navigation
_decimal: pi benchmark 31-45% slower compared to 3.9 #114682
Description
Activity
Can you try
--disable-gilwith current main?The figures are almost the same, around 30% (enable) and 43% (disable).
The base slowdown of 30% is of course due to the module state changes. For comparison, and to see that the i5-8600K is not inherently underperforming with threads:
The ideal situation is implemented in libmpdec++: It has a C++11 thread local context, which is also called for every inline operation. The C++11 TLS adds just 4% overhead compared to pure C (where the context is passed directly to the functions).
This of course would be quite difficult to achieve in CPython. But the thread local context may not even be the primary culprit. In Martin von Loewis' first module state implementation the basic slowdown for _decimal was 20%, and a good part of that was due to the heap types.
Reacted by sunmy2019The ideal situation is implemented in libmpdec++: It has a C++11 thread local context, which is also called for every inline operation. The C++11 TLS adds just 4% overhead compared to pure C (where the context is passed directly to the functions).
This of course would be quite difficult to achieve in CPython.
The wording was not ideal: The key point is that C++ achieves this without a cached context, which is of course now in the module state.
@encukou, any thoughts on this?
PyModule_GetState()PyType_GetModuleByDef()seems to be expensive for heavy use.
It would be nice if a heap type could own the module state's address at its creation time to recover more than 10%.Reacted by Erlend E. AaslandPyModule_GetState()viaPyType_GetModuleByDef()seems to be expensive for heavy use. It would be nice if a heap type could own the module state's address at its creation time to recover more than 10%._PyModule_GetState()is used, notPyModule_GetState(); I'd be surprised if this is the culprit:cpython/Include/internal/pycore_moduleobject.h
Lines 32 to 35 in 1aec064
OTOH,
PyType_GetModuleByDef()can be more expensive. Storing the state pointer in the type struct is a possibility.@ericsnowcurrently Can a module state be accessed correctly with the following?
_Py_thread_local module_state *thread_local_state; // set at the exec phaseIf not invalid, it will be useful for the function which currently has no way to get the module state. A
_Py_thread_localpointer may not be so fast to access, but it can also bypass a state struct?Multiple interpreters can run in the same thread, so a thread-local may be ambiguous in that case.
As to storing a reference to the module state on the type object, that shouldn't be necessary. Every heap type as a
ht_modulefield that points to the module, accessible viaPyType_GetModule(). From there you callPyModule_GetState(). In fact, we already havePyType_GetModuleState()that combines those two calls. That's fairly efficient, though obviously not as efficient as a static global.The only caveat is that
PyType_GetModule()(and thusPyType_GetModuleState()) operates relative to the given type. Thus subclasses make things trickier. IIRC, when they're involved, you end up having to walk the MRO one way or another, which is whatPyType_GetModuleByDef()does.FWIW, the common case where
PyType_GetModuleByDef()is used is either in "tp slot" functions or intermediate static functions that aren't doing the right thing. In both cases the problem lies in throwing away the expensive information we already have. For tp slot functions the API doesn't pass the module in, even though it could. We're stuck with backward compatibility issues there. For intermediate static functions, I expect it's all too common to figure out the module (or module state) at some "leaf node" function but not pass it through the intermediate functions. Thus the relatively expensivePyType_GetModuleByDef()gets called more than it needs to be.I suppose another problem case is with callbacks that don't provide a way to pass the module through. I'm not sure that's a common issue though.
Back to the _decimal module, I haven't looked at what usage patterns are resulting in such a significant slowdown.
Thus subclasses make things trickier. IIRC, when they're involved, you end up having to walk the MRO one way or another, which is what
PyType_GetModuleByDef()does.Yes; heap types that are not base classes can go the fast route via
PyType_GetModuleState()(and_PyType_GetModuleState()).The
floatobject gets the luxury of its own freelist and avoids the slowPyObject_GC_*functions altogether. TheDecimalconstructor is a pure function (the context is only used for recording errors) andDecimalsare immutable, just likefloats.It is likely that 15 percentage points can be shaved off just by doing the same as in
floatobject.cand then focus on the thread local context (indeed,PyType_GetModuleByDef()is called at dozens of places).Reacted by neoneneAFAICS, we can trim away a lot of the
PyType_GetModuleByDefcalls. For example, a lot of them are happening in methods; we can use theMETH_METHODcalling convention on practically all of these, so we can access the module state using_PyType_GetModuleState:cpython/Include/internal/pycore_typeobject.h
Lines 100 to 109 in 15f6f04
Also (but not performance relevant), the
find_state_left_or_right()calls in thePy_tp_richcompareslots can be replaced withget_module_state_by_def()calls.Reacted by neonene and Petr Viktorin23 remaining items
I'm getting a smaller speedup by using
PyType_GetModuleState()in the number methods and a larger one by usingPyObject_Newfor the decimal constructor.The
_threadmoduleitself usesPyObject_Newinnew_thread_handle.@ericsnowcurrently, is there a reason why
PyObject_Newcannot be used for the immutableDecimaltype?I withdraw myself from this discussion; I am no longer working on this issue.
That is an extraordinary comment from Mr. Stinner. It is entirely irrelevant to this issue
Well, same than Erlend for me: I unsubscribe. I'm not interested to work on an issue with such tone in the discussion.
Reacted by Erlend E. Aasland, Maxim Martynov and Gregory P. Smithis there a reason why
PyObject_Newcannot be used for the immutableDecimaltype?I'm not aware of any reason. It isn't a GC type, right?
I'm not aware of any reason. It isn't a GC type, right?
Yes, up to
3.12it wasn't a GC type. It has been made a GC type inmainduring the module state changes.I now found some explanations in PEP-630, but I still do not understand it:
Instances of heap types hold a reference to their type. This ensures that the type isn't destroyed before all its instances are, but may result in reference cycles that need to be broken by the garbage collector.How would one create a cycle?
>>> x = Decimal("3.222") >>> x.y = x Traceback (most recent call last): File "<stdin>", line 1, in <module> x.y = x ^^^ AttributeError: 'decimal.Decimal' object has no attribute 'y' and no __dict__ for setting new attributes>>> x = Decimal("3.222") >>> t = type(x) >>> t.y = x Traceback (most recent call last): File "<stdin>", line 1, in <module> t.y = x ^^^ TypeError: cannot set 'y' attribute of immutable type 'decimal.Decimal'>>> x.adjusted = 10 Traceback (most recent call last): File "<stdin>", line 1, in <module> x.adjusted = 10 ^^^^^^^^^^ AttributeError: 'decimal.Decimal' object attribute 'adjusted' is read-onlyYes, up to 3.12 it wasn't a GC type. It has been made a GC type in main during the module state changes.
Ah, there are definitely some subtleties involved with GC types, especially when switching from a non-GC type. I don't recall the specifics, but I'm sure @encukou does.
It might be more interesting to start with static (non-GC) types: these are “immortal” (statically allocated), but they do have a bunch of dynamically allocated, interpreter-specific data (e.g. subclasses, weakrefs). Getting this data properly cleaned up requires rather special hacks, appropriate for core types that really are created/destroyed with the interpreter itself, rather than extensions loaded on demand (see code around
static_types[]inObjects/object.cfor details).
On the other hand, GC types (heap types) use reference counting like any other Python object. Not saying that getting rid of all the assumptions of static-ness/immortality is/was smooth, but at least the model is much clearer: we have a good idea of what the details should be, even though getting there is a bumpy road full of backcompat issues.
Instances of heap types hold a reference to their type.
This ensures that the type isn't destroyed before all its instances are,
but may result in reference cycles that need to be broken by the
garbage collector.This is a generic statement. A class with no
__dict__nor user-settable attributes probably can't create a reference cycle.
While we could omit the instance→type reference only in cases where one can prove this, I don't think such a special case would be worth it.How would one create a cycle?
PEP 573 and
type_traverse()says:Usually, creating a class with ht_module set will create a reference cycle involving the class and the module.Lines 5526 to 5534 in 20eaf4d
To make
Decimaltype a non-GC heap type, at least themodulearg needs to beNULLinPyType_FromMetaclass(), likethread_handle_typein_threadmodule?I agree that
PyObject_GC_*()has some overhead, but I'm not sure how to hold a module safely outsideDecimaltype.FYI - skrah has been banned from python spaces, quoting details from my message posted on a duplicate-ish issue:
#114922 (comment)"""
Procedural comment: For all involved on this issue, @skrah was banned from Python spaces per a Conduct Working Group recommendation to the Steering Council in 2020. A configuration oversight on our part in 2020 made that not so on Github. The intended ban of @skrah was reinstated on 2024-02-22. Our apologies to all for the consequences of this mixup (it was the first core dev ban done on our modern systems, we now understand how and why mistakes were made).Stefan was informed of this privately via email and re-offered the opportunity to apologize for past actions and to accept our code of conduct going forwards per the original 2020 ban's terms and notification. We have not received a reply.
I'm posting this here as we view replying in the same forum where issues recently surfaced a reasonable way to notify past, present, and potential future participants of an incident response.
This says nothing for or against the validity of this issue. A new issue should be opened if people wish to proceed. I'm locking the comments on this one. For information on what the Github concept of a ban means, read the Github org ban docs.
-- The Python Steering Council, 2024 edition
""- locked as too heated and limited conversation to collaborators
on Mar 12, 2024
Bug report
Bug description:
The pi benchmark in
Modules/_decimal/tests/bench.pyis up to 31% slower with--enable-giland up to 45% slower with--disable-gilcompared to Python 3.9 as the baseline.Change the number of iterations to
100000for more stable benchmarks.This is heavily OS and hardware dependent. The worst case cited above was observed on a six core i5-8600K.
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Linked PRs
METH_METHODcalling convention in_decimal#115196_decimal#115401