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

gh-92216: improve performance of hasattr for type objects #99979

Merged
merged 9 commits into from Dec 23, 2022

Conversation

eendebakpt
Copy link
Contributor

@eendebakpt eendebakpt commented Dec 4, 2022

This PR improves performance of hasattr for type objects in the case the attribute is missing.

The optimization is similar to the one used for objects which have PyObject_GenericGetAttr as the attribute lookup method
(see specialization for PyObject_GenericGetAttr in _PyObject_LookupAttr and suppress in _PyObject_GenericGetAttrWithDict )

Benchmark:

import pyperf
runner = pyperf.Runner()

setup="""
from dataclasses import dataclass, asdict
@dataclass
class D:
	a : dict
	b: list
	
d=D({'1': 1}, [1,2,3])
asdict(d)
"""

runner.timeit(name=f"micro benchmark", stmt=f"hasattr(o, '__str__')", setup='import os; o=type(os)')
runner.timeit(name=f"micro benchmark: misssing attr", stmt=f"hasattr(o, '_no_such_attr')", setup='import os; o=type(os)')
runner.timeit(name=f"asdict on dataclass", stmt=f"asdict(d)", setup=setup)

Results:

micro benchmark: Mean +- std dev: [main] 42.1 ns +- 1.1 ns -> [pr] 44.1 ns +- 1.8 ns: 1.05x slower
micro benchmark: misssing attr: Mean +- std dev: [main] 256 ns +- 6 ns -> [pr] 37.6 ns +- 0.7 ns: 6.80x faster
asdict on dataclass: Mean +- std dev: [main] 8.87 us +- 0.17 us -> [pr] 6.61 us +- 0.05 us: 1.34x faster

Geometric mean: 2.06x faster

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Dec 5, 2022

Thanks! If I understand correctly, the speedup comes from not having to raise and clear an exception right?

@eendebakpt
Copy link
Contributor Author

eendebakpt commented Dec 5, 2022

Thanks! If I understand correctly, the speedup comes from not having to raise and clear an exception right?

Exactly. This was already happening for the objects with PyObject_GenericGetAttr, this PR extends this to type objects.

@netlify
Copy link

netlify bot commented Dec 13, 2022

Deploy Preview for python-cpython-preview ready!

Name Link
🔨 Latest commit c689b92
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/639b9d8fa912e80008dd983a
😎 Deploy Preview https://deploy-preview-99979--python-cpython-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@eendebakpt
Copy link
Contributor Author

eendebakpt commented Dec 22, 2022

@Fidget-Spinner Would you be able to review this PR?

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Dec 22, 2022

@eendebakpt alright. Sorry I've been putting this off because the code it touches isn't something I'm super familiar with.

However I will take a look at the other examples you pointed out that uses a similar concept.

Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Some suggestions. Good work overall!

Include/internal/pycore_typeobject.h Outdated Show resolved Hide resolved
Objects/object.c Outdated Show resolved Hide resolved
Objects/typeobject.c Outdated Show resolved Hide resolved
Objects/typeobject.c Outdated Show resolved Hide resolved
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Thanks for your patience.

@Fidget-Spinner Fidget-Spinner merged commit 7fc7909 into python:main Dec 23, 2022
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants