Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-89279: In ceval.c, redefine some macros for speed#32387
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
0be9d62abd9118172405650e3d7b59da3bfab6660eae6b53c01cff8169da3802894f6e4b709765617fcda901b91cceb53126e810711084c0b7dfcdc9a5c57ca8cba6ec43ec569a151945c992d27b342e43b21c5241cb06787aaf813508f45File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Improve interpreter performance on Windows by inlining a few specific macros. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -44,6 +44,55 @@ | ||
| # error "ceval.c must be build with Py_BUILD_CORE define for best performance" | ||
| #endif | ||
| #ifndef Py_DEBUG | ||
| // GH-89279: The MSVC compiler does not inline these static inline functions | ||
| // in PGO build in _PyEval_EvalFrameDefault(), because this function is over | ||
| // the limit of PGO, and that limit cannot be configured. | ||
| // Define them as macros to make sure that they are always inlined by the | ||
| // preprocessor. | ||
| #undef Py_DECREF | ||
| #define Py_DECREF(arg) \ | ||
| do { \ | ||
| PyObject *op = _PyObject_CAST(arg); \ | ||
| if (--op->ob_refcnt == 0) { \ | ||
| destructor dealloc = Py_TYPE(op)->tp_dealloc; \ | ||
| (*dealloc)(op); \ | ||
| } \ | ||
| } while (0) | ||
| #undef Py_XDECREF | ||
| #define Py_XDECREF(arg) \ | ||
| do { \ | ||
| PyObject *xop = _PyObject_CAST(arg); \ | ||
| if (xop != NULL) { \ | ||
| Py_DECREF(xop); \ | ||
| } \ | ||
| } while (0) | ||
| #undef Py_IS_TYPE | ||
| #define Py_IS_TYPE(ob, type) \ | ||
| (_PyObject_CAST(ob)->ob_type == (type)) | ||
| #undef _Py_DECREF_SPECIALIZED | ||
| #define _Py_DECREF_SPECIALIZED(arg, dealloc) \ | ||
| do { \ | ||
| PyObject *op = _PyObject_CAST(arg); \ | ||
| if (--op->ob_refcnt == 0) { \ | ||
| destructor d = (destructor)(dealloc); \ | ||
| d(op); \ | ||
| } \ | ||
| } while (0) | ||
| #endif | ||
| // GH-89279: Similar to above, force inlining by using a macro. | ||
| #if defined(_MSC_VER) && SIZEOF_INT == 4 | ||
| #define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) (assert(sizeof((ATOMIC_VAL)->_value) == 4), *((volatile int*)&((ATOMIC_VAL)->_value))) | ||
gvanrossum marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| #else | ||
| #define _Py_atomic_load_relaxed_int32(ATOMIC_VAL) _Py_atomic_load_relaxed(ATOMIC_VAL) | ||
| #endif | ||
| /* Forward declarations */ | ||
| static PyObject *trace_call_function( | ||
| PyThreadState *tstate, PyObject *callable, PyObject **stack, | ||
| @@ -192,10 +241,10 @@ COMPUTE_EVAL_BREAKER(PyInterpreterState *interp, | ||
| struct _ceval_state *ceval2) | ||
| { | ||
| _Py_atomic_store_relaxed(&ceval2->eval_breaker, | ||
| _Py_atomic_load_relaxed(&ceval2->gil_drop_request) | ||
| | (_Py_atomic_load_relaxed(&ceval->signals_pending) | ||
| _Py_atomic_load_relaxed_int32(&ceval2->gil_drop_request) | ||
| | (_Py_atomic_load_relaxed_int32(&ceval->signals_pending) | ||
| && _Py_ThreadCanHandleSignals(interp)) | ||
| | (_Py_atomic_load_relaxed(&ceval2->pending.calls_to_do) | ||
| | (_Py_atomic_load_relaxed_int32(&ceval2->pending.calls_to_do) | ||
| && _Py_ThreadCanHandlePendingCalls()) | ||
| | ceval2->pending.async_exc); | ||
| } | ||
| @@ -740,7 +789,7 @@ _Py_FinishPendingCalls(PyThreadState *tstate) | ||
| struct _pending_calls *pending = &tstate->interp->ceval.pending; | ||
| if (!_Py_atomic_load_relaxed(&(pending->calls_to_do))) { | ||
| if (!_Py_atomic_load_relaxed_int32(&(pending->calls_to_do))) { | ||
| return; | ||
| } | ||
| @@ -1187,22 +1236,22 @@ eval_frame_handle_pending(PyThreadState *tstate) | ||
| struct _ceval_runtime_state *ceval = &runtime->ceval; | ||
| /* Pending signals */ | ||
| if (_Py_atomic_load_relaxed(&ceval->signals_pending)) { | ||
| if (_Py_atomic_load_relaxed_int32(&ceval->signals_pending)) { | ||
| if (handle_signals(tstate) != 0) { | ||
| return -1; | ||
| } | ||
| } | ||
| /* Pending calls */ | ||
| struct _ceval_state *ceval2 = &tstate->interp->ceval; | ||
| if (_Py_atomic_load_relaxed(&ceval2->pending.calls_to_do)) { | ||
| if (_Py_atomic_load_relaxed_int32(&ceval2->pending.calls_to_do)) { | ||
| if (make_pending_calls(tstate->interp) != 0) { | ||
| return -1; | ||
| } | ||
| } | ||
| /* GIL drop request */ | ||
| if (_Py_atomic_load_relaxed(&ceval2->gil_drop_request)) { | ||
| if (_Py_atomic_load_relaxed_int32(&ceval2->gil_drop_request)) { | ||
| /* Give another thread a chance */ | ||
| if (_PyThreadState_Swap(&runtime->gilstate, NULL) != tstate) { | ||
| Py_FatalError("tstate mix-up"); | ||
| @@ -1360,7 +1409,7 @@ eval_frame_handle_pending(PyThreadState *tstate) | ||
| #define CHECK_EVAL_BREAKER() \ | ||
| _Py_CHECK_EMSCRIPTEN_SIGNALS_PERIODICALLY(); \ | ||
| if (_Py_atomic_load_relaxed(eval_breaker)) { \ | ||
| if (_Py_atomic_load_relaxed_int32(eval_breaker)) { \ | ||
| goto handle_eval_breaker; \ | ||
| } | ||
| @@ -1640,10 +1689,8 @@ typedef struct { | ||
| PyObject *kwnames; | ||
| } CallShape; | ||
| static inline bool | ||
| is_method(PyObject **stack_pointer, int args) { | ||
| return PEEK(args+2) != NULL; | ||
| } | ||
| // GH-89279: Must be a macro to be sure it's inlined by MSVC. | ||
| #define is_method(stack_pointer, args) (PEEK((args)+2) != NULL) | ||
gvanrossum marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. gvanrossum marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| #define KWNAMES_LEN() \ | ||
| (call_shape.kwnames == NULL ? 0 : ((int)PyTuple_GET_SIZE(call_shape.kwnames))) | ||
| @@ -1794,7 +1841,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int | ||
| PREDICTED(RESUME_QUICK); | ||
| assert(tstate->cframe == &cframe); | ||
| assert(frame == cframe.current_frame); | ||
| if (_Py_atomic_load_relaxed(eval_breaker) && oparg < 2) { | ||
| if (_Py_atomic_load_relaxed_int32(eval_breaker) && oparg < 2) { | ||
| goto handle_eval_breaker; | ||
| } | ||
| DISPATCH(); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if only MSVC is affected, only redefine these macros for MSVC, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer us running the same code on all platforms if possible, so that it's less likely that someone accidentally breaks the Windows version by want appears to be an innocent change on Linux.
(I have to make an exception for load_relaxed because it is heavily platform-specific.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be extra safe, you may also check Py_REF_DEBUG, even if Py_REF_DEBUG should not be defined if Py_DEBUG is defined.
Or maybe object.h should fail with
#errorwith Py_REF_DEBUG is defined whereas Py_DEBUG is not.