Skip to content

bpo-27535: Optimize warnings.warn() #4508

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.

Sign up for GitHub

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

Merged
vstinner merged 1 commit into from
Nov 22, 2017
Merged

bpo-27535: Optimize warnings.warn() #4508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions Lib/warnings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ def filterwarnings(action, message="", category=Warning, module="", lineno=0,
'lineno' -- an integer line number, 0 matches all warnings
'append' -- if true, append to the list of filters
"""
import re
assert action in ("error", "ignore", "always", "default", "module",
"once"), "invalid action: %r" % (action,)
assert isinstance(message, str), "message must be a string"
Expand All @@ -137,8 +136,20 @@ def filterwarnings(action, message="", category=Warning, module="", lineno=0,
assert isinstance(module, str), "module must be a string"
assert isinstance(lineno, int) and lineno >= 0, \
"lineno must be an int >= 0"
_add_filter(action, re.compile(message, re.I), category,
re.compile(module), lineno, append=append)

if message or module:
import re

if message:
message = re.compile(message, re.I)
else:
message = None
if module:
module = re.compile(module)
else:
module = None

_add_filter(action, message, category, module, lineno, append=append)

def simplefilter(action, category=Warning, lineno=0, append=False):
"""Insert a simple entry into the list of warnings filters (at the front).
Expand Down
75 changes: 33 additions & 42 deletions Python/_warnings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ check_matched(PyObject *obj, PyObject *arg)
A NULL return value can mean false or an error.
*/
static PyObject *
get_warnings_attr(const char *attr, int try_import)
get_warnings_attr(_Py_Identifier *attr_id, int try_import)
{
static PyObject *warnings_str = NULL;
PyObject *warnings_str;
PyObject *warnings_module, *obj;
_Py_IDENTIFIER(warnings);

warnings_str = _PyUnicode_FromId(&PyId_warnings);
if (warnings_str == NULL) {
warnings_str = PyUnicode_InternFromString("warnings");
if (warnings_str == NULL)
return NULL;
return NULL;
}

/* don't try to import after the start of the Python finallization */
Expand All @@ -64,7 +64,7 @@ get_warnings_attr(const char *attr, int try_import)
return NULL;
}

