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

Speed up typechecking of dict, set and list expressions #9477

Merged
merged 2 commits into from Oct 18, 2020

Conversation

huguesb
Copy link
Contributor

@huguesb huguesb commented Sep 24, 2020

Typechecking of dict, set, and list literals currently
goes through typechecking of the generic dict/set/list
constructor internally. This is usually fine but becomes
horrendously slow when the number of items is large:

  • for generic methods, infer_arg_types_in_context is
    called twice
  • infer_arg_types_in_context is O(n**2) where n is
    the number of arguments, which, in the case of a
    literal, is the number of items.

Add an O(n) fast path for deriving the type of simple
container literal expressions. This fast path only handle
a subset of cases but it provides a tremendous speedup for
the relatively common case of large literal constants.

The real-world example that motivated this change is a
1889 lines long dict constant representing the parsed value
of a mock JSON response from a 3rd party service, where
typechecking previously took upwards of 50s and is now
down to under 1s with this fast path.

Typechecking of dict, set, and list literals currentlly
goes through typechecking of the generic dict/set/list
constructor internally. This is usually fine but becomes
horrendously slow when the number of items is large:
 - for generic methods, `infer_arg_types_in_context` is
   called twice
 - `infer_arg_types_in_context` is O(n**2) where `n` is
   the number of arguments, which, in the case of a
   literal, is the number of items.

Add an `O(n)` fast path for deriving the type of simple
container literal expressions. This fast path only handle
a subset of cases but it provides a tremendous speedup for
the relatively common case of large literal constants.

The real-world example that motivated this change is a
1889 lines long dict constant representing the parsed value
of a mock JSON response from a 3rd party service, where
typechecking previously took upwards of 50s and is now
down to under 1s with this fast path.
Copy link
Collaborator

@msullivan msullivan left a comment

Choose a reason for hiding this comment

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

This is fantastic and I'm thrilled to merge it. This has been a pretty silly pain point for a while. Could you add docstrings to the fast path functions explaining the rationale and what cases are covered by the fast paths?

@huguesb
Copy link
Contributor Author

huguesb commented Oct 18, 2020

Added docstrings as requested

@msullivan
Copy link
Collaborator

Thank you!

@hauntsaninja
Copy link
Collaborator

Just wanted to chime in and say this is great! :-) Thanks again!

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