Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
Description
Feature or enhancement
Proposal:
Currently bothPyLong_AsInt32() andPyLong_AsInt64() are usingPyLong_AsNativeBytes(), butPyLong_AsLongAndOverflow()/PyLong_AsLongLongAndOverflow() should be in general - faster. This fact is already used in thePyLong_Export():
Lines 6875 to 6884 in4885ecf
| intoverflow; | |
| #ifSIZEOF_LONG==8 | |
| longvalue=PyLong_AsLongAndOverflow(obj,&overflow); | |
| #else | |
| // Windows has 32-bit long, so use 64-bit long long instead | |
| long longvalue=PyLong_AsLongLongAndOverflow(obj,&overflow); | |
| #endif | |
| Py_BUILD_ASSERT(sizeof(value)==sizeof(int64_t)); | |
| // the function cannot fail since obj is a PyLongObject | |
| assert(!(value==-1&&PyErr_Occurred())); |
I suggest using same code e.g. for PyLong_AsInt64(). Indeed, consider followingMETH_O module function:
staticPyObject*bar(PyObject*Py_UNUSED(module),PyObject*arg){longval;if (!PyLong_AsInt64(arg,&val)) {returnPyBool_FromLong(val!=0); }PyErr_Clear();Py_RETURN_FALSE;}
We have timings:
111: Mean +- std dev: 166 ns +- 1 ns(1<<32)+1: Mean +- std dev: 243 ns +- 1 ns(1<<62)+11: Mean +- std dev: 229 ns +- 7 ns(1<<128)+2: Mean +- std dev: 693 ns +- 11 ns(1<<256)+3: Mean +- std dev: 697 ns +- 13 nsWith the patchedPyLong_AsInt64():
111: Mean +- std dev: 170 ns +- 1 ns(1<<32)+1: Mean +- std dev: 173 ns +- 1 ns(1<<62)+11: Mean +- std dev: 176 ns +- 5 ns(1<<128)+2: Mean +- std dev: 614 ns +- 7 ns(1<<256)+3: Mean +- std dev: 614 ns +- 6 nsUnfortunately, overflow exception adds some extra overhead we can't avoid. Things are better for following method:
staticPyObject*baz(PyObject*Py_UNUSED(module),PyObject*arg){interr=1;longval=PyLong_AsLongAndOverflow(arg,&err);if (!err) {returnPyBool_FromLong(val!=0); }Py_RETURN_FALSE; }
111: Mean +- std dev: 165 ns +- 1 ns(1<<32)+1: Mean +- std dev: 169 ns +- 5 ns(1<<62)+11: Mean +- std dev: 170 ns +- 1 ns(1<<128)+2: Mean +- std dev: 156 ns +- 1 ns(1<<256)+3: Mean +- std dev: 156 ns +- 1 ns| Benchmark | ref | patch | baz |
|---|---|---|---|
| 111 | 166 ns | 170 ns: 1.02x slower | 165 ns: 1.01x faster |
| (1<<32)+1 | 243 ns | 173 ns: 1.40x faster | 169 ns: 1.44x faster |
| (1<<62)+11 | 229 ns | 176 ns: 1.30x faster | 170 ns: 1.35x faster |
| (1<<128)+2 | 693 ns | 614 ns: 1.13x faster | 156 ns: 4.44x faster |
| (1<<256)+3 | 697 ns | 614 ns: 1.13x faster | 156 ns: 4.46x faster |
| Geometric mean | (ref) | 1.18x faster | 2.08x faster |
So, probably we want an internal helper function with PyLong_AsLongAndOverflow-like API. It can be used in the PyLong_Export() and few other places, which use PyLong_AsInt64() and clear exception.
Details
diff --git a/Objects/longobject.c b/Objects/longobject.cindex 43c0db753a0..4baa7ba39a5 100644--- a/Objects/longobject.c+++ b/Objects/longobject.c@@ -6815,7 +6815,21 @@ int PyLong_AsInt32(PyObject *obj, int32_t *value) int PyLong_AsInt64(PyObject *obj, int64_t *value) {- LONG_TO_INT(obj, value, "C int64_t");+ int overflow;+#if SIZEOF_LONG == 8+ long x = PyLong_AsLongAndOverflow(obj, &overflow);+#else+ // Windows has 32-bit long, so use 64-bit long long instead+ long long x = PyLong_AsLongLongAndOverflow(obj, &overflow);+#endif+ Py_BUILD_ASSERT(sizeof(x) == sizeof(int64_t));+ *value = (int64_t)x;+ if (overflow) {+ PyErr_SetString(PyExc_OverflowError,+ "Python int too large to convert to C int64_t");+ return -1;+ }+ return PyErr_Occurred() ? -1 : 0; } #define LONG_TO_UINT(obj, value, type_name) \
# bench.pyimportpyperffromfooimportasint64asfrunner=pyperf.Runner()forzin ['111','(1<<32)+1','(1<<62)+11','(1<<128)+2','(1<<256)+3']:h=f"{z}"z=eval(z)runner.bench_func(h,f,z)
/* foo.c */#include<Python.h>staticPyObject*bar(PyObject*Py_UNUSED(module),PyObject*arg){longval;if (!PyLong_AsInt64(arg,&val)) {returnPyBool_FromLong(val!=0); }PyErr_Clear();Py_RETURN_FALSE;}staticPyMethodDefmethods[]= { {"asint64", (PyCFunction)bar,METH_O}, {NULL}};staticstructPyModuleDefdef= {PyModuleDef_HEAD_INIT,"foo",/* m_name */NULL,/* m_doc */-1,/* m_size */methods,/* m_methods */};PyMODINIT_FUNCPyInit_foo(void){returnPyModule_Create(&def);}
Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Links to previous discussion of this feature:
No response