Speed up bind_self() in trivial cases#19024
Conversation
This comment has been minimized.
This comment has been minimized.
|
Hm, the little |
This comment has been minimized.
This comment has been minimized.
|
OK, that didn't go well. I guess it would be easier to just bake out this part of the optimization, most likely it is minor compared to the other part. |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
| if isinstance(defn, Decorator): | ||
| return analyze_var(name, defn.var, itype, mx) | ||
| typ = function_type(defn, mx.chk.named_type("builtins.function")) | ||
| is_trivial_self = False |
There was a problem hiding this comment.
Would it make sense to introduce an interface implemented by Decorator and FuncBase and put an is_trivial_self() property/method there? I remember seeing similar decorator/func ladders for other reasons too, so such refactoring won't be wasted - they should have some common properties, after all. (and this block is already copy-pasted twice)
There was a problem hiding this comment.
Yeah, I was not sure about this, ultimately I decided to save some microseconds by "inlining" it.
There was a problem hiding this comment.
Would you mind me profiling this difference (inline vs shared interface) tomorrow? I wanted to figure out how to profile mypyc-compiled code reliably, sounds like a good chance. (Anyway this isn't blocking, this part can be refactored separately later - perhaps together with some other "shared" methods)
There was a problem hiding this comment.
I definitely can't stop you, but I guess this may be tricky, as difference is likely small.
There was a problem hiding this comment.
Likely non-existent when running, say, whole self-check, but easy to microbenchmark! (but probably it'd be better to profile some reasonable refactoring attempt instead)
There was a problem hiding this comment.
FWIW, I've been working on a script to simplify profiling mypyc-compiled mypy. The current version I have uses py-spy and doesn't work on 3.12 and later, though. I'll probably switch to perf instead (Linux only).
There was a problem hiding this comment.
Yep, I'm actually looking at perf for this purpose since I'm already familiar with it:)
There was a problem hiding this comment.
Anyway, I don't want to wait long time for more precise measurements here (since the perf win is obvious). Unless there are objections, I will merge this soon (you can still profile and fine tune this later).
|
Anyway, after baking out the second part, I still see >3% performance improvement. |
3 participants
See #18991 for context.
We can skip all of logic in
check_self_arg()and 90% of logic inbind_self()for methods with trivialself/cls(i.e. if first argument has no explicit annotation and there is noSelfin signature).Locally I see 3-4% performance improvement (for self-check with non-compiled mypy).