classification
Title: Windows: enable compiler optimizations when building Python in debug mode
Type: Stage: resolved
Components: Windows Versions: Python 3.11
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: corona10, paul.moore, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords: patch

Created on 2021-09-06 15:21 by vstinner, last changed 2021-09-07 13:47 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 28181 closed vstinner, 2021-09-06 15:25
PR 28128 vstinner, 2021-09-06 15:35
Messages (5)
msg401141 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-06 15:21
The Visual Studio project of Python, PCBuild\ directory, disables compiler optimizations when Python is built in debug mode. It seems to be the default in Visual Studio.

Disabling compiler optimizations cause two kinds of issues:

* It increases the stack memory: tests using a deep call stack are more likely to crash (test_pickle, test_marshal, test_exceptions).

* Running the Python test suite take 19 min 41 sec instead of 12 min 19 sec on Windows x64: 1.6x slower. Because of that, we cannot use a debug build in the GitHub Action pre-commit CI, and we miss bugs which are catched "too late", in Windows buildbots. See my latest attempt to use a debug build in GitHub Actions: https://github.com/python/cpython/pull/24914

Example of test_marshal:

        # The max stack depth should match the value in Python/marshal.c.
        # BUG: https://bugs.python.org/issue33720
        # Windows always limits the maximum depth on release and debug builds
        #if os.name == 'nt' and hasattr(sys, 'gettotalrefcount'):
        if os.name == 'nt':
            MAX_MARSHAL_STACK_DEPTH = 1000
        else:
            MAX_MARSHAL_STACK_DEPTH = 2000

I propose to only change the compiler options for the pythoncore project which builds most important files for the Python "core". See attached PR.
msg401142 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-06 15:26
This change is motivated by my PR 28128 which converts the Py_TYPE() macro to a static inline function. The problem is that by default, MSC disables inlining and test_exceptions does crash with a stack overflow, since my change increases the usage of the stack memory: see bpo-44348.

By the problem is wider than just Py_TYPE().

See also bpo-45094: "Consider using __forceinline and __attribute__((always_inline)) on static inline functions (Py_INCREF, Py_TYPE) for debug builds".
msg401145 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-06 15:38
I recently documented the "Python debug build":
https://docs.python.org/dev/using/configure.html#python-debug-build
msg401156 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2021-09-06 16:48
I strongly disagree. If CI needs to be faster, please just change the CI configuration. If contributors have to wait a few minutes longer, they can wait - they'll save that time in local compilations.

Local debugging absolutely relies on debug builds. You'd be destroying the workflow of most Windows-based developers with this change. It's not worth it.
msg401273 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-07 13:47
Steve:
> I strongly disagree. If CI needs to be faster, please just change the CI configuration. If contributors have to wait a few minutes longer, they can wait - they'll save that time in local compilations.
> Local debugging absolutely relies on debug builds. You'd be destroying the workflow of most Windows-based developers with this change. It's not worth it.

Ok, I reject my issue.

I found a portable way to fix bpo-44348: it doesn't depend on the OS, nor the compiler, nor compiler options. The fix is to use the trashcan mecanism in the BaseException deallocator: commit fb305092a5d7894b41f122c1a1117b3abf4c567e.
History
Date User Action Args
2021-09-07 13:47:19vstinnersetstatus: open -> closed
resolution: rejected
messages: + msg401273

stage: patch review -> resolved
2021-09-07 02:19:21corona10setmessages: - msg401202
2021-09-07 02:19:06corona10setnosy: + corona10
messages: + msg401202
2021-09-06 16:48:45steve.dowersetmessages: + msg401156
2021-09-06 15:38:39vstinnersetmessages: + msg401145
2021-09-06 15:35:54vstinnersetpull_requests: + pull_request26610
2021-09-06 15:26:42vstinnersetmessages: + msg401142
2021-09-06 15:25:04vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request26609
2021-09-06 15:21:42vstinnercreate