perf: add __slots__ to SubtypeContext

[ngnpope]

Follow up to afd5a38.

master                    6.544s (0.0%) | stdev 0.056s 
slotted-subtypecontext    6.512s (-0.5%) | stdev 0.060s 

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

[sterliakov]

Nice, thanks! This shouldn't harm, but should probably have less impact: one SubtypeContext is reused in nested is_subtype calls, while visitors are created in each of them.

[JukkaL]

Adding __slots__ isn't expected to have any real performance impact after compilation, since mypyc essentially applies the rough equivalent of the __slots__ optimization automatically to compiled (native) classes. So any performance change here is likely noise -- random fluctuations are often about 0.5%, when using the perf_compare.py tool, unfortunately.

A potential workaround to noise is to add a few different optimizations in a branch and measure their overall impact. If they add up to close to 1.0% performance improvement, we can be reasonably sure that they are beneficial in aggregate.

[ngnpope]

Ok. Good to know! 🙂

[ngnpope]

Just out of interest, I did another run to see how this and Stanislav's previous one stack over the commit before:

cb3bddd4                  6.356s (0.0%) | stdev 0.042s 
afd5a382                  6.354s (-0.0%) | stdev 0.039s 
slotted-subtypecontext    6.338s (-0.3%) | stdev 0.030s 

So neither of them are significant. I'll leave this up to you as to whether you want to merge or close it - it won't do any harm, but isn't going to make much of a dent as you say.

[sterliakov]

add a few different optimizations in a branch

I thought about that, but finally ruled this out: mixing multiple updates together means some of them can be masked regressions, with total improvement being caused by other changes. Maybe we should write a benchmark suite for subtype checks instead? Something that doesn't take tens of minutes to run.

[JukkaL]

I thought about that, but finally ruled this out: mixing multiple updates together means some of them can be masked regressions, with total improvement being caused by other changes.

This only makes sense if all the updates are new optimizations that you are working on. If you can't measure the impact of an individual optimization, it can be hard to decide whether to merge it or not. If you have 5 optimizations that together improve performance by 1.0%, it will be easier to make the decision to accept all the optimizations as a group. Even if some of them might actually be regressions, we can live with it since we have a high confidence that together they improve performance significantly. (There still needs to be some plausible justification why each optimization is likely to improve performance.)

[JukkaL]

Let's close this since managing __slots__ adds some minor friction to editing code. This was a good attempt though, and it's not obvious that __slots__ doesn't help generally. However, if you can show that adding __slots__ helps with test performance when running without compilation, we can reconsider some of these.