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

Only calculate hash once for Marker objects #513

Merged
merged 6 commits into from
Jun 16, 2024

Conversation

bdraco
Copy link
Contributor

@bdraco bdraco commented Jun 8, 2024

before
validate_mapping_before

after
validate_mapping_after

voluptuous/__init__.py Outdated Show resolved Hide resolved
description: typing.Optional[str] = None,
) -> None:
super().__init__(schema_, msg, description)
self.__hash__ = lru_cache(maxsize=None)(lambda: object.__hash__(self))
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this necessary? Doesn't Marker.__hash__ cover this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its overriding the hash here to use the base object.__hash__ instead of Marker.__hash__

@bdraco
Copy link
Contributor Author

bdraco commented Jun 8, 2024

I'm letting this run over the weekend on my production Home Assistant so I can examine the cache on Monday once its run for a bit. If all still looks well, I'll mark it ready for review than

@bdraco bdraco mentioned this pull request Jun 8, 2024
@ds-cbo
Copy link
Contributor

ds-cbo commented Jun 10, 2024

@bdraco Out of curiosity a side-question: What did you do/use to make those graphs?

@bdraco
Copy link
Contributor Author

bdraco commented Jun 10, 2024

They are made using cProfile and than it's converted to a callgrind file and opened in qcachegrind

More details at https://www.home-assistant.io/integrations/profiler/

@bdraco bdraco marked this pull request as ready for review June 10, 2024 14:25
@spacegaier spacegaier merged commit b1b1090 into alecthomas:master Jun 16, 2024
8 checks passed
@bdraco bdraco deleted the hash_once branch June 16, 2024 17:09
@bdraco
Copy link
Contributor Author

bdraco commented Jun 16, 2024

Thanks. Looking forward to this one in a production HA

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.

4 participants