-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Fine-grained: Support NewType and reset subtype caches #4656
Conversation
NewType work highlighted an issue with subtype caches with stale information leaking, and this fixes that issue as well. We reset the subtype cache in two places: * When calculating the MRO; we reset caches of all base classes as well. * When merging a new version of a TypeInfo, which may have a different MRO; we reset all caches of base classes in the old MRO, as they might no longer be supertypes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question, but looks good
@@ -2122,6 +2127,7 @@ def calculate_mro(self) -> None: | |||
self.is_enum = self._calculate_is_enum() | |||
# The property of falling back to Any is inherited. | |||
self.fallback_to_any = any(baseinfo.fallback_to_any for baseinfo in self.mro) | |||
self.reset_subtype_cache() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't fully understand why this is necessary (though it certainly isn't /wrong/). Adding a new subclass shouldn't invalidate any of the caches, since they store only positive information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that some changes in generic base classes may require this. Say, originally the base class was C[A]
and after update it is C[B]
. This would change the subtype relation even though the MRO is unchanged. If we are doing a refresh of the class there would be no AST merge so without this the subtype cache would not get emptied.
* master: New files shouldn't trigger a coarse-grained rebuild in fg cache mode (python#4669) Bump version to 0.580-dev Update revision history for 0.570 (python#4662) Fine-grained: Fix crashes when refreshing synthetic types (python#4667) Fine-grained: Support NewType and reset subtype caches (python#4656) Fine-grained: Detect changes in additional TypeInfo attributes (python#4659) Fine-grained: Apply semantic analyzer patch callbacks (python#4658) Optimize fine-grained update by using Graph as the cache (python#4622) Cleanup check_reverse_op_method (python#4017) Fine-grained: Fix AST merge issues (python#4652) Optionally check that we don't have duplicate nodes after AST merge (python#4647)
NewType work highlighted an issue with subtype caches with stale information leaking, and this fixes that issue as well. We reset the subtype cache in two places: * When calculating the MRO; we reset caches of all base classes as well. * When merging a new version of a TypeInfo, which may have a different MRO; we reset all caches of base classes in the old MRO, as they might no longer be supertypes.
NewType work highlighted an issue with subtype caches with stale
information leaking, and this fixes that issue as well. We reset the
subtype cache in two places:
When calculating the MRO; we reset caches of all base classes as
well.
When merging a new version of a TypeInfo, which may have a
different MRO; we reset all caches of base classes in the old
MRO, as they might no longer be supertypes.