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.

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

Jump to bottom

Speed up and clean up getting optional attributes in C code #76752

Closed
serhiy-storchaka opened this issue Jan 16, 2018 · 8 comments
Closed

Speed up and clean up getting optional attributes in C code #76752

serhiy-storchaka opened this issue Jan 16, 2018 · 8 comments
Labels
3.7 (EOL) end of life extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@serhiy-storchaka
Copy link
Member

BPO 32571
Nosy @vstinner, @njsmith, @methane, @serhiy-storchaka, @1st1
PRs
  • bpo-32571: Avoid raising unneeded AttributeError and silencing it C code. #5205
  • bpo-32571: Avoid raising unneeded AttributeError and silencing it in C code (alt). #5222
  • bpo-32571: Fix reading uninitialized memory. #5332
    		•

    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 2018-01-26.20:08:05.868>
created_at = <Date 2018-01-16.18:18:05.797>
labels = ['extension-modules', 'interpreter-core', '3.7', 'performance']
title = 'Speed up and clean up getting optional attributes in C code'
updated_at = <Date 2018-01-26.20:08:05.866>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2018-01-26.20:08:05.866>
actor = 'yselivanov'
assignee = 'none'
closed = True
closed_date = <Date 2018-01-26.20:08:05.868>
closer = 'yselivanov'
components = ['Extension Modules', 'Interpreter Core']
creation = <Date 2018-01-16.18:18:05.797>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 32571
keywords = ['patch']
message_count = 8.0
messages = ['310101', '310153', '310199', '310200', '310204', '310665', '310746', '310787']
nosy_count = 5.0
nosy_names = ['vstinner', 'njs', 'methane', 'serhiy.storchaka', 'yselivanov']
pr_nums = ['5205', '5222', '5332']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue32571'
versions = ['Python 3.7']
    @serhiy-storchaka
    Copy link
    Member Author

    In bpo-32544 there was introduced a new private C API function _PyObject_GetAttrWithoutError() which never raises an AttributeError, but returns NULL without error set if an attribute is absent. This allowed to speed up Python builtins hasattr() and getattr() with the default argument.

    But C code also could gain a benefit from this. It is common to look up an attribute and silence an AttributeError. Actually it is more common than the opposite case.

    The proposed patch adds yet one function, _PyObject_GetAttrIdWithoutError(), and uses these two functions if it is possible to avoid checking an AttributeError after PyObject_GetAttr() and _PyObject_GetAttrId(). This could speed up some code, and also makes it cleaner.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Jan 16, 2018
    @njsmith
    Copy link
    Contributor

    njsmith commented Jan 17, 2018

    Can I ask why this is a private API? It's extremely useful for extension modules too. For example, numpy has been carrying around a reimplementation of PyObject_GetAttr for years, exactly to avoid the cost of creating an AttributeError exception in common cases:

    https://github.com/numpy/numpy/blob/master/numpy/core/src/private/get_attr_string.h

    (To emphasize the similarity, note that the code above used to be called PyArray_GetAttrString_SuppressException, until a refactoring last year: numpy/numpy@69a423b. It's always been specifically used for looking up numpy-specific special methods like __array__, __array_interface__, etc., though, which is why it can get away with those shortcuts -- we know they usually aren't defined.)

    @serhiy-storchaka
    Copy link
    Member Author

    PR 5222 provides functions _PyObject_LookupAttr() and _PyObject_LookupAttrId() with alternate interface. Some code becomes even more simpler.

    The possible drawback of this approach is that the compiler should allocate the variable for the result on the stack instead of passing the result in a register (unless compilers are smart enough for optimizing out this case).

    @serhiy-storchaka
    Copy link
    Member Author

    Because this API is experimental. It was changed in recent 24 hours and may be changed more before merging or even after releasing 3.7.

    @njsmith
    Copy link
    Contributor

    njsmith commented Jan 17, 2018

    Well, mostly I just want to draw your attention to that use case to encourage you to make it public when it's ready :-)

    @methane
    Copy link
    Member

    methane commented Jan 25, 2018

    New changeset f320be7 by INADA Naoki (Serhiy Storchaka) in branch 'master':
    bpo-32571: Avoid raising unneeded AttributeError and silencing it in C code (GH-5222)
    f320be7

    @methane
    Copy link
    Member

    methane commented Jan 26, 2018

    New changeset e76daeb by INADA Naoki in branch 'master':
    bpo-32571: Fix reading uninitialized memory (GH-5332)
    e76daeb

    @1st1
    Copy link
    Member

    1st1 commented Jan 26, 2018

    Closing this one now. Thanks Serhiy and Inada-san!

    @1st1 1st1 closed this as completed Jan 26, 2018
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @eendebakpt eendebakpt mentioned this issue May 3, 2022
    Closed
    This was referenced Jul 1, 2023
    Closed
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @methane @1st1 @njsmith @serhiy-storchaka