Skip to content

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

Merged
vstinner merged 1 commit intopython:masterfrom
vstinner:warn_optim2
Nov 22, 2017
Merged

bpo-27535: Optimize warnings.warn()#4508
vstinner merged 1 commit intopython:masterfrom
vstinner:warn_optim2

Conversation

@vstinner
Copy link
Copy Markdown
Member

@vstinner vstinner commented Nov 22, 2017

  • Optimize warnings.filterwarnings(). Replace re.compile('') with
    None to avoid the cost of calling a regex.match() method, whereas
    it always matchs.
  • Optimize get_warnings_attr(): replace PyObject_GetAttrString() with
    _PyObject_GetAttrId().

Cleanup also create_filter():

  • Use _Py_IDENTIFIER() to allow to cleanup strings at Python
    finalization
  • Replace Py_FatalError() with a regular exceptions

https://bugs.python.org/issue27535

* Optimize warnings.filterwarnings(). Replace re.compile('') with
  None to avoid the cost of calling a regex.match() method, whereas
  it always matchs.
* Optimize get_warnings_attr(): replace PyObject_GetAttrString() with
  _PyObject_GetAttrId().

Cleanup also create_filter():

* Use _Py_IDENTIFIER() to allow to cleanup strings at Python
  finalization
* Replace Py_FatalError() with a regular exceptions
@vstinner vstinner merged commit 8265627 into python:master Nov 22, 2017
@vstinner vstinner deleted the warn_optim2 branch November 22, 2017 22:51
}
else {
Py_FatalError("unknown action");
PyErr_SetString(PyExc_ValueError, "unknown action");
Copy link
Copy Markdown
Member

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

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

static PyObject *default_str = NULL;
static PyObject *always_str = NULL;
PyObject *action_obj = NULL;
_Py_IDENTIFIER(ignore);
Copy link
Copy Markdown
Member

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants