Skip to content

Commit e853b46

Browse files
rchen152meta-codesync[bot]
authored andcommitted
Borrow overload callable instead of cloning for every candidate
Summary: During overload resolution, `call_overload` was cloning the entire `TargetWithTParams<Function>` into every `CalledOverload` result, including for losing candidates that are immediately discarded. Store a reference instead, deferring the clone to the single return point where the winning overload's `Callable` signature is extracted. This eliminates N-1 deep clones per overload call (where N is the number of candidates). Profiling shows ~1.5% CPU reduction and -0.05s (−3.1%) wall time improvement on the static-frame workload. Reviewed By: stroxler Differential Revision: D99483127 fbshipit-source-id: accc3fce718706d42edf0dbad51a9a1f73e760df
1 parent 612133d commit e853b46

1 file changed

Lines changed: 35 additions & 26 deletions

File tree

pyrefly/lib/alt/overload.rs

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ use crate::types::callable::Params;
4545
use crate::types::literal::Lit;
4646
use crate::types::types::Type;
4747

48-
struct CalledOverload {
49-
func: TargetWithTParams<Function>,
48+
struct CalledOverload<'f> {
49+
func: &'f TargetWithTParams<Function>,
5050
res: Type,
5151
ctor_targs: Option<TArgs>,
5252
call_errors: ErrorCollector,
@@ -261,7 +261,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
261261
let (closest_overload, matched) = match Vec1::try_from_vec(arity_compatible_overloads) {
262262
Err(_) => (
263263
CalledOverload {
264-
func: arity_closest_overload.unwrap().0.clone(),
264+
func: arity_closest_overload.unwrap().0,
265265
res: self.heap.mk_any_error(),
266266
ctor_targs: None,
267267
call_errors: self.error_collector(),
@@ -313,10 +313,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
313313
matched_overloads.push(cur_closest);
314314
}
315315
if let Some(first_overload) = matched_overloads.first() {
316+
let func = first_overload.func;
317+
let ctor_targs = first_overload.ctor_targs.clone();
318+
let expected_types = first_overload.expected_types.clone();
316319
closest_overload = CalledOverload {
317-
func: first_overload.func.clone(),
318-
ctor_targs: first_overload.ctor_targs.clone(),
319-
expected_types: first_overload.expected_types.clone(),
320+
func,
321+
ctor_targs,
322+
expected_types,
320323
res: self.unions(matched_overloads.into_map(|o| o.res)),
321324
call_errors: self.error_collector(),
322325
};
@@ -348,7 +351,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
348351
OverloadTrace::new(target.1.signature.clone(), tparams)
349352
};
350353
let all_overload_traces = overloads.iter().map(&mut overload_trace).collect();
351-
let closest_overload_trace = overload_trace(&closest_overload.func);
354+
let closest_overload_trace = overload_trace(closest_overload.func);
352355
self.record_overload_trace(
353356
arguments_range,
354357
all_overload_traces,
@@ -374,7 +377,10 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
374377
);
375378
}
376379
errors.extend(closest_overload.call_errors);
377-
(closest_overload.res, closest_overload.func.1.signature)
380+
(
381+
closest_overload.res,
382+
closest_overload.func.1.signature.clone(),
383+
)
378384
} else {
379385
// Build a string showing the argument types for error messages
380386
let mut arg_type_strs = Vec::new();
@@ -405,7 +411,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
405411
),
406412
"Possible overloads:".to_owned(),
407413
];
408-
for overload in overloads {
414+
for overload in &overloads {
409415
let suffix = if overload.1.signature == closest_overload.func.1.signature {
410416
" [closest match]"
411417
} else {
@@ -417,8 +423,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
417423
.signature
418424
.split_first_param(&mut Owner::new())
419425
.map(|(_, signature)| signature)
420-
.unwrap_or(overload.1.signature),
421-
None => overload.1.signature,
426+
.unwrap_or_else(|| overload.1.signature.clone()),
427+
None => overload.1.signature.clone(),
422428
};
423429
let signature = self
424430
.solver()
@@ -433,7 +439,10 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
433439
ErrorInfo::new(ErrorKind::NoMatchingOverload, context),
434440
msg,
435441
);
436-
(self.heap.mk_any_error(), closest_overload.func.1.signature)
442+
(
443+
self.heap.mk_any_error(),
444+
closest_overload.func.1.signature.clone(),
445+
)
437446
}
438447
}
439448

@@ -480,9 +489,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
480489

