Repository navigation
decimal: Use Argument Clinic #73487
Description
Activity
I'm currently working on the isuse bpo-29259: "Add tp_fastcall to PyTypeObject: support FASTCALL calling convention for all callable objects". I used bm_telco of the performance benchmark suite to check which functions still require to create a temporary tuple for positional arguments and a temporary dict for keyword arguments. I found 3 remaining functions which have an impact on the result of the benchmark:
- print(): optimized by the issue bpo-29296
- _struct.unpack(): I just created the issue bpo-29300 "Modify the _struct module to use FASTCALL and Argument Clinic"
- _decimal.Decimal.quantize()
I would like to know if Stephan would be ok to modify the _decimal module to use FASTCALL. I know that recently he reverted changes to keep the same code base on Python 3.5, 3.6 and 3.7.
With 4 changes (tp_fastcall bpo-29259, print bpo-29296, unpack bpo-29300 and this issue), bm_telco becomes 22% faster which is not negligible!
20.9 ms +- 0.5 ms => 16.4 ms +- 0.5 ms
Attached decimal.patch patch is the strict minimum change to optimize bm_telco, but I would prefer to change all _decimal functions and methods using METH_VARARGS and METH_VARARGS|METH_KEYWORDS to convert them to METH_FASTCALL.
The best would be to use Argument Clinic. AC exists since Python 3.5, so it should be possible to keep the same code base on Python 3.5-3.7, only generated code would be different.
Oh wait, I'm not sure that attached patch has a significant impact on performances. It seems like the speedup mostly comes from the print patch:
http://bugs.python.org/issue29296#msg285668But well, it is still interesting to use METH_FASTCALL in decimal ;-)
This should not go in without Stephan Krah's approval. This code is finely tuned and carefully arranged.
Raymond Hettinger: "This should not go in without Stephan Krah's approval. This code is finely tuned and carefully arrang"
Sure, that's why I opened this issue.
Serhiy Storchaka: "See msg207652 in bpo-20177."
Oh, I didn't know that Stefan Krah already gave his opinion.
Since the performance impact is unclear, I prefer to close this issue. I will come back if argument parsing is more clearly identified as a performance bottleneck.
Hello, I'm back! :-)
I almost abandonned my the tp_fastcall change (bpo-29259).
print() now uses FASTCALL (bpo-29296), it already made bm_telco faster:
https://speed.python.org/timeline/#/?exe=5&ben=telco&env=1&revs=50&equid=off&quarts=on&extr=on
(see the speedup around January 16 at the right)For the unpack module, I'm still working on my patch in the issue bpo-29300, but it doesn't seem to have a significant impact on bm_telco.
So the remaining question is the usage of FASTCALL or Argument Clinic in the _decimal module.
Since the performance impact is unclear, I prefer to close this issue. I will come back if argument parsing is more clearly identified as a performance bottleneck.
I ran a new benchmark and FASTCALL makes bm_telco benchmark 1.11x faster:
Median +- std dev: [ref] 19.4 ms +- 0.7 ms -> [decimal-2.patch] 17.5 ms +- 0.6 ms: 1.11x faster (-10%)
So I decided to reopen the issue.
Stefan: what do you think of using Argument Clinic and/or FASTCALL in _decimal? Is "bm_telco 1.11x faster" significant enough for you to justify such change?
"The best would be to use Argument Clinic. AC exists since Python 3.5, so it should be possible to keep the same code base on Python 3.5-3.7, only generated code would be different."
I would dissuade from direct using of FASTCALL, but Argument Clinic is enough reliable. I would recommend to use Argument Clinic unless Stefan supports external fork of the decimal module and is limited by public API.
Oops, I forgot to attach decimal-2.patch, here you have.
I would dissuade from direct using of FASTCALL, but Argument Clinic is enough reliable.
I wrote decimal-2.patch in a few minutes, when I was trying to find all functions still calling _PyStack_AsTuple() in my tp_fast{new,init,call} fork. I just wrote it to identify which parts of CPython can be optimized to make bm_telco faster.
I agree that Argument Clinic should be preferred over using directly METH_FASTCALL, especially for the _decimal module: Stefan already wrote that he wants to have the same code base on Python 3.5, 3.6 and 3.7.
I would recommend to use Argument Clinic unless Stefan supports external fork of the decimal module and is limited by public API.
What is the status of Argument Clinic? Is it something "public" or not?
The status of _decimal is also unclear to me. Is it part of CPython or not? :-) I know that there is also a "backported" module for Python 2:
https://pypi.python.org/pypi/cdecimal
http://www.bytereef.org/mpdecimal/index.htmlIMHO it's a great tool. Maybe it's time to make it usable outside CPython in Python 3.7? Or maybe we should wait until the remaining missing features are implemented? Issues bpo-20291 and bpo-29299 for example.
I'm now waiting for Stefan's feedback ;-)
skrah commented
on Jan 27, 2017 skrahmannequinMannequinI'll take a look when I have the opportunity.
AC will not happen: It makes the module too large and unreadable.
What is the status of Argument Clinic? Is it something "public" or not?
No, it is for for internal CPython use only. It lacks some features (support
of var-positional and var-keyword parameters, optional parameters without
default value), the syntax of positional-only parameters is not officially
accepted still, and future optimizations can require incompatible changes.Only when all CPython builtins and extensions be converted to Argument Clinic,
PEP-457 (or an alternative) be accepted, Argument Clinic issues be resolved,
we could say it stable enough. For now even Argument Clinic tests are broken.46 remaining items
- added a commit that references this issue
on Aug 22, 2025 With #138015 - conversion will be finished.
Some things that left out so far:
- conversion of getters/setters (this stuff is undocumented so far)
- use defining_class AC converter
- something else?
I would appreciate feedback.
The pt.2 shows slight performance boost, I'm not sure it worth:
Benchmark ref patch Decimal(123).to_integral_value() 605 ns 593 ns: 1.02x faster Details
diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index de7200af8c..0c5bd43361 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -4042,6 +4042,7 @@ _decimal_Decimal_as_integer_ratio_impl(PyObject *self) /*[clinic input] _decimal.Decimal.to_integral_value + cls: defining_class rounding: object = None context: object = None @@ -4053,15 +4054,16 @@ rounding mode of the current default context is used. [clinic start generated code]*/ static PyObject * -_decimal_Decimal_to_integral_value_impl(PyObject *self, PyObject *rounding, +_decimal_Decimal_to_integral_value_impl(PyObject *self, PyTypeObject *cls, + PyObject *rounding, PyObject *context) -/*[clinic end generated code: output=7301465765f48b6b input=04e2312d5ed19f77]*/ +/*[clinic end generated code: output=23047d848ef84db1 input=85aa9499a21ea8d7]*/ { PyObject *result; uint32_t status = 0; mpd_context_t workctx; - decimal_state *state = get_module_state_by_def(Py_TYPE(self)); + decimal_state *state = PyType_GetModuleState(cls); CONTEXT_CHECK_VA(state, context); workctx = *CTX(context);
Thank you!
IMO,
defining_classis best practice, for methods that can use it.PyType_GetModuleStateis a lookup that can be avoided.I don't think we need AC for getters & setters.
- added a commit that references this issue
on Aug 28, 2025 Ok, then #138221 should finish conversion.
Yes, I think so.
Reacted by Victor Stinner, Adam Turner and Erlend E. Aasland
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:
bugs.python.org fields:
Linked PRs
_decimalto use Argument Clinic (part 1) #137606_decimalto use Argument Clinic (part 7) #138221