obj = PyObject_GetAttrString(warnings_module, attr);
obj = _PyObject_GetAttrId(warnings_module, attr_id);
Py_DECREF(warnings_module);
if (obj == NULL && PyErr_ExceptionMatches(PyExc_AttributeError)) {
PyErr_Clear();
Expand All @@ -77,8 +77,9 @@ static PyObject *
get_once_registry(void)
{
PyObject *registry;
_Py_IDENTIFIER(onceregistry);

registry = get_warnings_attr("onceregistry", 0);
registry = get_warnings_attr(&PyId_onceregistry, 0);
if (registry == NULL) {
if (PyErr_Occurred())
return NULL;
Expand All @@ -102,8 +103,9 @@ static PyObject *
get_default_action(void)
{
PyObject *default_action;
_Py_IDENTIFIER(defaultaction);

default_action = get_warnings_attr("defaultaction", 0);
default_action = get_warnings_attr(&PyId_defaultaction, 0);
if (default_action == NULL) {
if (PyErr_Occurred()) {
return NULL;
Expand Down Expand Up @@ -132,8 +134,9 @@ get_filter(PyObject *category, PyObject *text, Py_ssize_t lineno,
PyObject *action;
Py_ssize_t i;
PyObject *warnings_filters;
_Py_IDENTIFIER(filters);

warnings_filters = get_warnings_attr("filters", 0);
warnings_filters = get_warnings_attr(&PyId_filters, 0);
if (warnings_filters == NULL) {
if (PyErr_Occurred())
return NULL;
Expand Down Expand Up @@ -389,11 +392,13 @@ call_show_warning(PyObject *category, PyObject *text, PyObject *message,
PyObject *sourceline, PyObject *source)
{
PyObject *show_fn, *msg, *res, *warnmsg_cls = NULL;
_Py_IDENTIFIER(_showwarnmsg);
_Py_IDENTIFIER(WarningMessage);

/* If the source parameter is set, try to get the Python implementation.
The Python implementation is able to log the traceback where the source
was allocated, whereas the C implementation doesn't. */
show_fn = get_warnings_attr("_showwarnmsg", source != NULL);
show_fn = get_warnings_attr(&PyId__showwarnmsg, source != NULL);
if (show_fn == NULL) {
if (PyErr_Occurred())
return -1;
Expand All @@ -407,7 +412,7 @@ call_show_warning(PyObject *category, PyObject *text, PyObject *message,
goto error;
}

warnmsg_cls = get_warnings_attr("WarningMessage", 0);
warnmsg_cls = get_warnings_attr(&PyId_WarningMessage, 0);
if (warnmsg_cls == NULL) {
if (!PyErr_Occurred()) {
PyErr_SetString(PyExc_RuntimeError,
Expand Down Expand Up @@ -1146,50 +1151,36 @@ static PyMethodDef warnings_functions[] = {
static PyObject *
create_filter(PyObject *category, const char *action)
{
static PyObject *ignore_str = NULL;
static PyObject *error_str = NULL;
static PyObject *default_str = NULL;
static PyObject *always_str = NULL;
PyObject *action_obj = NULL;
_Py_IDENTIFIER(ignore);
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Nov 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This optimization doesn't make sense since create_filter() is used only few times at start. It would be better to pass _Py_Identifier * instead of const char *, this could save few lines of code.

Copy link
Copy Markdown
Member Author

@vstinner vstinner Nov 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I chose to put two changes into the same commit. This change was made to allow to free strings at Python finallization, it's not an optimization.

_Py_IDENTIFIER(error);
_Py_IDENTIFIER(always);
_Py_static_string(PyId_default, "default");
_Py_Identifier *id;

if (!strcmp(action, "ignore")) {
if (ignore_str == NULL) {
ignore_str = PyUnicode_InternFromString("ignore");
if (ignore_str == NULL)
return NULL;
}
action_obj = ignore_str;
id = &PyId_ignore;
}
else if (!strcmp(action, "error")) {
if (error_str == NULL) {
error_str = PyUnicode_InternFromString("error");
if (error_str == NULL)
return NULL;
}
action_obj = error_str;
id = &PyId_error;
}
else if (!strcmp(action, "default")) {
if (default_str == NULL) {
default_str = PyUnicode_InternFromString("default");
if (default_str == NULL)
return NULL;
}
action_obj = default_str;
id = &PyId_default;
}
else if (!strcmp(action, "always")) {
if (always_str == NULL) {
always_str = PyUnicode_InternFromString("always");
if (always_str == NULL)
return NULL;
}
action_obj = always_str;
id = &PyId_always;
}
else {
Py_FatalError("unknown action");
PyErr_SetString(PyExc_ValueError, "unknown action");
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka Nov 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert this change. This is an internal function called with predefined set of arguments. The code in the else branch is a dead code. Here should be Py_FatalError() or Py_UNREACHABLE().

Copy link
Copy Markdown
Member Author

@vstinner vstinner Nov 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your idea of using _Py_Identifier: I proposed #4516 to implement your idea. It avoids the need to have to unhandle "unknown action".

FYI I'm trying to remove all Py_FatalError() calls from the Python source code, one by one, when it's possible and when it makes sense. I added a new _PyInitError API to report errors at Python initialization, so the caller can decide when Py_FatalError() will be called, and maybe release memory before calling Py_FatalError() (or flush files, or something else).

return NULL;
}

PyObject *action_str = _PyUnicode_FromId(id);
if (action_str == NULL) {
return NULL;
}

/* This assumes the line number is zero for now. */
return PyTuple_Pack(5, action_obj, Py_None,
return PyTuple_Pack(5, action_str, Py_None,
category, Py_None, _PyLong_Zero);
}

Expand Down