Skip to content
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

Windows: enable compiler optimizations when building Python in debug mode #89278

Closed
vstinner opened this issue Sep 6, 2021 · 5 comments
Closed
Labels
3.11 only security fixes OS-windows

Comments

@vstinner
Copy link
Member

vstinner commented Sep 6, 2021

BPO 45115
Nosy @pfmoore, @vstinner, @tjguk, @zware, @zooba, @corona10
PRs
  • bpo-45115: Enable optimiaztions on Windows debug build #28181
  • bpo-39573: Py_TYPE becomes a static inline function #28128
  • 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:

    assignee = None
    closed_at = <Date 2021-09-07.13:47:19.165>
    created_at = <Date 2021-09-06.15:21:42.501>
    labels = ['OS-windows', '3.11']
    title = 'Windows: enable compiler optimizations when building Python in debug mode'
    updated_at = <Date 2021-09-07.13:47:19.164>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2021-09-07.13:47:19.164>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-09-07.13:47:19.165>
    closer = 'vstinner'
    components = ['Windows']
    creation = <Date 2021-09-06.15:21:42.501>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45115
    keywords = ['patch']
    message_count = 5.0
    messages = ['401141', '401142', '401145', '401156', '401273']
    nosy_count = 6.0
    nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'zach.ware', 'steve.dower', 'corona10']
    pr_nums = ['28181', '28128']
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue45115'
    versions = ['Python 3.11']

    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 6, 2021

    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: bpo-43244: GitHub Action builds Python in debug mode on Windows #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.

    @vstinner vstinner added 3.11 only security fixes OS-windows labels Sep 6, 2021
    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 6, 2021

    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".

    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 6, 2021

    I recently documented the "Python debug build":
    https://docs.python.org/dev/using/configure.html#python-debug-build

    @zooba
    Copy link
    Member

    zooba commented Sep 6, 2021

    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.

    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 7, 2021

    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 fb30509.

    @vstinner vstinner closed this as completed Sep 7, 2021
    @vstinner vstinner closed this as completed Sep 7, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes OS-windows
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants