classification
Title: Disable runtime checks in release mode
Type: performance Stage: resolved
Components: Interpreter Core Versions: Python 3.9
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: jdemeyer, lemburg, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2019-06-25 23:16 by vstinner, last changed 2019-07-16 10:13 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 14384 closed vstinner, 2019-06-25 23:21
PR 14386 merged vstinner, 2019-06-26 00:47
Messages (7)
msg346570 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-25 23:16
A Python debug build is ABI compatible with a Python release build since Python 3.8 on most platforms (except Windows, Cygwin and Android):
https://docs.python.org/3.8/whatsnew/3.8.html#debug-build-uses-the-same-abi-as-release-build

It should now be way easier to debug an application with a debug build.

I propose to remove runtime debug checks in release mode. assert() is already widely used in the C code base, but there are many runtime checks using PyErr_BadInternalCall() or PyErr_BadArgument. Example:

Py_ssize_t
PyUnicode_GetLength(PyObject *unicode)
{
    if (!PyUnicode_Check(unicode)) {
        PyErr_BadArgument();
        return -1;
    }
    if (PyUnicode_READY(unicode) == -1)
        return -1;
    return PyUnicode_GET_LENGTH(unicode);
}

Attached PR removes these checks when Python is compiled in release mode: when Py_DEBUG is not defined.

Even if I marked this issue as "performance", I don't expect a significant speedup.
msg346577 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-26 01:16
New changeset c6a2320e876354ee62cf8149b849bcff9492d38a by Victor Stinner in branch 'master':
bpo-37406: sqlite3 raises TypeError for wrong operation type (GH-14386)
https://github.com/python/cpython/commit/c6a2320e876354ee62cf8149b849bcff9492d38a
msg346588 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2019-06-26 07:47
Given that extensions call these APIs, I find it highly risky to
disable these checks in any version of the Python runtime and
am -1 on such a change.

Using assert() in C is a pretty bad alternative, since this crashes
the whole process. It should really only be used where no other
means of error handling are possible. Python's exception mechanism
is a much better way to signal and handle such errors at the
application level.

-- 
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Experts (#1, Jun 26 2019)
>>> Python Projects, Coaching and Consulting ...  http://www.egenix.com/
>>> Python Database Interfaces ...           http://products.egenix.com/
>>> Plone/Zope Database Interfaces ...           http://zope.egenix.com/
________________________________________________________________________

::: We implement business ideas - efficiently in both time and costs :::

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
    D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
           Registered at Amtsgericht Duesseldorf: HRB 46611
               http://www.egenix.com/company/contact/
                      http://www.malemburg.com/
msg346643 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-06-26 15:58
I think that the public C API should have runtime checks for its arguments, except performance sensitive functions and macros (like PyTuple_GET_ITEM). The internal C API can use just asserts.

I have a work-in-progress patch which adds tests for the public C API, including calls with invalid arguments.
msg346983 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2019-07-01 09:17
> Python's exception mechanism is a much better way to signal and handle such errors at the application level.

I disagree. There is a difference between exceptions which are possible in normal usage of the code and real bugs, which shouldn't ever happen. A NULL argument to a C API function is not an ordinary exception, it's a real bug. Even if we raise a Python exception for that, it should never be handled in a try/exception block like an ordinary exception.

> Using assert() in C is a pretty bad alternative, since this crashes the whole process.

I *prefer* debugging an assert() failure (which is pretty easy with a decent debugger) rather than debugging a Python exception raised from C (which typically does NOT give a useful traceback).
msg346995 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-07-01 10:44
FYI I'm finishing an article to explain the problem that I am trying to solve here. Sorry for the delay, I will reply later :-)
msg348012 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-07-16 10:13
I didn't find time to write a proper a rationale for this change, and I will not be available next weeks. So I prefer to close this issue. I may reopen it later once I have a strong rationale :-) In the meanwhile, have a look at the new PyHandle API idea which will start without sanity check in release mode since day 1 :-)
https://github.com/pyhandle/pyhandle/
History
Date User Action Args
2019-07-16 10:13:32vstinnersetstatus: open -> closed
resolution: out of date
messages: + msg348012

stage: patch review -> resolved
2019-07-01 10:44:41vstinnersetmessages: + msg346995
2019-07-01 09:17:44jdemeyersetnosy: + jdemeyer
messages: + msg346983
2019-06-26 15:58:03serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg346643
2019-06-26 07:47:24lemburgsetnosy: + lemburg

messages: + msg346588
title: Disable debug runtime checks in release mode -> Disable runtime checks in release mode
2019-06-26 01:16:27vstinnersetmessages: + msg346577
2019-06-26 00:47:57vstinnersetpull_requests: + pull_request14200
2019-06-25 23:21:17vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request14198
2019-06-25 23:19:30vstinnersettitle: Disable runtime checks in release mode -> Disable debug runtime checks in release mode
2019-06-25 23:16:31vstinnercreate