From 2ff391c5141adef19c28e9e2ea8e01a74e28c57f Mon Sep 17 00:00:00 2001
From: Hugues <hugues.bruant@affirm.com>
Date: Wed, 20 Apr 2022 04:47:33 -0700
Subject: [PATCH] make_simplified_union: simpler and faster (#12630)

Recent attempts at speedup introduced some convoluted logic that both
reduced accuracy and caused performance regressions. Fix this and add
more comments to clarify the reasoning behind the optimization.
---
 mypy/typeops.py | 47 ++++++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/mypy/typeops.py b/mypy/typeops.py
index 9eb2c9bee18f..e9127aee0060 100644
--- a/mypy/typeops.py
+++ b/mypy/typeops.py
@@ -410,26 +410,29 @@ def _remove_redundant_union_items(items: List[ProperType], keep_erased: bool) ->
 
         # Keep track of the truishness info for deleted subtypes which can be relevant
         cbt = cbf = False
-        num_items = len(items)
         for j, tj in enumerate(items):
-            if i != j:
-                # NB: The first check below is an optimization to
-                #     avoid very expensive computations with large
-                #     unions involving literals. We approximate the
-                #     results for unions with many items. This is
-                #     "fine" since simplifying these union items is
-                #     (almost) always optional.
-                if (
-                    (num_items < 5
-                     or is_likely_literal_supertype(item)
-                     or not is_simple_literal(tj))
-                    and is_proper_subtype(tj, item, keep_erased_types=keep_erased)
-                    and is_redundant_literal_instance(item, tj)  # XXX?
-                ):
-                    # We found a redundant item in the union.
-                    removed.add(j)
-                    cbt = cbt or tj.can_be_true
-                    cbf = cbf or tj.can_be_false
+            if (
+                i == j
+                # avoid further checks if this item was already marked redundant.
+                or j in removed
+                # if the current item is a simple literal then this simplification loop can
+                # safely skip all other simple literals as two literals will only ever be
+                # subtypes of each other if they are equal, which is already handled above.
+                # However, if the current item is not a literal, it might plausibly be a
+                # supertype of other literals in the union, so we must check them again.
+                # This is an important optimization as is_proper_subtype is pretty expensive.
+                or (k is not None and is_simple_literal(tj))
+            ):
+                continue
+            # actual redundancy checks
+            if (
+                is_redundant_literal_instance(item, tj)  # XXX?
+                and is_proper_subtype(tj, item, keep_erased_types=keep_erased)
+            ):
+                # We found a redundant item in the union.
+                removed.add(j)
+                cbt = cbt or tj.can_be_true
+                cbf = cbf or tj.can_be_false
         # if deleted subtypes had more general truthiness, use that
         if not item.can_be_true and cbt:
             items[i] = true_or_false(item)
@@ -439,12 +442,6 @@ def _remove_redundant_union_items(items: List[ProperType], keep_erased: bool) ->
     return [items[i] for i in range(len(items)) if i not in removed]
 
 
-def is_likely_literal_supertype(t: ProperType) -> bool:
-    """Is the type likely to cause simplification of literal types in unions?"""
-    return isinstance(t, Instance) and t.type.fullname in ('builtins.object',
-                                                           'builtins.str')
-
-
 def _get_type_special_method_bool_ret_type(t: Type) -> Optional[Type]:
     t = get_proper_type(t)