-
-
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
Speed up stubs suggestions #17965
Speed up stubs suggestions #17965
Conversation
This is starting to show up on profiles, especially incremental ones
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Thanks! Looks good, left some minor optional comments.
mypy/stubinfo.py
Outdated
def is_legacy_bundled_package(prefix: str) -> bool: | ||
return prefix in legacy_bundled_packages | ||
def is_module_from_legacy_bundled_package(module: str) -> bool: | ||
top_level = module.split(".")[0] |
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 wonder if module.split(".", 1)[0]
would be faster.
assert stub_distribution_name("babel") == "types-babel" | ||
assert stub_distribution_name("google.cloud.ndb") == "types-google-cloud-ndb" | ||
assert stub_distribution_name("google.cloud.ndb.submodule") == "types-google-cloud-ndb" | ||
assert stub_distribution_name("google.cloud.unknown") is None |
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.
Maybe also test good.protobuf
, since it's a slightly different case as there are two packages under the google
prefix?
mypy/stubinfo.py
Outdated
def approved_stub_package_exists(prefix: str) -> bool: | ||
return is_legacy_bundled_package(prefix) or prefix in non_bundled_packages | ||
def approved_stub_package_exists(module: str) -> bool: | ||
components = module.split(".") |
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.
Again, it might be slightly faster to use module.split(".", 1)[0]
to calculate prefix, and only do the full split in the body of the last if statement as needed.
This comment has been minimized.
This comment has been minimized.
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
See #17948 This is starting to show up on profiles - 1.01x faster on clean (below noise) - 1.02x faster on long - 1.02x faster on openai - 1.01x faster on openai incremental I had a dumb bug that was preventing the optimisation for a while, I'll see if I can make it even faster. Currently it's a small improvement We could also get rid of the legacy stuff in mypy 2.0
See #17948
This is starting to show up on profiles
I had a dumb bug that was preventing the optimisation for a while, I'll see if I can make it even faster. Currently it's a small improvement
We could also get rid of the legacy stuff in mypy 2.0