Skip to content

Commit abaa735

Browse files
authored
[ty] Improve UnionBuilder performance by changing Type::is_subtype_of calls to Type::is_redundant_with (#22337)
1 parent c02d164 commit abaa735

1 file changed

Lines changed: 24 additions & 25 deletions

File tree

crates/ty_python_semantic/src/types/builder.rs

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,13 @@
2828
//!
2929
//! In practice, there are two kinds of unions found in the wild: relatively-small unions made up
3030
//! of normal user types (classes, etc), and large unions made up of literals, which can occur via
31-
//! large enums (not yet implemented) or from string/integer/bytes literals, which can grow due to
32-
//! literal arithmetic or operations on literal strings/bytes. For normal unions, it's most
33-
//! efficient to just store the member types in a vector, and do O(n^2) `is_subtype_of` checks to
34-
//! maintain the union in simplified form. But literal unions can grow to a size where this becomes
35-
//! a performance problem. For this reason, we group literal types in `UnionBuilder`. Since every
36-
//! different string literal type shares exactly the same possible super-types, and none of them
37-
//! are subtypes of each other (unless exactly the same literal type), we can avoid many
38-
//! unnecessary `is_subtype_of` checks.
31+
//! large enums or from string/integer/bytes literals, which can grow due to literal arithmetic or
32+
//! operations on literal strings/bytes. For normal unions, it's most efficient to just store the
33+
//! member types in a vector, and do O(n^2) redundancy checks to maintain the union in simplified
34+
//! form. But literal unions can grow to a size where this becomes a performance problem. For this
35+
//! reason, we group literal types in `UnionBuilder`. Since every different string literal type
36+
//! shares exactly the same possible super-types, and none of them are subtypes of each other
37+
//! (unless exactly the same literal type), we can avoid many unnecessary redundancy checks.
3938
4039
use crate::types::enums::{enum_member_literals, enum_metadata};
4140
use crate::types::type_ordering::union_or_intersection_elements_ordering;
@@ -110,30 +109,30 @@ impl<'db> UnionElement<'db> {
110109
// to determine whether the element should be retained in the set.
111110
//
112111
// If `ignore` or `collapse` is `true` for any element in the set,
113-
// we no longer need to do any expensive subtyping checks for any
112+
// we no longer need to do any expensive redundancy checks for any
114113
// further elements in the set:
115114
//
116-
// - if `ignore` is `true`, this indicates that `other_type` is a
117-
// subtype of one of the literals in this set. Given this fact,
115+
// - if `ignore` is `true`, this indicates that `other_type` is
116+
// redundant with one of the literals in this set. Given this fact,
118117
// it cannot be possible for any other literals in this set to be
119-
// a subtype of `other_type`.
118+
// redundant with `other_type`.
120119
// - if `collapse` is `true`, all literals of this kind will be
121120
// removed from the union, so it's irrelevant to answer the
122121
// question of which literals should remain in this set.
123122
//
124-
// We therefore only ask if `ty` is a subtype of `other_type` if
123+
// We therefore only ask if `ty` is redundant with `other_type` if
125124
// both `ignore` and `collapse` are `false`. If either is `true`,
126-
// we skip the expensive subtype check and return `true`.
125+
// we skip the expensive redundancy check and return `true`.
127126
let mut should_retain_type = |ty| {
128-
if ignore || other_type.is_subtype_of(db, ty) {
127+
if ignore || other_type.is_redundant_with(db, ty) {
129128
ignore = true;
130129
return true;
131130
}
132131
if collapse || other_type_negated().is_subtype_of(db, ty) {
133132
collapse = true;
134133
return true;
135134
}
136-
!ty.is_subtype_of(db, other_type)
135+
!ty.is_redundant_with(db, other_type)
137136
};
138137

139138
let should_keep = match self {
@@ -142,23 +141,23 @@ impl<'db> UnionElement<'db> {
142141
literals.retain(|literal| should_retain_type(Type::IntLiteral(*literal)));
143142
!literals.is_empty()
144143
} else {
145-
!Type::IntLiteral(literals[0]).is_subtype_of(db, other_type)
144+
!Type::IntLiteral(literals[0]).is_redundant_with(db, other_type)
146145
}
147146
}
148147
UnionElement::StringLiterals(literals) => {
149148
if other_type.splits_literals(db, LiteralKind::String) {
150149
literals.retain(|literal| should_retain_type(Type::StringLiteral(*literal)));
151150
!literals.is_empty()
152151
} else {
153-
!Type::StringLiteral(literals[0]).is_subtype_of(db, other_type)
152+
!Type::StringLiteral(literals[0]).is_redundant_with(db, other_type)
154153
}
155154
}
156155
UnionElement::BytesLiterals(literals) => {
157156
if other_type.splits_literals(db, LiteralKind::Bytes) {
158157
literals.retain(|literal| should_retain_type(Type::BytesLiteral(*literal)));
159158
!literals.is_empty()
160159
} else {
161-
!Type::BytesLiteral(literals[0]).is_subtype_of(db, other_type)
160+
!Type::BytesLiteral(literals[0]).is_redundant_with(db, other_type)
162161
}
163162
}
164163
UnionElement::Type(existing) => return ReduceResult::Type(*existing),
@@ -367,10 +366,10 @@ impl<'db> UnionBuilder<'db> {
367366
UnionElement::Type(existing) => {
368367
// e.g. `existing` could be `Literal[""] & Any`,
369368
// and `ty` could be `Literal[""]`
370-
if ty.is_subtype_of(self.db, *existing) {
369+
if ty.is_redundant_with(self.db, *existing) {
371370
return;
372371
}
373-
if existing.is_subtype_of(self.db, ty) {
372+
if existing.is_redundant_with(self.db, ty) {
374373
to_remove = Some(index);
375374
continue;
376375
}
@@ -412,12 +411,12 @@ impl<'db> UnionBuilder<'db> {
412411
continue;
413412
}
414413
UnionElement::Type(existing) => {
415-
if ty.is_subtype_of(self.db, *existing) {
414+
if ty.is_redundant_with(self.db, *existing) {
416415
return;
417416
}
418417
// e.g. `existing` could be `Literal[b""] & Any`,
419418
// and `ty` could be `Literal[b""]`
420-
if existing.is_subtype_of(self.db, ty) {
419+
if existing.is_redundant_with(self.db, ty) {
421420
to_remove = Some(index);
422421
continue;
423422
}
@@ -459,12 +458,12 @@ impl<'db> UnionBuilder<'db> {
459458
continue;
460459
}
461460
UnionElement::Type(existing) => {
462-
if ty.is_subtype_of(self.db, *existing) {
461+
if ty.is_redundant_with(self.db, *existing) {
463462
return;
464463
}
465464
// e.g. `existing` could be `Literal[1] & Any`,
466465
// and `ty` could be `Literal[1]`
467-
if existing.is_subtype_of(self.db, ty) {
466+
if existing.is_redundant_with(self.db, ty) {
468467
to_remove = Some(index);
469468
continue;
470469
}

0 commit comments

Comments
 (0)