481490
/// Returns the overload that matches the given arguments, or the one that produces the fewest
482491
/// errors if none matches, plus a bool to indicate whether we found a match.
483-
fn find_closest_overload(
492+
fn find_closest_overload<'c>(
484493
&self,
485-
overloads: &Vec1<&TargetWithTParams<Function>>,
494+
overloads: &Vec1<&'c TargetWithTParams<Function>>,
486495
metadata: &FuncMetadata,
487496
self_obj: Option<&Type>,
488497
args: &[CallArg],
@@ -491,9 +500,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
491500
errors: &ErrorCollector,
492501
hint: Option<HintRef>,
493502
ctor_targs: &Option<&mut TArgs>,
494-
) -> (CalledOverload, bool) {
503+
) -> (CalledOverload<'c>, bool) {
495504
let mut matched_overloads = Vec::with_capacity(overloads.len());
496-
let mut closest_unmatched_overload: Option<CalledOverload> = None;
505+
let mut closest_unmatched_overload: Option<CalledOverload<'c>> = None;
497506
for callable in overloads {
498507
let called_overload = self.call_overload(
499508
callable,
@@ -536,7 +545,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
536545
}
537546
});
538547
if nargs_unknown {
539-
let has_varargs = |o: &CalledOverload| {
548+
let has_varargs = |o: &CalledOverload<'_>| {
540549
matches!(
541550
&o.func.1.signature.params, Params::List(params)
542551
if params.items().iter().any(|p| matches!(p, Param::Varargs(..))))
@@ -549,7 +558,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
549558
kw.arg.is_none() && !matches!(kw.value.infer(self, errors), Type::TypedDict(_))
550559
});
551560
if nkeywords_unknown {
552-
let has_kwargs = |o: &CalledOverload| {
561+
let has_kwargs = |o: &CalledOverload<'_>| {
553562
matches!(
554563
&o.func.1.signature.params, Params::List(params)
555564
if params.items().iter().any(|p| matches!(p, Param::Kwargs(..))))
@@ -615,7 +624,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
615624
.iter()
616625
.find_position(|o| {
617626
let res = self.call_overload(
618-
&o.func,
627+
o.func,
619628
metadata,
620629
self_obj,
621630
&materialized_args,
@@ -645,7 +654,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
645654
.expect("Could not find selected overload");
646655
// Now that we've selected an overload, use the hint to contextually type the arguments.
647656
let contextual_overload = self.call_overload(
648-
&overload.func,
657+
overload.func,
649658
metadata,
650659
self_obj,
651660
args,
@@ -682,7 +691,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
682691

683692
fn disambiguate_overloads_spec_compliant(
684693
&self,
685-
matched_overloads: &[CalledOverload],
694+
matched_overloads: &[CalledOverload<'_>],
686695
) -> Option<usize> {
687696
// Step 5, part 2: are all remaining return types equivalent to one another?
688697
// If not, the call is ambiguous.
@@ -697,7 +706,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
697706
Some(0)
698707
}
699708

700-
fn disambiguate_overloads(&self, matched_overloads: &[CalledOverload]) -> Option<usize> {
709+
fn disambiguate_overloads(&self, matched_overloads: &[CalledOverload<'_>]) -> Option<usize> {
701710
// When a call to an overloaded function may match multiple overloads, the spec says to
702711
// return Any when the return types are not all equivalent.
703712
// However, neither mypy nor pyright fully follows this part of the spec, and many
@@ -729,9 +738,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
729738
Some(candidate)
730739
}
731740

732-
fn call_overload(
741+
fn call_overload<'c>(
733742
&self,
734-
callable: &TargetWithTParams<Function>,
743+
callable: &'c TargetWithTParams<Function>,
735744
metadata: &FuncMetadata,
736745
self_obj: Option<&Type>,
737746
args: &[CallArg],
@@ -740,7 +749,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
740749
errors: &ErrorCollector,
741750
hint: Option<HintRef>,
742751
ctor_targs: &Option<&mut TArgs>,
743-
) -> CalledOverload {
752+
) -> CalledOverload<'c> {
744753
// Create a copy of the class type arguments (if any) that should be filled in by this call.
745754
// The `callable_infer` call below will fill in this copy with the type arguments set
746755
// by the current overload, and we'll later use the copy to fill in the original
@@ -771,7 +780,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
771780
}
772781

773782
CalledOverload {
774-
func: callable.clone(),
783+
func: callable,
775784
res,
776785
ctor_targs: overload_ctor_targs,
777786
call_errors,

0 commit comments

Comments
 (0)