Skip to content

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Aug 7, 2023

"""
result = set(_handlers.keys())
return frozenset(result)
return frozenset(_handlers.keys())
Copy link
Member

Choose a reason for hiding this comment

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

Why keep .keys()?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've tested with frozenset(_handlers) it is a bit slower:

(.venv) ~/Desktop/cpython  issue-107710 ✗                                              1 ⚠️
» pyperf timeit --setup 'import ex; l = ex.logging' 'l.getHandlerNames()'
.....................
Mean +- std dev: 2.07 us +- 0.02 us
                                                                                           
(.venv) ~/Desktop/cpython  issue-107710 ✗                                                 
» pyperf timeit --setup 'import ex; l = ex.logging' 'l.getHandlerNames()'
.....................
Mean +- std dev: 2.07 us +- 0.01 us
                                                                                           
(.venv) ~/Desktop/cpython  issue-107710 ✗                                                 
» pyperf timeit --setup 'import ex; l = ex.logging' 'l.getHandlerNames()'
.....................
Mean +- std dev: 2.07 us +- 0.01 us

Copy link
Member

Choose a reason for hiding this comment

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

I don't think a 3us difference is a good enough reason to avoid using the simpler code here. And it is still faster than main :)

Copy link
Member

Choose a reason for hiding this comment

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

The difference is so small that I would classify this change as a code cleanup rather than an optimization.

This comment was marked as spam.

Choose a reason for hiding this comment

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

Please guys I need a help

Copy link

@lizzydavis695 lizzydavis695 left a comment

Choose a reason for hiding this comment

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

Login

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.

5 participants