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

Speed up hasattr(o, name) and getattr(o, name, default) #76725

Closed
methane opened this issue Jan 13, 2018 · 6 comments
Closed

Speed up hasattr(o, name) and getattr(o, name, default) #76725

methane opened this issue Jan 13, 2018 · 6 comments
Labels
3.7 interpreter-core performance

Comments

@methane
Copy link
Member

@methane methane commented Jan 13, 2018

BPO 32544
Nosy @vstinner, @methane, @serhiy-storchaka, @ilevkivskyi
PRs
  • #5173
  • 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-16.11:53:07.136>
    created_at = <Date 2018-01-13.18:29:29.915>
    labels = ['interpreter-core', '3.7', 'performance']
    title = 'Speed up hasattr(o, name) and getattr(o, name, default)'
    updated_at = <Date 2018-01-16.15:42:10.231>
    user = 'https://github.com/methane'

    bugs.python.org fields:

    activity = <Date 2018-01-16.15:42:10.231>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-01-16.11:53:07.136>
    closer = 'methane'
    components = ['Interpreter Core']
    creation = <Date 2018-01-13.18:29:29.915>
    creator = 'methane'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32544
    keywords = ['patch']
    message_count = 6.0
    messages = ['309896', '309991', '310049', '310084', '310085', '310090']
    nosy_count = 4.0
    nosy_names = ['vstinner', 'methane', 'serhiy.storchaka', 'levkivskyi']
    pr_nums = ['5173']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue32544'
    versions = ['Python 3.7']

    @methane
    Copy link
    Member Author

    @methane methane commented Jan 13, 2018

    ABCMeta.new calls getattr(value, "__isabstractmethod__", False) many times.

    cpython/Lib/abc.py

    Lines 135 to 142 in 0f31c74

    abstracts = {name
    for name, value in namespace.items()
    if getattr(value, "__isabstractmethod__", False)}
    for base in bases:
    for name in getattr(base, "__abstractmethods__", set()):
    value = getattr(cls, name, None)
    if getattr(value, "__isabstractmethod__", False):
    abstracts.add(name)

    Since metaclass is used by subclasses, it checks all members of all subclasses (including concrete class).

    But getattr() and hasattr() is inefficient when attribution is not found,
    because it raises AttributeError and clear it internally.

    I tried to bypass AttributeError when tp_getattro == PyObject_GenericGetAttr.
    Here is quick micro benchmark:

    $ ./python-patched -m perf timeit --compare-to=`pwd`/python-master -s 'def foo(): pass' 'hasattr(foo, "__isabstractmethod__")'python-master: ..................... 385 ns +- 2 ns
    python-patched: ..................... 87.6 ns +- 1.6 ns

    Mean +- std dev: [python-master] 385 ns +- 2 ns -> [python-patched] 87.6 ns +- 1.6 ns: 4.40x faster (-77%)

    (I added Ivan to nosy list because he may implement C version of ABC.)

    @serhiy-storchaka
    Copy link
    Member

    @serhiy-storchaka serhiy-storchaka commented Jan 15, 2018

    I considered the idea of implementing _PyObject_GetAttrWithoutError() for bpo-31572, but dropped it because the code simplification was too small. But if this has performance benefits I'm +1 to implementing it. I'll update my still open PRs to use this API.

    @serhiy-storchaka serhiy-storchaka added interpreter-core 3.7 performance labels Jan 15, 2018
    @methane
    Copy link
    Member Author

    @methane methane commented Jan 16, 2018

    I run pyperformance.
    It seems django_template is significant faster.

    # Without PGO+LTO

    ./python -m perf compare_to -G --min-speed=2 default.json patched.json
    Slower (6):

    • scimark_monte_carlo: 217 ms +- 9 ms -> 237 ms +- 10 ms: 1.09x slower (+9%)
    • scimark_lu: 326 ms +- 14 ms -> 351 ms +- 20 ms: 1.08x slower (+8%)
    • float: 219 ms +- 4 ms -> 226 ms +- 8 ms: 1.03x slower (+3%)
    • crypto_pyaes: 208 ms +- 2 ms -> 213 ms +- 2 ms: 1.02x slower (+2%)
    • pickle: 20.1 us +- 0.2 us -> 20.6 us +- 0.4 us: 1.02x slower (+2%)
    • xml_etree_iterparse: 184 ms +- 3 ms -> 188 ms +- 3 ms: 1.02x slower (+2%)

    Faster (10):

    • django_template: 315 ms +- 4 ms -> 287 ms +- 3 ms: 1.10x faster (-9%)
    • logging_format: 25.9 us +- 0.3 us -> 24.7 us +- 0.3 us: 1.05x faster (-5%)
    • logging_simple: 21.6 us +- 0.2 us -> 20.6 us +- 0.4 us: 1.05x faster (-5%)
    • sympy_str: 399 ms +- 3 ms -> 381 ms +- 2 ms: 1.05x faster (-5%)
    • unpack_sequence: 117 ns +- 6 ns -> 112 ns +- 0 ns: 1.04x faster (-4%)
    • sympy_expand: 886 ms +- 7 ms -> 851 ms +- 7 ms: 1.04x faster (-4%)
    • regex_effbot: 5.00 ms +- 0.09 ms -> 4.84 ms +- 0.11 ms: 1.03x faster (-3%)
    • xml_etree_process: 185 ms +- 3 ms -> 180 ms +- 4 ms: 1.02x faster (-2%)
    • nqueens: 215 ms +- 3 ms -> 211 ms +- 3 ms: 1.02x faster (-2%)
    • telco: 16.0 ms +- 0.6 ms -> 15.6 ms +- 0.4 ms: 1.02x faster (-2%)

    Benchmark hidden because not significant (44)

    # With PGO+LTO

    $ ./python -m perf compare_to -G --min-speed=2 default-opt.json patched-opt.json
    Slower (8):
    - xml_etree_process: 179 ms +- 3 ms -> 195 ms +- 3 ms: 1.09x slower (+9%)
    - xml_etree_generate: 208 ms +- 5 ms -> 224 ms +- 3 ms: 1.08x slower (+8%)
    - xml_etree_iterparse: 190 ms +- 5 ms -> 203 ms +- 3 ms: 1.07x slower (+7%)
    - pickle_list: 6.80 us +- 0.30 us -> 7.18 us +- 0.72 us: 1.06x slower (+6%)
    - genshi_text: 72.8 ms +- 1.1 ms -> 76.3 ms +- 0.8 ms: 1.05x slower (+5%)
    - mako: 32.6 ms +- 0.1 ms -> 34.1 ms +- 0.6 ms: 1.05x slower (+5%)
    - unpickle_pure_python: 713 us +- 12 us -> 744 us +- 16 us: 1.04x slower (+4%)
    - scimark_sor: 375 ms +- 13 ms -> 383 ms +- 11 ms: 1.02x slower (+2%)

    Faster (14):

    • django_template: 327 ms +- 3 ms -> 296 ms +- 2 ms: 1.10x faster (-9%)
    • scimark_lu: 339 ms +- 20 ms -> 309 ms +- 13 ms: 1.10x faster (-9%)
    • sympy_str: 418 ms +- 4 ms -> 387 ms +- 3 ms: 1.08x faster (-7%)
    • sympy_expand: 926 ms +- 5 ms -> 860 ms +- 5 ms: 1.08x faster (-7%)
    • logging_simple: 22.4 us +- 0.3 us -> 21.0 us +- 0.3 us: 1.07x faster (-6%)
    • scimark_fft: 741 ms +- 61 ms -> 697 ms +- 25 ms: 1.06x faster (-6%)
    • telco: 17.6 ms +- 0.4 ms -> 16.6 ms +- 0.4 ms: 1.06x faster (-5%)
    • logging_format: 26.5 us +- 0.3 us -> 25.3 us +- 0.3 us: 1.05x faster (-5%)
    • sqlite_synth: 7.35 us +- 0.16 us -> 7.08 us +- 0.18 us: 1.04x faster (-4%)
    • scimark_sparse_mat_mult: 8.83 ms +- 0.42 ms -> 8.51 ms +- 0.37 ms: 1.04x faster (-4%)
    • pathlib: 43.1 ms +- 0.6 ms -> 41.7 ms +- 0.7 ms: 1.03x faster (-3%)
    • html5lib: 193 ms +- 7 ms -> 188 ms +- 7 ms: 1.03x faster (-3%)
    • richards: 149 ms +- 3 ms -> 145 ms +- 2 ms: 1.02x faster (-2%)
    • pickle_pure_python: 1.01 ms +- 0.01 ms -> 984 us +- 20 us: 1.02x faster (-2%)

    Benchmark hidden because not significant (38)

    @methane
    Copy link
    Member Author

    @methane methane commented Jan 16, 2018

    New changeset 378edee by INADA Naoki in branch 'master':
    bpo-32544: Speed up hasattr() and getattr() (GH-5173)
    378edee

    @methane methane closed this Jan 16, 2018
    @methane
    Copy link
    Member Author

    @methane methane commented Jan 16, 2018

    I confirmed django_template is hasattr-heavy benchmark.

    Function was called by...
    ncalls tottime cumtime
    {built-in method builtins.hasattr} <- 1 0.000 0.000 /home/inada-n/local/py37/lib/python3.7/site-packages/django/apps/registry.py:20(init)
    ...
    16318 0.003 0.003 /home/inada-n/local/py37/lib/python3.7/site-packages/django/utils/functional.py:81(prepare_class)
    90000 0.065 0.065 /home/inada-n/local/py37/lib/python3.7/site-packages/django/utils/html.py:79(conditional_escape)
    93200 0.064 0.064 /home/inada-n/local/py37/lib/python3.7/site-packages/django/utils/safestring.py:129(mark_safe)
    90000 0.075 0.075 /home/inada-n/local/py37/lib/python3.7/site-packages/django/utils/safestring.py:149(mark_for_escaping)

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Jan 16, 2018

    Cool. I always wanted to implement something similar. Nice work Naoki!

    @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.7 interpreter-core performance
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants